Separate headers by CR LF#471
Conversation
It's literally not an HTTP message, which is clearly a mistake for a library with HTTP in its name. ;)
Allowing LF is completely optional, and can lead to security issues, particularly on the server side:
|
|
|
||
| for item in string_list: | ||
| fk.write(utf8(item) + b'\n') | ||
| fk.write(utf8(item) + b'\r\n') |
There was a problem hiding this comment.
Also, UTF-8 is not valid in headers:
Field values are usually constrained to the range of US-ASCII characters [USASCII]. Fields needing a greater range of characters can use an encoding, such as the one defined in [RFC8187].
https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-4
A recipient MUST parse an HTTP message as a sequence of octets in an encoding that is a superset of US-ASCII [USASCII]. Parsing an HTTP message as a stream of Unicode characters, without regard for the specific encoding, creates security vulnerabilities due to the varying ways that string processing libraries handle invalid multibyte character sequences that contain the octet LF (%x0A).
https://www.rfc-editor.org/rfc/rfc9112.html#section-2.2-2
2 participants
While using HTTPretty together with the last version of
aiohttp,aiohttpfailed to parse the response generated byHTTPrettydue to the way headers are separated.Specifically,
HTTPrettyseparates them via the LF (\n) control code whileaiohttpexpects CR LF (\r\n).This behavior (on
aiohttp's side) is also mentioned here aio-libs/aiohttp#7494 (comment) .By searching a bit on the Internet (eg. here) it seems that http parsers are expected to be lenient wrt parsing newlines, so I'm not sure this is technically a fault on
HTTPretty's side.However I think it could be better to stay on the safe side and just use
\r\nas separator for headers, which is the change I implemented with this PR.