Skip to content

Commit

Permalink
fix: any handler sets a default application/json
Browse files Browse the repository at this point in the history
Now it sets application/octet-stream as the generic type.
  • Loading branch information
steve-chavez committed Dec 12, 2023
1 parent 4fb521c commit 6b9fde5
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #3054, Fix not allowing special characters in JSON keys - @laurenceisla
- #2344, Replace JSON parser error with a clearer generic message - @develop7
- #3100, Add missing in-database configuration option for `jwt-cache-max-lifetime` - @laurenceisla
- #3089, The any media type handler now sets `Content-Type: application/octet-stream` by default instead of `Content-Type: application/json` - @steve-chavez

## [12.0.0] - 2023-12-01

Expand Down
21 changes: 9 additions & 12 deletions src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import PostgREST.SchemaCache.Representations (DataRepresentation (..),
RepresentationsMap)
import PostgREST.SchemaCache.Routine (MediaHandler (..),
MediaHandlerMap,
ResolvedHandler,
Routine (..),
RoutineMap,
RoutineParam (..),
Expand Down Expand Up @@ -992,9 +993,9 @@ addFilterToLogicForest :: CoercibleFilter -> [CoercibleLogicTree] -> [CoercibleL
addFilterToLogicForest flt lf = CoercibleStmnt flt : lf

-- | Do content negotiation. i.e. choose a media type based on the intersection of accepted/produced media types.
negotiateContent :: AppConfig -> ApiRequest -> QualifiedIdentifier -> [MediaType] -> MediaHandlerMap -> Either ApiRequestError (MediaHandler, MediaType)
negotiateContent :: AppConfig -> ApiRequest -> QualifiedIdentifier -> [MediaType] -> MediaHandlerMap -> Either ApiRequestError ResolvedHandler
negotiateContent conf ApiRequest{iAction=act, iPreferences=Preferences{preferRepresentation=rep}} identifier accepts produces =
defaultMTAnyToMTJSON $ case (act, firstAcceptedPick) of
case (act, firstAcceptedPick) of
(_, Nothing) -> Left . MediaTypeError $ map MediaType.toMime accepts
(ActionMutate _, Just (x, mt)) -> Right (if rep == Just Full then x else NoAgg, mt)
-- no need for an aggregate on HEAD https://github.com/PostgREST/postgrest/issues/2849
Expand All @@ -1003,23 +1004,19 @@ negotiateContent conf ApiRequest{iAction=act, iPreferences=Preferences{preferRep
(ActionInvoke InvHead, Just (_, mt)) -> Right (NoAgg, mt)
(_, Just (x, mt)) -> Right (x, mt)
where
-- the initial handler in the schema cache has a */* to BuiltinAggJson but it doesn't preserve the media type (application/json)
-- we just convert the default */* to application/json here
-- TODO resolving to "application/json" for "*/*" is not correct when using a "*/*" custom handler media type.
-- We should return "application/octet-stream" as the generic type instead.
defaultMTAnyToMTJSON = mapRight (\(x, y) -> (x, if y == MTAny then MTApplicationJSON else y))
firstAcceptedPick = listToMaybe $ mapMaybe matchMT accepts -- If there are multiple accepted media types, pick the first. This is usual in content negotiation.
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)
m@MTVndArrayJSONStrip -> Just (BuiltinAggArrayJsonStrip, m)
m@(MTVndPlan (MTVndSingularJSON strip) _ _) -> mtPlanToNothing $ Just (BuiltinAggSingleJson strip, m)
m@(MTVndPlan MTVndArrayJSONStrip _ _) -> mtPlanToNothing $ Just (BuiltinAggArrayJsonStrip, m)
-- TODO the plan should have its own MediaHandler instead of relying on MediaType
m@(MTVndPlan mType _ _) -> mtPlanToNothing $ (,) <$> (fst <$> lookupHandler mType) <*> pure m
-- all the other media types can be overridden
m@(MTVndPlan mType _ _) -> mtPlanToNothing $ (,) <$> lookupHandler mType <*> pure m
x -> (,) <$> lookupHandler x <*> pure x
x -> lookupHandler x
mtPlanToNothing x = if configDbPlanEnabled conf then x else Nothing -- don't find anything if the plan media type is not allowed
lookupHandler mt =
HM.lookup (RelId identifier, MTAny) produces <|> -- lookup handler that applies to `*/*` and identifier
HM.lookup (RelId identifier, mt) produces <|> -- lookup handler that applies to a particular media type and identifier
HM.lookup (RelAnyElement, mt) produces -- lookup handler that applies to a particular media type and anyelement
HM.lookup (RelId identifier, MTAny) produces <|> -- lookup for identifier and `*/*`
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
27 changes: 17 additions & 10 deletions src/PostgREST/SchemaCache.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,10 @@ allViewsKeyDependencies =

initialMediaHandlers :: MediaHandlerMap
initialMediaHandlers =
HM.insert (RelAnyElement, MediaType.MTAny ) BuiltinOvAggJson $
HM.insert (RelAnyElement, MediaType.MTApplicationJSON) BuiltinOvAggJson $
HM.insert (RelAnyElement, MediaType.MTTextCSV ) BuiltinOvAggCsv $
HM.insert (RelAnyElement, MediaType.MTGeoJSON ) BuiltinOvAggGeoJson
HM.insert (RelAnyElement, MediaType.MTAny ) (BuiltinOvAggJson, MediaType.MTApplicationJSON) $
HM.insert (RelAnyElement, MediaType.MTApplicationJSON) (BuiltinOvAggJson, MediaType.MTApplicationJSON) $
HM.insert (RelAnyElement, MediaType.MTTextCSV ) (BuiltinOvAggCsv, MediaType.MTTextCSV) $
HM.insert (RelAnyElement, MediaType.MTGeoJSON ) (BuiltinOvAggGeoJson, MediaType.MTGeoJSON)
HM.empty

mediaHandlers :: PgVersion -> Bool -> SQL.Statement [Schema] MediaHandlerMap
Expand All @@ -1142,7 +1142,11 @@ mediaHandlers pgVer =
lower(t.typname) as typname,
b.oid as base_oid,
b.typname AS basetypname,
t.typnamespace
t.typnamespace,
case t.typname
when '*/*' then 'application/octet-stream'
else t.typname
end as resolved_media_type
FROM pg_type t
JOIN pg_type b ON t.typbasetype = b.oid
WHERE
Expand All @@ -1154,15 +1158,16 @@ mediaHandlers pgVer =
proc.proname as handler_name,
arg_schema.nspname::text as target_schema,
arg_name.typname::text as target_name,
media_types.typname as media_type
media_types.typname as media_type,
media_types.resolved_media_type
from media_types
join pg_proc proc on proc.prorettype = media_types.oid
join pg_namespace proc_schema on proc_schema.oid = proc.pronamespace
join pg_aggregate agg on agg.aggfnoid = proc.oid
join pg_type arg_name on arg_name.oid = proc.proargtypes[0]
join pg_namespace arg_schema on arg_schema.oid = arg_name.typnamespace
where
proc_schema.nspname = ANY($1) and
proc_schema.nspname = ANY('{test}') and
proc.pronargs = 1 and
arg_name.oid in (select reltype from all_relations)
union
Expand All @@ -1171,7 +1176,8 @@ mediaHandlers pgVer =
mtype.typname as handler_name,
pro_sch.nspname as target_schema,
proname as target_name,
mtype.typname as media_type
mtype.typname as media_type,
mtype.resolved_media_type
from pg_proc proc
join pg_namespace pro_sch on pro_sch.oid = proc.pronamespace
join media_types mtype on proc.prorettype = mtype.oid
Expand All @@ -1181,12 +1187,13 @@ mediaHandlers pgVer =

decodeMediaHandlers :: HD.Result MediaHandlerMap
decodeMediaHandlers =
HM.fromList . fmap (\(x, y, z) -> ((if isAnyElement y then RelAnyElement else RelId y, z), CustomFunc x) ) <$> HD.rowList caggRow
HM.fromList . fmap (\(x, y, z, w) -> ((if isAnyElement y then RelAnyElement else RelId y, z), (CustomFunc x, w)) ) <$> HD.rowList caggRow
where
caggRow = (,,)
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)

timezones :: Bool -> SQL.Statement () TimezoneNames
timezones = SQL.Statement sql HE.noParams decodeTimezones
Expand Down
5 changes: 4 additions & 1 deletion src/PostgREST/SchemaCache/Routine.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module PostgREST.SchemaCache.Routine
, funcReturnsCompositeAlias
, funcReturnsSingle
, MediaHandlerMap
, ResolvedHandler
, MediaHandler(..)
) where

Expand Down Expand Up @@ -145,4 +146,6 @@ funcTableName proc = case pdReturnType proc of
Single (Composite qi _) -> Just $ qiName qi
_ -> Nothing

type MediaHandlerMap = HM.HashMap (RelIdentifier, MediaType.MediaType) MediaHandler
-- the resolved handler also carries the media type because MTAny (*/*) is resolved to a different media type
type ResolvedHandler = (MediaHandler, MediaType.MediaType)
type MediaHandlerMap = HM.HashMap (RelIdentifier, MediaType.MediaType) ResolvedHandler
16 changes: 7 additions & 9 deletions test/spec/Feature/Query/CustomMediaSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -225,31 +225,30 @@ spec = describe "custom media types" $ do

context "any media type" $ do
context "on functions" $ do
-- TODO not correct, it should return the generic "application/octet-stream"
it "returns application/json for */* if not explicitly set" $ do
request methodGet "/rpc/ret_any_mt" (acceptHdrs "*/*") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}

it "accepts any media type and sets it as a header" $ do
it "accepts any media type and sets the generic octet-stream as content type" $ do
request methodGet "/rpc/ret_any_mt" (acceptHdrs "app/bingo") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "app/bingo"]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}

request methodGet "/rpc/ret_any_mt" (acceptHdrs "text/bango") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "text/bango"]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}

request methodGet "/rpc/ret_any_mt" (acceptHdrs "image/boingo") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "image/boingo"]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}

it "returns custom media type for */* if explicitly set" $ do
Expand All @@ -276,12 +275,11 @@ spec = describe "custom media types" $ do
`shouldRespondWith` 415

context "on tables" $ do
-- TODO not correct, it should return the generic "application/octet-stream"
it "returns application/json for */* if not explicitly set" $ do
request methodGet "/some_numbers?val=eq.1" (acceptHdrs "*/*") ""
`shouldRespondWith` "anything\n1"
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}

it "accepts any media type and sets it as a header" $ do
Expand All @@ -298,5 +296,5 @@ spec = describe "custom media types" $ do
request methodGet "/some_numbers?val=eq.4" (acceptHdrs "unknown/unknown") ""
`shouldRespondWith` "anything\n4"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "unknown/unknown"]
, matchHeaders = ["Content-Type" <:> "application/octet-stream"]
}
12 changes: 9 additions & 3 deletions test/spec/fixtures/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3663,10 +3663,14 @@ declare
resp bytea;
begin
case req_accept
when 'app/chico' then resp := 'chico';
when 'app/harpo' then resp := 'harpo';
when 'app/chico' then
perform set_config('response.headers', json_build_array(json_build_object('Content-Type', req_accept))::text, true);
resp := 'chico';
when 'app/harpo' then
perform set_config('response.headers', json_build_array(json_build_object('Content-Type', req_accept))::text, true);
resp := 'harpo';
when '*/*' then
perform set_config('response.headers', '[{"Content-Type": "app/groucho"}]', true);
perform set_config('response.headers', json_build_array(json_build_object('Content-Type', 'app/groucho'))::text, true);
resp := 'groucho';
else
raise sqlstate 'PT415' using message = 'Unsupported Media Type';
Expand All @@ -3689,8 +3693,10 @@ declare
begin
case req_accept
when 'magic/number' then
perform set_config('response.headers', json_build_array(json_build_object('Content-Type', req_accept))::text, true);
prefix := 'magic';
when 'crazy/bingo' then
perform set_config('response.headers', json_build_array(json_build_object('Content-Type', req_accept))::text, true);
prefix := 'crazy';
else
prefix := 'anything';
Expand Down

0 comments on commit 6b9fde5

Please sign in to comment.