Forbid HTTP protocols other than 1.#283
Conversation
weissi
left a comment
weissi
left a comment
There was a problem hiding this comment.
that looks great, one minor issue
| var readerIndexAdjustment = 0 | ||
| // This is set before http_parser_execute(...) is called and set to nil again after it finish | ||
| var baseAddress: UnsafePointer<UInt8>? | ||
| var currentError: Error? |
There was a problem hiding this comment.
you can save an existential here if you make the type HTTPParserError. That probably also save an allocation, maybe ;)
There was a problem hiding this comment.
actually, only potentially saves an allocation on assignment (which doesn't matter as this never happens). But definitely saves a couple of words in this structure if we don't make that an existential type
| XCTAssertNoThrow(try channel.pipeline.add(handler: HTTPRequestDecoder()).wait()) | ||
|
|
||
| // We tolerate higher versions of HTTP/1 than we know about because RFC 7230 | ||
| // says that these should be treated like HTTP/1.1 by our users. |
There was a problem hiding this comment.
I mean, the right thing for our users to do is to send HTTP/1.1 responses in response to HTTP/1.2 requests. But we can't really enforce that sensibly.
There was a problem hiding this comment.
Anyway it's largely theoretical as HTTP/1.2 is never gonna happen.
Motivation: Our HTTP code handles only HTTP/1.X. There is no reason to support HTTP/0.9, and we cannot safely handle a major protocol higher than 1 in this code, so we should simply treat requests/responses claiming to be of those protocols as errors. Modifications: HTTPDecoder now checks the major version is equal to 1 before it continues with parsing. If it hits an error, that error will be propagated out to the user. Result: Better resilience against bad HTTP messages.
Lukasa
force-pushed
the
cb-refuse-http-versions-other-than-1
branch
from
e4d1e08 to
937c458
Compare
|
@swift-nio-bot test this please |
|
Gah, this hit |
|
@swift-nio-bot test this please |
4 participants
Motivation:
Our HTTP code handles only HTTP/1.X. There is no reason to support
HTTP/0.9, and we cannot safely handle a major protocol higher than 1 in
this code, so we should simply treat requests/responses claiming to be
of those protocols as errors.
Modifications:
HTTPDecoder now checks the major version is equal to 1 before it
continues with parsing. If it hits an error, that error will be propagated
out to the user.
Result:
Better resilience against bad HTTP messages.
Resolves #281