Add :downcase_request_headers option to HTTP1.connect #399
Conversation
|
Do you know if this is a behavior that we should have for HTTP/2 as well? |
I would say no until proven otherwise. AFAIK all HTTP/2 implementers properly handle header casing. |
|
I've added some commits which address the comments. The name of the option is currently Also, locally the twitter integration test is failing for me with a 302 instead of a 200. |
Pull Request Test Coverage Report for Build 17d2697acfe911fefda4291ee5a04f392057aa6b-PR-399Details
💛 - Coveralls |
DunyaKokoschka
force-pushed
the
preserve_header_case
branch
from
2f3f291
to
94c52ab
Compare
DunyaKokoschka
force-pushed
the
preserve_header_case
branch
from
94c52ab
to
33207b8
Compare
Also, rename name to canonical_name
ericmj
force-pushed
the
preserve_header_case
branch
from
33207b8
to
8fd0806
Compare
|
@whatyouhide I rebased and did some clean up. Do you think we are good to merge this? |
lib/mint/http1.ex
Outdated
|
|
||
| * `:case_sensitive_headers` - (boolean) if set to true the case of the supplied | ||
| headers in requests will be preserved. The default is to lowercase the headers | ||
| because http/1.1 header names are not case-insensitive. |
There was a problem hiding this comment.
| because http/1.1 header names are not case-insensitive. | |
| because HTTP/1.1 header names are case-sensitive. |
There was a problem hiding this comment.
Oh wait. This text is even wrong, we can lower because they are in fact case-insensitive.
lib/mint/core/headers.ex
Outdated
| {original_name :: String.t(), canonical_name :: String.t(), value :: String.t()} | ||
| @type raw_header() :: {original_name :: String.t(), value :: String.t()} | ||
|
|
||
| @unallowed_trailer_headers MapSet.new([ |
There was a problem hiding this comment.
I believe I mentioned it on some other occasion: "trailer headers" feels like a misnomer. Quickly looking at RFC 7231 the naming didn't seem to be super explicit, but in the newer one that surpasses it, RFC 9110, it is: http has fields: header fields (colloquially "headers") and trailer fields (colloquially "trailers")
I only mention this as an offer to send a patch to use "trailers" or "trailer fields" naming internally. It's probably not worth it to go through deprecations if the trailer_headers name was used in public APIs so I wouldn't touch these.
Feel free to ignore, it's not a big deal in the grand scheme of things. :)
ericmj
force-pushed
the
preserve_header_case
branch
from
ef3da3e
to
17d2697
Compare
ericmj
changed the title
i hear you want not lower header. i give you not lower header. but also i make default headers not lower if you pass
in not lower option. alexi say this change is slow. i tell alexi if you care about slow why using elixir.
#256