Correctly manage Content-Length on HEAD responses (#2277) (#2289)

Motivation

When we receive a HEAD response, it's possible that the response
contains a content-length. llhttp has a bug
(nodejs/llhttp#202) that prevents it from
properly managing that issue, which causes us to incorrectly parse
responses.

Modifications

Forcibly set llhttp's content-length value to 0.

Result

Correctly handle HTTP framing around llhttp's issues.

(cherry picked from commit 5aa4498)

Sources/NIOHTTP1/HTTPDecoder.swift

@@ -113,6 +113,9 @@ private class BetterHTTPParser {
         }
         self.settings.pointee.on_message_complete = { opaque in
             BetterHTTPParser.fromOpaque(opaque).didReceiveMessageCompleteNotification()
+            // Temporary workaround for https://github.com/nodejs/llhttp/issues/202, should be removed
+            // when that issue is fixed. We're tracking the work in https://github.com/apple/swift-nio/issues/2274.
+            opaque?.pointee.content_length = 0
             return 0
         }
         self.withExclusiveHTTPParser { parserPtr in

Tests/NIOHTTP1Tests/HTTPDecoderTest+XCTest.swift

@@ -66,6 +66,8 @@ extension HTTPDecoderTest {
                 ("testDecodingInvalidTrailerFieldNames", testDecodingInvalidTrailerFieldNames),
                 ("testDecodingInvalidHeaderFieldValues", testDecodingInvalidHeaderFieldValues),
                 ("testDecodingInvalidTrailerFieldValues", testDecodingInvalidTrailerFieldValues),
+                ("testDecodeAfterHEADResponse", testDecodeAfterHEADResponse),
+                ("testDecodeAfterHEADResponseChunked", testDecodeAfterHEADResponseChunked),
            ]
    }
 }

Tests/NIOHTTP1Tests/HTTPDecoderTest.swift

@@ -1255,4 +1255,132 @@ class HTTPDecoderTest: XCTestCase {
             _ = try? channel.finish()
         }
     }
+
+    func testDecodeAfterHEADResponse() throws {
+        let channel = EmbeddedChannel()
+        try channel.pipeline.syncOperations.addHTTPClientHandlers()
+        defer {
+            _ = try? channel.finish()
+        }
+
+        let headRequest = HTTPRequestHead(version: .http1_1, method: .HEAD, uri: "/")
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.head(headRequest)))
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.end(nil)))
+
+        // Send a response.
+        let goodResponse = ByteBuffer(string: "HTTP/1.1 200 OK\r\nServer: foo\r\nContent-Length: 4\r\n\r\n")
+        XCTAssertNoThrow(try channel.writeInbound(goodResponse))
+
+        var maybeParsedHead: HTTPClientResponsePart?
+        var maybeEnd: HTTPClientResponsePart?
+
+        XCTAssertNoThrow(maybeParsedHead = try channel.readInbound())
+        XCTAssertNoThrow(maybeEnd = try channel.readInbound())
+        guard case .some(.head(let head)) = maybeParsedHead else {
+            XCTFail("Expected head, got \(String(describing: maybeParsedHead))")
+            return
+        }
+        guard case .some(.end(nil)) = maybeEnd else {
+            XCTFail("Expected end, got \(String(describing: maybeEnd))")
+            return
+        }
+        XCTAssertEqual(head.status, .ok)
+
+        // Send a GET request
+        let secondHeadRequest = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.head(secondHeadRequest)))
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.end(nil)))
+
+        // Send a response.
+        let goodResponseWithContent = ByteBuffer(string: "HTTP/1.1 200 OK\r\nServer: foo\r\nContent-Length: 4\r\n\r\nGood")
+        XCTAssertNoThrow(try channel.writeInbound(goodResponseWithContent))
+
+        var maybeBody: HTTPClientResponsePart?
+
+        XCTAssertNoThrow(maybeParsedHead = try channel.readInbound())
+        XCTAssertNoThrow(maybeBody = try channel.readInbound())
+        XCTAssertNoThrow(maybeEnd = try channel.readInbound())
+        guard case .some(.head(let secondHead)) = maybeParsedHead else {
+            XCTFail("Expected head, got \(String(describing: maybeParsedHead))")
+            return
+        }
+        guard case .some(.body(let body)) = maybeBody else {
+            XCTFail("Expected body, got \(String(describing: maybeBody))")
+            return
+        }
+        guard case .some(.end(nil)) = maybeEnd else {
+            XCTFail("Expected end, got \(String(describing: maybeEnd))")
+            return
+        }
+        XCTAssertEqual(secondHead.status, .ok)
+        XCTAssertEqual(body, ByteBuffer(string: "Good"))
+
+        // Should not throw.
+        channel.pipeline.fireChannelActive()
+        XCTAssertNoThrow(try channel.throwIfErrorCaught())
+    }
+
+    func testDecodeAfterHEADResponseChunked() throws {
+        let channel = EmbeddedChannel()
+        try channel.pipeline.syncOperations.addHTTPClientHandlers()
+        defer {
+            _ = try? channel.finish()
+        }
+
+        let headRequest = HTTPRequestHead(version: .http1_1, method: .HEAD, uri: "/")
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.head(headRequest)))
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.end(nil)))
+
+        // Send a response.
+        let goodResponse = ByteBuffer(string: "HTTP/1.1 200 OK\r\nServer: foo\r\nTransfer-Encoding: chunked\r\n\r\n")
+        XCTAssertNoThrow(try channel.writeInbound(goodResponse))
+
+        var maybeParsedHead: HTTPClientResponsePart?
+        var maybeEnd: HTTPClientResponsePart?
+
+        XCTAssertNoThrow(maybeParsedHead = try channel.readInbound())
+        XCTAssertNoThrow(maybeEnd = try channel.readInbound())
+        guard case .some(.head(let head)) = maybeParsedHead else {
+            XCTFail("Expected head, got \(String(describing: maybeParsedHead))")
+            return
+        }
+        guard case .some(.end(nil)) = maybeEnd else {
+            XCTFail("Expected end, got \(String(describing: maybeEnd))")
+            return
+        }
+        XCTAssertEqual(head.status, .ok)
+
+        // Send a GET request
+        let secondHeadRequest = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.head(secondHeadRequest)))
+        XCTAssertNoThrow(try channel.writeOutbound(HTTPClientRequestPart.end(nil)))
+
+        // Send a response.
+        let goodResponseWithContent = ByteBuffer(string: "HTTP/1.1 200 OK\r\nServer: foo\r\nContent-Length: 4\r\n\r\nGood")
+        XCTAssertNoThrow(try channel.writeInbound(goodResponseWithContent))
+
+        var maybeBody: HTTPClientResponsePart?
+
+        XCTAssertNoThrow(maybeParsedHead = try channel.readInbound())
+        XCTAssertNoThrow(maybeBody = try channel.readInbound())
+        XCTAssertNoThrow(maybeEnd = try channel.readInbound())
+        guard case .some(.head(let secondHead)) = maybeParsedHead else {
+            XCTFail("Expected head, got \(String(describing: maybeParsedHead))")
+            return
+        }
+        guard case .some(.body(let body)) = maybeBody else {
+            XCTFail("Expected body, got \(String(describing: maybeBody))")
+            return
+        }
+        guard case .some(.end(nil)) = maybeEnd else {
+            XCTFail("Expected end, got \(String(describing: maybeEnd))")
+            return
+        }
+        XCTAssertEqual(secondHead.status, .ok)
+        XCTAssertEqual(body, ByteBuffer(string: "Good"))
+
+        // Should not throw.
+        channel.pipeline.fireChannelActive()
+        XCTAssertNoThrow(try channel.throwIfErrorCaught())
+    }
 }