Preserve HTTP header case but compare them case insensitive #1166
Conversation
|
cc @losipiuk |
wendigo
force-pushed
the
findepi/fix-http-request-builder-setheader-case-sensitivity-be0601
branch
from
8d856f5
to
bd5f3d5
Compare
| Request request = prepareGet() | ||
| .setUri(URI.create("http://example.com")) | ||
| .addHeader("my-header", "lower") | ||
| .addHeader("My-Header", "camel") |
wendigo
force-pushed
the
findepi/fix-http-request-builder-setheader-case-sensitivity-be0601
branch
from
bd5f3d5
to
871266b
Compare
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public final class Request | ||
| { | ||
| private final URI uri; | ||
| private final String method; | ||
| private final ListMultimap<String, String> headers; | ||
| private final ListMultimap<String, String> headers = createHeadersMultimap(); |
There was a problem hiding this comment.
keep this field immutable.
the Request is built with builder, so the "case-insensitively duplicated" keys are already collapsed to one.
there is however a semantics difference in the getHeader(String) behavior. this should be test covered
There was a problem hiding this comment.
After reconsideration I think that we can actually lower-case headers everywhere in the airlift. According to the RFC 7540, Hypertext Transfer Protocol Version 2 (HTTP/2), Section 8.1.2:
Just as in HTTP/1.x, header field names are strings of ASCII characters that are compared in a case-insensitive fashion. However, header field names MUST be converted to lowercase prior to their encoding in HTTP/2. A request or response containing uppercase header field names MUST be treated as malformed
There was a problem hiding this comment.
Of course. Nothing that I didn't know. The desire to preserve case is not resulted of a whim, or lack of understanding how http works. i have no hard data to support that desire, it's more like an intuitions that something else can break.
There was a problem hiding this comment.
@findepi so let's fix the broken places instead
altered copy of #1165