Support multiple values for HTTP header with same key

[qnga]

Deprecated

Shared

• DefaultHttClient.additionalHeaders is deprecated. Set all the headers when creating a new HttpRequest, or modify outgoing requests in DefaultHttpClient.Callback.onStartRequest().

[mickael-menu]

I think we should also modify HttpHeaders.getAll() to split the values by , here:

kotlin-toolkit/readium/shared/src/main/java/org/readium/r2/shared/util/http/HttpHeaders.kt

Line 46 in 1570e80

value

As the caller is using getAll() instead of get(), we can infer that the header supports multiple values and so should be split by , to expand all the values into a single List<String>.

[qnga]

That's right. The get method could become confusing then though: the caller might call it because he's interested only in the first value, not because he knows that he header can't contain multiple values. So get would require him to split values while getAll would not. What do you think?

[qnga]

Actually, public methods are valueForHeader and valuesForHeader. I wonder if the best solution wouldn't be that valueForHeader must be used for single-value headers and valuesForHeader for multiple-value headers. Alternatively, we could leave all splitting to callers as we do now in my PR (that's the OkHttp strategy for instance).

[mickael-menu]

I wonder if the best solution wouldn't be that valueForHeader must be used for single-value headers and valuesForHeader for multiple-value headers.

That's what I had in mind. But we could also have valueForHeader join all the field values with a comma if there are several. If the caller is interested in the first value, they should use valuesForHeader().first.

Unfortunately we can't preemptively split the values as some single-value headers are allowed to contain a comma, e.g.

Expires: Wed, 21 Oct 2015 07:28:00 GMT

[qnga]

Unfortunately, even single-value headers can have multiple occurrences. For instance, I'm afraid that none of both methods would be suitable for reading headers such as Set-Cookie.
valueForHeader would return either only the first occurrence or all the values concatenated in a form difficult to split back. On the other hand, valuesForHeader would dangerously split values at commas.

OkHttp's strategy seems the wisest choice to me.

[mickael-menu]

You're right, HTTP headers are a mess! Which strategy OkHttp uses?

[qnga]

It is the caller’s responsibility to detect and split on commas if their field permits multiple values.

Actually they still offer a helper to get the raw value of the last header with a given name.

In our case, that would mean offering headers(name: String): List<String> and header(name: String): String? (I feel like value could imply true single value). This way, a caller interested in only one value of a header not allowing comma-separated list would only have to call header(name: String).

[mickael-menu]

Sounds good. With header() returning the last value for the name, instead of the first?

[qnga]

Yes! I updated the interface.