[HTTPDecoder] Decode informational http response head correctly #1984
Conversation
fabianfett
force-pushed
the
ff-decode-informal-headers
branch
from
72a4a72
to
08a71c9
Compare
Sources/NIOHTTP1/HTTPDecoder.swift
Outdated
| } | ||
|
|
||
| /// Strategy to use when a HTTPDecoder receives an informal HTTP response (1xx except 101) | ||
| public enum InformalResponseStrategy { |
There was a problem hiding this comment.
Nit: these are not informal responses, they are informational responses. All the terminology in this PR should be changed appropriately.
There was a problem hiding this comment.
Similarly, we should probably use an extensible enum for our API design here (i.e. enum wrapped in struct with static lets.
Sources/NIOHTTP1/HTTPDecoder.swift
Outdated
| } else if statusCode / 100 == 1 || // 1XX codes | ||
| statusCode == 204 || statusCode == 304 { | ||
|
|
||
| if (HTTPResponseStatus.continue.code <= statusCode && statusCode < HTTPResponseStatus.ok.code) |
There was a problem hiding this comment.
I think it's fine to treat these codes as numbers here.
Sources/NIOHTTP1/HTTPDecoder.swift
Outdated
| private let kind: HTTPDecoderKind | ||
| private var stopParsing = false // set on upgrade or HTTP version error | ||
| private var lastHeaderWasInformal = false |
There was a problem hiding this comment.
We don't really mean "header" here, we mean "response".
Sources/NIOHTTP1/HTTPDecoder.swift
Outdated
| status: .init(statusCode: statusCode), | ||
| headers: HTTPHeaders(self.headers, | ||
| keepAliveState: keepAliveState)) | ||
| message = NIOAny(HTTPClientResponsePart.head(resHead)) |
There was a problem hiding this comment.
Can I propose we wrap the common code in this case and the case below into a single helper?
Sources/NIOHTTP1/HTTPDecoder.swift
Outdated
| public enum InformalResponseStrategy { | ||
| /// Drop the informal response and only forward the "real" response | ||
| case drop | ||
| /// Forward the informal response and then forward the "real" response |
There was a problem hiding this comment.
We should clarify what this looks like (namely, multiple .head messages for a single .end).
Co-authored-by: Cory Benfield <lukasa@apple.com>
fabianfett
force-pushed
the
ff-decode-informal-headers
branch
from
e74a0c9
to
6b36c6f
Compare
fabianfett
changed the title
We should support HTTP requests that are answered with an informational response first, which is followed up by the actual response. See issue: swift-server/async-http-client#461
Motivation:
Modifications:
InformalResponseStrategyenum that allows the user to specify whether http 1xx responses shall be forwarded or dropped.HTTPDecoderandBetterHTTPParserResult:
We will be able to fix swift-server/async-http-client#461