search

PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.29k stars 1.02k forks source link

Content-type not set when function returning a domain representing media type is requested with Accept "*/*" header #3391

Open mkleczek opened 5 months ago

mkleczek commented 5 months ago

Environment

Description of issue

I have the following function:

CREATE DOMAIN "text/javascript" AS text;
CREATE OR REPLACE FUNCTION "module.js"() RETURNS "text/javascript"...

When requested by a browser from:

<script type="module" src="/rpc/module.js"></script>

returned response Content-Type is application/json

It seems the browser is setting Accept header to */* and PostgREST does not set proper content type in this case. As a workaround it is necessary to explicitly set content type in the function using current_setting.

steve-chavez commented 5 months ago

This needs the any handler as mentioned on https://github.com/PostgREST/postgrest/discussions/3387#discussioncomment-9192985

wolfgangwalther commented 5 months ago

Sorry, but that's still a bug. The function definition clearly tells us that this function is only ever going to return text/javascript, so we should never choose application/json when the header is */*.

steve-chavez commented 4 months ago

I agree that this is the ideal behavior and I gave it a shot on https://github.com/steve-chavez/postgrest/commit/760f5ac08d2f7583901eb9593fa865c2ad1a85ac. Adding support for this is simple enough but it's hard to retrofit it with the current table media handlers done with aggregates, many tests break.

Likely we will need to adjust aggregates behavior and cause breaking changes, so I will leave this for a next major version.

wolfgangwalther commented 4 months ago

I'm probably missing something, but shouldn't this just be solved on the haskell part of the code? We should have all information available already, no? I don't understand why we'd need to query more stuff from the SQL side for this. I would have expected we'd just need to adjust the logic where the actual negotiation happens. What am I missing?

steve-chavez commented 3 months ago

Yeah, maybe it can be done purely on the Plan side. I mostly was trying to keep that module simple (since it's already pretty complex) by adding extra info on the schema cache.

taimoorzaeem commented 3 months ago

@steve-chavez @laurenceisla I am still not sure what should be the correct Content-Type that should be returned here. I think the correct Mediatype would be text/javascript. I understand that we need to access this return type from schema cache and return it as the Content-Type. Currently when Accept: */*, it returns Content-Type: application/json which is because of following code line:

HM.insert (RelAnyElement, MediaType.MTAny) (BuiltinOvAggJson, MediaType.MTApplicationJSON) -- SchemaCache.hs:1137:

My question is, how are we gonna know that there is domain which is also a mediatype that we need to return? Sorry if that doesn't make any sense.

wolfgangwalther commented 3 months ago

@steve-chavez @laurenceisla I am still not sure what should be the correct Content-Type that should be returned here. I think the correct Mediatype would be text/javascript.

Yes. Once we decide to call the function defined as RETURNS "text/javascript", we must set text/javascript as the content-type.