Treat headers case insenitively, internally#190
Open
jhnstrk wants to merge 2 commits intoKludex:masterfrom
Open
Treat headers case insenitively, internally#190jhnstrk wants to merge 2 commits intoKludex:masterfrom
jhnstrk wants to merge 2 commits intoKludex:masterfrom
Conversation
Contributor
Author
|
Fixes #165 |
Kludex
reviewed
Dec 6, 2024
Comment on lines
+1656
to
+1658
| # Convert header name to title case. | ||
| header_name_tc = b"".join(header_name).decode().title() | ||
| headers[header_name_tc] = b"".join(header_value) |
Owner
There was a problem hiding this comment.
Shouldn't we actually lower case instead of title?
Contributor
Author
There was a problem hiding this comment.
Either would work, there's a trade off.
Lower case will require more of the code and tests to change as they currently expect title case. It also impacts the outside interface, where we say "The only required header is Content-Type." and don't say anything about the case-sensitivity of the input dict. I don't want to have to re-ingest the input header dict to ensure it's case insensitive. Switching to using only the content-type header value here would help, but entirely breaks the external API.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Converts header keys to title case before inserting into the header dict, so they are treated case-insensitively.
This seems like the easiest way, while preserving external compatibility since previous code was always assuming title case headers. However, it's not consistent with
email.Messagewhich converts to lower case.The input headers dict for
parse_formwas changed to allow any type supportingget- which allows for case-insensitive dicts. It seems overkill to implement another dict type or input when we only want to get one or two values from it.