Consistent handling of multi-valued headers in HttpHeaders [SPR-14223]

[spring-projects-issues]

Sebastiaan van Erk opened SPR-14223 and commented

The org.springframework.http.HttpHeaders class wraps HTTP request headers and makes them easy to work with (e.g., parsing HTTP dates such as in the Last-Modified header) for you.

However, it treats multi-valued headers incorrectly in my opinion. From RFC2616 https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2:

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.

Thus, it seems to me that the following sets of headers mean the same thing:

X-List: a, b, c

X-List: a
X-List: b
X-List: c

X-List: a, b
X-List: c

In all cases the header value of the X-List header is the list of strings "a", "b", "c". Note that a proxy may convert the second or third form to the first form.

According to the documentation on HttpHeaders:

getFirst(String) returns the first value associated with a given header name

I would expect in all 3 cases that getFirst("X-List") returns the string "a". However this is not the case. In the first case it returns "a, b, c", in the second case "a", and in the third case "a, b".

Note that this is actually a problem in the handling of some of the headers (the various Access-Control headers, the If-None-Match header), because they call getFirst or getFirstValueAsList. But this way they ignore any values in repeated headers, which are valid and should be appended to the list according to the RFC.

Since the processing of multiple header values is rather complicated, it would be really helpful if the HttpHeaders class removed this complexity from the API user.

NOTE:

As commented in #18790, there is a problem with the method to split the header into values (getFirstValueAsList): the values themselves may contain commas (ETags are an example of this). It seems that to split the header you need to understand the syntax of the values themselves. I don't see how one could implement this correctly for the generic case (e.g., unknown X-* headers). However, for the known HTTP headers it should be possible to split the list correctly.

---

Affects: 4.2.5

Issue Links:

• ServletWebRequest.isEtagNotModified does not support commas and spaces in client ETags [SPR-14216] #18790 ServletWebRequest.isEtagNotModified does not support commas and spaces in client ETags
• Regression: DefaultCorsProcessor ignores already present Access-Control-Allow-Origin header [SPR-14406] #18977 Regression: DefaultCorsProcessor ignores already present Access-Control-Allow-Origin header
• CorsConfiguration checkHeaders wildcard ALL should allow matching empty strings header [SPR-14617] #19184 CorsConfiguration checkHeaders wildcard ALL should allow matching empty strings header
• Add get/set-IfUnmodifiedSince, get/set-IfMatch methods on HttpHeaders [SPR-14228] #18802 Add get/set-IfUnmodifiedSince, get/set-IfMatch methods on HttpHeaders

Referenced from: commits 55dae61

[spring-projects-issues]

Brian Clozel commented

Hi Sebastiaan van Erk

Just a note on getFirst() - I don't think we should change this contract for two reasons:

• first, HttpHeaders is a MultiValueMap and that method is about getting the first value for a given key
• second, the spec itself seems to describe a header field value as the full value, itself composed of tokens and separators.

I can see why the getFirstValueAsList should be fixed - in fact, I'm currently renaming it to getValuesAsList and appending all the values as you suggested.
On the other hand, I don't know why you would try to only get the first token of a header value with getFirst(); could you explain a bit more here the underlying use case?

Thanks,

[spring-projects-issues]

Sebastiaan van Erk commented

Hi Brian,

I think I misunderstood where "getFirst" came from: I just noticed it's inherited from the MultiValueMap, and as such, in all cases where the header is a "non-list" header you obviously want to return the whole header (and not split it on commas). So I agree it's a bad idea to change the way "getFirst" works. I thought it was the intention of "getFirst" to return the first element of a list header, in which case the above behavior would be weird. I guess it's easy to be confused due to the name of the method (in the case of a "non-list" header, "getFirst" sounds weird because there is only one).

Anyway, for "list" headers I don't see why I would ever care how the list is communicated to me: as 1 header or as multiple, or how it was communicated to me as multiple (e.g., "a,b" and "c" or "a" and "b,c"). As an API user I don't want to deal with the complexity of treating those as different cases. For me it's just a list of values. And in this case, I don't really see a use case for a "getFirstListValue" method (which is how I interpreted the existing "getFirst" method). However, I would very much like to call "getValuesAsList" (so the complexity of merging headers and splitting values has already been done for me). It would be very useful if that method was public.

Best regards,
Sebastiaan

[spring-projects-issues]

Brian Clozel commented

This is now fixed with 55dae618a6.

Note that the new getValuesAsList will only deal with "," separators and non-ETag parsing related issues, since it seems all accessors are available for those already.

This will be available in the next SNAPSHOT version. Feel free to voice your opinion before the next RC version next week!