Downcase headers when inserting/fetching in HPACK

[whatyouhide]

Before this commit, we would insert headers in the HPACK table without downcasing their name, and we would look up headers without downcasing their name either. With this commit, we downcase header names (since they're case insensitive) before inserting/looking up in the HPACK table.

Closes #171.

@ericmj not sure if there's a better or more efficient way of doing this. We might downcase at the boundary when headers are given to Mint but it's pretty much the same thing I think.

[ericmj]

not sure if there's a better or more efficient way of doing this.

The more efficient way would be to only char + 32 since headers are ASCII only.

We might downcase at the boundary when headers are given to Mint but it's pretty much the same thing I think.

Another option would be to validate and return an error, not sure what is preferred. I do think we should do it at the boundary and not in HPACK.

[ericmj]

We can do the same we do for HTTP/1 https://github.com/ericmj/mint/blob/master/lib/mint/http1/request.ex#L33.

[whatyouhide]

I pushed an update that uses the lowercasing we had for HTTP/1. However, not sure we should do the downcasing at the boundary since I think HPACK should work in a semantically correct way so that if you insert my-header in the HPACK table and try to fetch My-Header it still works correctly, no?

[ericmj]

However, not sure we should do the downcasing at the boundary since I think HPACK should work in a semantically correct way so that if you insert my-header in the HPACK table and try to fetch My-Header it still works correctly, no?

I am not sure HPACK requires lower case header keys. I could only find it in the HTTP/2 spec.

[whatyouhide]

@ericmj updated the PR, we now downcase header names in HTTP/2 and HTTP/1 and I added a comment in the HPACK implementation that headers should get to HPACK already downcased.

[chulkilee]

Please note that there are services (incorrectly) using case-sensitive headers. Yes, I know they're bad, but they exist :(

I'm not familiar with HPACK.. but shouldn't we delegate the responsibility of "using same case" when putting and retrieving headers to callers, not downcasing blindly? Or at least provide option to use raw value. Or use it only for HTTP2.

[ericmj]

I would rather do that when we have an example of a service that does this. I think it's better to follow the spec until we find a counter-example. Plug does the same thing but we haven't found any clients that require case-sensitive headers yet.

[chulkilee]

Spec says client/server should treat them case insensitive, not client/server should down case them.

(Correction: case sensitive -> case insensitive)

Bad server application implementation exists - and I did have that issue once.

Plug is different - it's server side so users decide how to consume it. On the contrary, if server behaves bad ( expecting specific case), I cannot use any library mutating outgoing http header.

(Note: I think it's reasonable to downcase response headers - since it's less likely to have a webservice returns same header in different cases to differentiate them)

I think it's fine to apply some changes on response, but outgoing changes should be more carefully decided.

.... or let's just document it and keep going! I'm fine with that - but I really wish any such built-in transformation must be explicitly mentioned :)

[ericmj]

Spec says client/server should treat them case sensitive, not client/server should down case them.

Can you link to the spec sections that say that header keys should be treated case sensitive?

Bad server application implementation exists - and I did have that issue once.

That does not mean a new client in 2019 should still allow for bad implementations. Like I said before if you can find examples in the wild that does not follow the spec then we can discuss around those, but right now it's hypothetical.

.... or let's just document it and keep going! I'm fine with that - but I really wish any such built-in transformation must be explicitly mentioned :)

I agree, could you send a PR that mentions this in the documentation?

[dowusu]

I ran into a strange issue today when connecting to an upstream server with Mojito - there’s a custom header which is being converted to lower case, however the upstream server expects the exact casing. Though I think this is a bad design, is there an option to force Mojito or Mint to maintain your case without converting them. Counting on your usual support.

[ericmj]

What is the header and what upstream server is expecting in a specific casing?

[dowusu]

The upstream server has a custom header field which is set with {"Msisdn", "#{msisdn}"} in Mojito, however the server is reading a case sensitive header Msisdn, my request is failing due to the lower case conversion by Mojito.
It's an Ericsson transactional platform, there's also a Huawei endpoint with similar behavior.
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<tns:errorResponse xmlns:tns=\"http://www.ericsson.com/lwac\" errorcode=\"MANDATORY_FIELD_MISSING\"><arguments name=\"Parameter_name\" value=\"Msisdn\"/></tns:errorResponse >"

I tried using php curl and it works since curl/libcurl is not down casing the header. I would like to stick entirely with Elixir.

A quick update - I used hackney for the same call and it works.

[ericmj]

It is hard for mint to support this because we support both HTTP/1.1 and HTTP/2 with automatic version negation and the user not having to know which version is in use. HTTP/2 requires that headers are downcased, if we have different behavior for HTTP/1.1 and HTTP/2 it will be confusing for users.

[dowusu]

Thanks for the swift response. Kudos for the awesome work done with Mint. It's my go to http client, will fallback on another library for a case like the above.

[whatyouhide]

I just want to make sure it's noted that they're the ones breaking the spec 😄 You might consider bringing it up with them and see if they're interested in a fix :)