add: add "Vary" header to responses #4609
Conversation
develop7
requested review from
laurenceisla,
steve-chavez and
wolfgangwalther
develop7
force-pushed
the
feat-add_vary_header
branch
from
ee88609 to
acad7de
Compare
|
Linking the RFC 7231: Vary for reference. |
|
From reading up on the RFC only a tiny bit it looks to me like... we might not actually need much more sophisticated logic. Of course, it'd be great have some kind of sophisticated way of encoding, possibly in the haskell type system or so, which headers are in the list - instead of hardcoding them (and the forgetting to update the list accordingly when changes are made). But this might well be overengineering. We should certainly not send this header only when we actually varied the response - this defeats the purpose of using this as a cache key on intermediates. So.. this might already be close to "it"? |
|
Let me update the changelog then |
develop7
force-pushed
the
feat-add_vary_header
branch
from
acad7de to
b9fd8d0
Compare
|
BTW do I need tests? |
|
There probably can't be tested much in a meaningful way here. Maybe you can add some expectations for these headers to an existing test somewhere. Conflicted in the changelog. |
src/PostgREST/App.hs
Outdated
| toWaiResponse :: ServerTiming -> Response.PgrstResponse -> Wai.Response | ||
| toWaiResponse timing (Response.PgrstResponse st hdrs bod) = Wai.responseLBS st (hdrs ++ ([serverTimingHeader timing | configServerTimingEnabled])) bod | ||
| toWaiResponse timing (Response.PgrstResponse st hdrs bod) = | ||
| Wai.responseLBS st (hdrs ++ serverTimingHeaders timing ++ [varyHeader]) bod |
There was a problem hiding this comment.
I think it is worth filtering hdrs to make sure Vary is not there.
Not sure what to do in such a case. Error out? Leave them?
There was a problem hiding this comment.
got it, I'd leave it as is
develop7
force-pushed
the
feat-add_vary_header
branch
2 times, most recently
from
e7f1909 to
4472cea
Compare
It would be good to add at least one to prevent regression. |
develop7
force-pushed
the
feat-add_vary_header
branch
from
4472cea to
1163c19
Compare
4 participants
RFC: Introduces hardcoded (for now)
Varyheader for all responses. I'm almost certain the logic should be a bit more complex, so absolute do chime in when that's the case.Fixes #133