HTTPDecoder: don't deliver unsolicited responses

Sources/NIOHTTP1/HTTPDecoder.swift

@@ -265,15 +265,8 @@ private class BetterHTTPParser {
         }
         assert(self.firstNonDiscardableOffset == nil)
         self.decodingState = .headersComplete
-        let success = self.delegate.didFinishHead(versionMajor: versionMajor,
-                                                  versionMinor: versionMinor,
-                                                  isUpgrade: isUpgrade,
-                                                  method: method,
-                                                  statusCode: statusCode,
-                                                  keepAliveState: keepAliveState)
-        guard success else {
-            return .error(HPE_INVALID_VERSION)
-        }
+
+        var skipBody = false
 
         if self.kind == .response {
             // http_parser doesn't correctly handle responses to HEAD requests. We have to do something
@@ -296,24 +289,30 @@ private class BetterHTTPParser {
             // does not meet the requirement of RFC 7230. This is an outstanding http_parser issue:
             // https://github.com/nodejs/http-parser/issues/251. As a result, we check for these status
             // codes and override http_parser's handling as well.
-            if self.requestHeads.count > 0 {
-                let method = self.requestHeads.removeFirst().method
-                if method == .HEAD || method == .CONNECT {
-                    return .skipBody
-                }
-            } else {
+            guard let method = self.requestHeads.popFirst()?.method else {
                 self.richerError = NIOHTTPDecoderError.unsolicitedResponse
                 return .error(HPE_UNKNOWN)
             }
 
-            if (statusCode / 100 == 1 ||  // 1XX codes
-                statusCode == 204 ||
-                statusCode == 304) {
-                return .skipBody
+            if method == .HEAD || method == .CONNECT {
+                skipBody = true
+            } else if statusCode / 100 == 1 ||  // 1XX codes
+                statusCode == 204 || statusCode == 304 {
+                skipBody = true
             }
         }
 
-        return .normal
+        let success = self.delegate.didFinishHead(versionMajor: versionMajor,
+                                                  versionMinor: versionMinor,
+                                                  isUpgrade: isUpgrade,
+                                                  method: method,
+                                                  statusCode: statusCode,
+                                                  keepAliveState: keepAliveState)
+        guard success else {
+            return .error(HPE_INVALID_VERSION)
+        }
+
+        return skipBody ? .skipBody : .normal
     }
 
     func start() {

Tests/NIOHTTP1Tests/HTTPDecoderTest+XCTest.swift

@@ -52,6 +52,7 @@ extension HTTPDecoderTest {
                 ("testBytesCanBeDroppedWhenHandlerRemoved", testBytesCanBeDroppedWhenHandlerRemoved),
                 ("testAppropriateErrorWhenReceivingUnsolicitedResponse", testAppropriateErrorWhenReceivingUnsolicitedResponse),
                 ("testAppropriateErrorWhenReceivingUnsolicitedResponseDoesNotRecover", testAppropriateErrorWhenReceivingUnsolicitedResponseDoesNotRecover),
+                ("testOneRequestTwoResponses", testOneRequestTwoResponses),
            ]
    }
 }

Tests/NIOHTTP1Tests/HTTPDecoderTest.swift

@@ -785,4 +785,32 @@ class HTTPDecoderTest: XCTestCase {
         }
     }
 
+    func testOneRequestTwoResponses() {
+        let eventCounter = EventCounterHandler()
+        let responseDecoder = ByteToMessageHandler(HTTPResponseDecoder())
+        let channel = EmbeddedChannel(handler: responseDecoder)
+        XCTAssertNoThrow(try channel.pipeline.addHandler(eventCounter).wait())
+
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.head(.init(version: .init(major: 1, minor: 1),
+                                                                                    method: .GET, uri: "/"))))
+        var buffer = channel.allocator.buffer(capacity: 128)
+        buffer.writeString("HTTP/1.1 200 ok\r\ncontent-length: 0\r\n\r\nHTTP/1.1 200 ok\r\ncontent-length: 0\r\n\r\n")
+        XCTAssertThrowsError(try channel.writeInbound(buffer)) { error in
+            XCTAssertEqual(.unsolicitedResponse, error as? NIOHTTPDecoderError)
+        }
+        XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1),
+                                                    status: .ok,
+                                                    headers: ["content-length": "0"])),
+                                        try channel.readInbound(as: HTTPClientResponsePart.self)))
+        XCTAssertNoThrow(XCTAssertEqual(.end(nil),
+                                        try channel.readInbound(as: HTTPClientResponsePart.self)))
+        XCTAssertNoThrow(XCTAssertNil(try channel.readInbound(as: HTTPClientResponsePart.self)))
+        XCTAssertNoThrow(XCTAssertNotNil(try channel.readOutbound()))
+        XCTAssertEqual(1, eventCounter.writeCalls)
+        XCTAssertEqual(1, eventCounter.flushCalls)
+        XCTAssertEqual(2, eventCounter.channelReadCalls) // .head & .end
+        XCTAssertEqual(1, eventCounter.channelReadCompleteCalls)
+        XCTAssertEqual(["channelReadComplete", "write", "flush", "channelRead", "errorCaught"], eventCounter.allTriggeredEvents())
+        XCTAssertNoThrow(XCTAssertTrue(try channel.finish().isClean))
+    }
 }