smithy4s icon indicating copy to clipboard operation
smithy4s copied to clipboard

Add support for comma-separated headers in SimpleRestJson

Open ghostbuster91 opened this issue 10 months ago • 5 comments

According to the RFC:

Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.

It should be possible to send multiple values for a single header by passing them as comma separated list.

However, SimpleRestJson smithy4s treats all such values as a single text value.

repository to reproduce: https://github.com/ghostbuster91/demos/tree/multi-header

calling the endpoint should return {"weather":"2"} but it returns:

$ curl localhost:8080/weather -X POST -H "X-Foo: max-age=3600, must-revalidate"                                       
{"weather":"1"}%

smithy4s version: 0.18.15

ghostbuster91 avatar Apr 15 '24 12:04 ghostbuster91

Thanks for the repro.

I'm not sure whether this should be a smithy4s issue or http4s issue though. @kubukoz, thoughts ?

Baccata avatar Apr 15 '24 13:04 Baccata

Smells like http4s...

kubukoz avatar Apr 15 '24 15:04 kubukoz

ugh I was distracted when submitting the comment, should've elaborated a bit.

http4s appears to just have a single Raw header for an X-Foo like the above, so we'd have to apply extra parsing on it. This seems to me like something that belongs in http4s, because in virtually every other area it gives us solid pre-parsed, structured representations of HTTP messages.

kubukoz avatar Apr 15 '24 16:04 kubukoz

Let's open in http4s. I'm not opposed to fixing it here if they offer a rationale for why they shouldn't do it, but I'd rather we asked first

Baccata avatar Apr 15 '24 19:04 Baccata

I opened an issue in http4s

ghostbuster91 avatar Apr 17 '24 10:04 ghostbuster91