urllib3 2.2 explicitly casts all headers to HTTPHeaderDict about urllib3 HOT 4 CLOSED

Adding some more context after further digging. The reason this isn't more widely impacting is this will only affect code subclassing HTTPConnection in urllib3. In urllib3<2.2, headers were always left in their provided representation and a copy was created.

botocore has a ~10 year old usage of HTTPConnection where request is overwritten to handle HTTP 100-continue responses. This was the earlier sticking point for upgrading botocore to urllib3 2.0.

With the recent change in 4ece59b, the type being passed to request is now different, leading to code accessing headers failing with the above error. This is avoided in the common case because urllib3's request implementation uses items() when iterating headers and forwards any bytes values to httplib's putheader for handling. items() appears to be one of the only interfaces on HTTPHeaderDict that behaves when bytes are provided.

While HTTPConnection isn't explicitly part of the public interface in 2.x, it appears to have been when botocore's code was written. Shazow created the separate connection.py module for this use case in #254 at the request of a similar use case in #253.

I don't know what the current maintenance team wants to do in this case or what the current expectations are around this behavior. I think the options I'm seeing currently are:

1. Revert the type change back to .copy() to preserve the previous functionality.
2. Correct the behavior in HTTPHeaderDict when bytes values are provided. I think it can be argued the current inconsistencies aren't expected and might be fixed/improved regardless.
3. Attempt to update the workflow in botocore to remove the usage of bytes. It's not immediately clear what the scope of impact would be there since there are other widely used libraries that have some reliance on the current behavior.

from urllib3.

There is already an existing ticket + PR to accept bytes values in the HTTPHeaderDict:

#3072
#3279

Would be interesting to see if that PR on its own would fix this issue.

from urllib3.

Thanks for pointing those out, @bblommers! Yeah, I agree those seem to be inline with this issue. Anecdotally from Requests, I'll say guessing with the encoding is usually the wrong decision (especially around latin-1). If we get bytes, it should probably keep going over the wire as it was received. Otherwise you end up with data corruption between latin-1 and utf-8.

It looks like Quentin already covered some of that in #3279. I agree that having mixed values for the same key would be an exception case (although perhaps it could be explicitly handled rather than failing on the join):

So my current thinking is that we should indeed join byte values using b", " and forbid mixed encoding and bytes (to be able to join). And not try to decode bytes to latin-1.

Edit: I can confirm #3279 does appear to resolve this issue when rebased onto main.

from urllib3.

Thanks for the investigation!

We need to review and merge #3279, but we should also consider if we really want to cast headers to HTTPHeaderDict.

from urllib3.