diff --git a/CHANGELOG.md b/CHANGELOG.md index dc5ed5f257..95b899df8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. From versio ## Unreleased +## Fixed + +- Fix `Content-Type` not set with custom domain type and `Accept: */*` by @taimoorzaeem in #3391 + ## [14.1] - 2025-11-05 ## Fixed diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index fed1c6ac0d..a41481f7af 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -98,7 +98,7 @@ userApiRequest conf prefs req reqBody = do , iMethod = method , iSchema = schema , iNegotiatedByProfile = negotiatedByProfile - , iAcceptMediaType = maybe [MTAny] (map MediaType.decodeMediaType . parseHttpAccept) $ lookupHeader "accept" + , iAcceptMediaType = maybe [MTApplicationJSON] (map MediaType.decodeMediaType . parseHttpAccept) $ lookupHeader "accept" , iContentMediaType = contentMediaType } where diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index df62291525..cd5c9f71d8 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -1127,7 +1127,10 @@ negotiateContent conf ApiRequest{iAction=act, iPreferences=Preferences{preferRep (ActDb (ActRoutine _ (InvRead True)), Just (_, mt)) -> Right (NoAgg, mt) (_, Just (x, mt)) -> Right (x, mt) where - firstAcceptedPick = listToMaybe $ mapMaybe matchMT accepts -- If there are multiple accepted media types, pick the first. This is usual in content negotiation. + -- If there are multiple accepted media types, pick the first. This is usual in content negotiation. + firstAcceptedPick = listToMaybe $ mapMaybe matchMT accepts <|> mapMaybe matchMTAny accepts + + -- TODO: Needs a refactoring and code cleanup, do not merge matchMT mt = case mt of -- all the vendored media types have special handling as they have media type parameters, they cannot be overridden m@(MTVndSingularJSON strip) -> Just (BuiltinAggSingleJson strip, m) @@ -1138,11 +1141,19 @@ negotiateContent conf ApiRequest{iAction=act, iPreferences=Preferences{preferRep m@(MTVndPlan mType _ _) -> mtPlanToNothing $ ((,) . fst <$> lookupHandler mType) <*> pure m -- all the other media types can be overridden x -> lookupHandler x + + matchMTAny mt = case mt of + m@(MTVndPlan{}) -> mtPlanToNothing $ ((,) . fst <$> lookupAnyHandler) <*> pure m + _ -> lookupAnyHandler + mtPlanToNothing x = if configDbPlanEnabled conf then x else Nothing -- don't find anything if the plan media type is not allowed + lookupHandler mt = - when' defaultSelect (HM.lookup (RelId identifier, MTAny) produces) <|> -- lookup for identifier and `*/*` - when' defaultSelect (HM.lookup (RelId identifier, mt) produces) <|> -- lookup for identifier and a particular media type - HM.lookup (RelAnyElement, mt) produces -- lookup for anyelement and a particular media type + when' defaultSelect (HM.lookup (RelId identifier, mt) produces) <|> -- lookup for identifier and a particular media type + HM.lookup (RelAnyElement , mt) produces -- lookup for anyelement and a particular media type + + lookupAnyHandler = when' defaultSelect (HM.lookup (RelId identifier, MTAny) produces) -- lookup for identifier and */* media type + when' :: Bool -> Maybe a -> Maybe a when' True (Just a) = Just a when' _ _ = Nothing diff --git a/src/PostgREST/SchemaCache.hs b/src/PostgREST/SchemaCache.hs index 528d7905f4..6487572375 100644 --- a/src/PostgREST/SchemaCache.hs +++ b/src/PostgREST/SchemaCache.hs @@ -1078,17 +1078,45 @@ mediaHandlers = proc.pronamespace = ANY($$1::regnamespace[]) and NOT proretset and prokind = 'f'|] +-- We take the union of two maps: +-- First map builds the mapping of domain types -> resolved media type +-- Second map creates the same mapping, but with "*/*" key +-- Example: +-- +------------------------------------------------+ +-- | domain type | resolved media type | +-- +------------------------------------------------+ +-- | text/plain | text/plain | +-- +------------------------------------------------+ +-- | */* | text/plain | +-- +------------------------------------------------+ +-- This allows us to resolve to "text/plain" on both +-- "Accept: text/plain" and +-- "Accept: */*" +-- +-- TODO: This basically doubles the size of the map, so we should do some +-- production scale memory tests to make sure with aren't hitting OOM errors decodeMediaHandlers :: HD.Result MediaHandlerMap -decodeMediaHandlers = - HM.fromList . fmap (\(x, y, z, w) -> - let rel = if isAnyElement y then RelAnyElement else RelId y - in ((rel, z), (CustomFunc x rel, w)) ) <$> HD.rowList caggRow - where - caggRow = (,,,) - <$> (QualifiedIdentifier <$> column HD.text <*> column HD.text) - <*> (QualifiedIdentifier <$> column HD.text <*> column HD.text) - <*> (MediaType.decodeMediaType . encodeUtf8 <$> column HD.text) - <*> (MediaType.decodeMediaType . encodeUtf8 <$> column HD.text) +decodeMediaHandlers = mapFromList <$> HD.rowList caggRow + where + mapFromList aggRowList = + HM.union + (HM.fromList . fmap (aggRowToList False) $ aggRowList) -- type -> type map + (HM.fromList . fmap (aggRowToList True) $ aggRowList) -- */* -> type map + + caggRow = (,,,) + <$> (QualifiedIdentifier <$> column HD.text <*> column HD.text) + <*> (QualifiedIdentifier <$> column HD.text <*> column HD.text) + <*> (MediaType.decodeMediaType . encodeUtf8 <$> column HD.text) + <*> (MediaType.decodeMediaType . encodeUtf8 <$> column HD.text) + + aggRowToList withMTAny (x,y,z,w) = ((rel, mt), (CustomFunc x rel, w)) + where + -- we don't add the */* mapping when rel is "anyelement" because we + -- already have (RelAnyElement, MTAny) in the initial media handler + -- map which maps to "application/json" + mt = if withMTAny && (rel /= RelAnyElement) then MediaType.MTAny else z + rel = if isAnyElement y then RelAnyElement else RelId y + timezones :: Bool -> SQL.Statement () TimezoneNames timezones = SQL.Statement sql HE.noParams decodeTimezones diff --git a/test/spec/Feature/Query/CustomMediaSpec.hs b/test/spec/Feature/Query/CustomMediaSpec.hs index e20b6da672..ed7470413d 100644 --- a/test/spec/Feature/Query/CustomMediaSpec.hs +++ b/test/spec/Feature/Query/CustomMediaSpec.hs @@ -28,14 +28,11 @@ spec = describe "custom media types" $ do simpleBody r `shouldBe` readFixtureFile "1.twkb" simpleHeaders r `shouldContain` [("Content-Type", "application/vnd.twkb")] - it "will fail if there's no aggregate defined for the table" $ do - request methodGet "/lines" (acceptHdrs "text/plain") "" - `shouldRespondWith` - [json| {"code":"PGRST107","details":null,"hint":null,"message":"None of these media types are available: text/plain"} |] - { matchStatus = 406 - , matchHeaders = [ matchContentTypeJson - , "Content-Length" <:> "110" ] - } + it "will succeed if an aggregate exist with the correct media type" $ do + r <- request methodGet "/lines" (acceptHdrs "text/plain") "" + liftIO $ do + simpleBody r `shouldBe` readFixtureFile "lines.twkb" + simpleHeaders r `shouldContain` [("Content-Type", "application/vnd.twkb")] it "can get raw xml output with Accept: text/xml if there's an aggregate defined" $ do request methodGet "/xmltest" (acceptHdrs "text/xml") "" @@ -116,14 +113,22 @@ spec = describe "custom media types" $ do , matchHeaders = ["Content-Type" <:> "text/xml; charset=utf-8"] } - it "should fail with function returning text and Accept: text/xml" $ do + it "should get the return type of function when accept is */*" $ do + request methodGet "/rpc/javascript" + [("Accept", "*/*")] + "" + `shouldRespondWith` + "This is Javascript." + { matchStatus = 200 + , matchHeaders = ["Content-Type" <:> "text/javascript"] + } + + it "should return the content-type as given in function return type" $ do request methodGet "/rpc/welcome" (acceptHdrs "text/xml") "" `shouldRespondWith` - [json| - {"code":"PGRST107","details":null,"hint":null,"message":"None of these media types are available: text/xml"} - |] - { matchStatus = 406 - , matchHeaders = ["Content-Type" <:> "application/json; charset=utf-8"] + "Welcome to PostgREST" + { matchStatus = 200 + , matchHeaders = ["Content-Type" <:> "text/plain; charset=utf-8"] } it "should not fail when the function doesn't return a row" $ do @@ -160,12 +165,11 @@ spec = describe "custom media types" $ do simpleBody r `shouldBe` readFixtureFile "lines.twkb" simpleHeaders r `shouldContain` [("Content-Type", "application/vnd.twkb")] - it "fails if doesn't have an aggregate defined" $ do - request methodGet "/rpc/get_lines" - (acceptHdrs "application/octet-stream") "" - `shouldRespondWith` - [json| {"code":"PGRST107","details":null,"hint":null,"message":"None of these media types are available: application/octet-stream"} |] - { matchStatus = 406 } + it "return the content with the available aggregate and media type" $ do + r <- request methodGet "/rpc/get_lines" (acceptHdrs "application/octet-stream") "" + liftIO $ do + simpleBody r `shouldBe` readFixtureFile "lines.twkb" + simpleHeaders r `shouldContain` [("Content-Type", "application/vnd.twkb")] -- TODO SOH (start of heading) is being added to results it "works if there's an anyelement aggregate defined" $ do diff --git a/test/spec/Feature/Query/RawOutputTypesSpec.hs b/test/spec/Feature/Query/RawOutputTypesSpec.hs index b76df7813e..125fe86b63 100644 --- a/test/spec/Feature/Query/RawOutputTypesSpec.hs +++ b/test/spec/Feature/Query/RawOutputTypesSpec.hs @@ -23,11 +23,20 @@ spec = describe "When raw-media-types config variable is missing or left empty" `shouldRespondWith` [json| [{"id":1}] |] { matchHeaders= ["Content-Type" <:> "application/json; charset=utf-8"] } - it "responds json to a GET request to RPC with Firefox Accept headers" $ - request methodGet "/rpc/get_projects_below?id=3" firefoxAcceptHdrs "" - `shouldRespondWith` [json|[{"id":1,"name":"Windows 7","client_id":1}, {"id":2,"name":"Windows 10","client_id":1}]|] - { matchHeaders= ["Content-Type" <:> "application/json; charset=utf-8"] } - it "responds json to a GET request to RPC with Chrome Accept headers" $ - request methodGet "/rpc/get_projects_below?id=3" chromeAcceptHdrs "" - `shouldRespondWith` [json|[{"id":1,"name":"Windows 7","client_id":1}, {"id":2,"name":"Windows 10","client_id":1}]|] - { matchHeaders= ["Content-Type" <:> "application/json; charset=utf-8"] } + it "TODO" $ + request methodGet "/rpc/get_projects_below?id=3" + firefoxAcceptHdrs "" + `shouldRespondWith` + "id\tname\tclient_id\n1\tWindows 7\t1\n2\tWindows 10\t1\n" + { matchStatus = 200 + , matchHeaders = ["Content-Type" <:> "text/tab-separated-values"] + } + + it "TODO" $ + request methodGet "/rpc/get_projects_below?id=3" + chromeAcceptHdrs "" + `shouldRespondWith` + "id\tname\tclient_id\n1\tWindows 7\t1\n2\tWindows 10\t1\n" + { matchStatus = 200 + , matchHeaders = ["Content-Type" <:> "text/tab-separated-values"] + } diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql index 19167867b6..73c5d474f2 100644 --- a/test/spec/fixtures/schema.sql +++ b/test/spec/fixtures/schema.sql @@ -107,7 +107,7 @@ SET search_path = test, pg_catalog; SET default_tablespace = ''; SET default_with_oids = false; - +create domain "text/javascript" as text; create domain "text/plain" as text; create domain "text/html" as text; create domain "text/xml" as pg_catalog.xml; @@ -1884,6 +1884,10 @@ create or replace function welcome() returns "text/plain" as $$ select 'Welcome to PostgREST'::"text/plain"; $$ language sql; +create or replace function javascript() returns "text/javascript" as $$ +select 'This is Javascript.'::"text/javascript"; +$$ language sql; + create or replace function welcome_twice() returns setof "text/plain" as $$ select 'Welcome to PostgREST' union all