Fix multiple headers with the same name #1666
Conversation
|
@dgaw Thanks a lot! Could you also include a changelog entry? :) We collect entries in different files in https://github.com/haskell-servant/servant/tree/master/changelog.d and amalgamate them later during release |
|
@tchoutri I've added a changelog entry and removed an old comment. |
| buildHeadersTo headers = case L.find wantedHeader headers of | ||
| Nothing -> MissingHeader `HCons` buildHeadersTo headers | ||
| Just header@(_, val) -> case parseHeader val of | ||
| Left _err -> UndecodableHeader val `HCons` buildHeadersTo (L.delete header headers) | ||
| Right h -> Header h `HCons` buildHeadersTo (L.delete header headers) | ||
| where wantedHeader (h, _) = h == wantedHeaderName | ||
| wantedHeaderName = CI.mk . pack $ symbolVal (Proxy :: Proxy h) |
There was a problem hiding this comment.
I had a look at multiple RFCs regarding HTTP header fields, i.e.:
- [1] https://www.rfc-editor.org/rfc/rfc7230#section-3.2.2
- [2] https://www.rfc-editor.org/rfc/rfc9110#name-field-order
- [3] https://www.rfc-editor.org/rfc/rfc6265.html
- [4] https://www.rfc-editor.org/rfc/rfc8941.html
- [5] https://httpwg.org/http-extensions/draft-ietf-httpbis-retrofit.html
At the end, we do not still have a precise definition for which field lines with the same field name are allowed to be present in the HTTP message or not (except for the Set-Cookie), and which ones are allowed to be merged in some way.
We have something about it, see [4] and [5], and those RFCs are pretty new or are still drafts.
I think that the current (fixed) approach of not manipulating the requested header fields is ok.
We'll be able to create also not valid messages, and this is good for testing (here being not valid means that we could generate HTTP messages with header fields with the same field name, for which we require to have just one line, and not multiple ones).
I would just add a short comment that summarise this behaviour, like: The current implementation do not manipulate HTTP header field lines in any way, like merging field lines with the same field name in a single line.
There was a problem hiding this comment.
Thanks for doing the research @marinelli! I've added the suggested comment.
|
This is now released on Hackage and is visible in the changelog for servant-client-0.20. |
Fix recursion in the instance
BuildHeadersTo (Header h v ': xs)so that multiple headers with the same name work inservant-client.Fixes #1665