From 90b3558c5ba9f7565788a01bda811d847ed6d2d2 Mon Sep 17 00:00:00 2001 From: Jon Shier Date: Mon, 23 Oct 2023 15:49:33 -0400 Subject: [PATCH 1/4] Reenable parsing after failed upgrade. --- Sources/NIOHTTP1/HTTPDecoder.swift | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index b37c99a965..b12b95b9bb 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -473,7 +473,7 @@ public typealias HTTPResponseDecoder = HTTPDecoder: ByteToMessageDecoder, HTTPDecoderDelega case .response: self.context!.fireChannelRead(NIOAny(HTTPClientResponsePart.body(self.buffer!.readSlice(length: bytes.count)!))) } - } func didReceiveHeaderName(_ bytes: UnsafeRawBufferPointer) throws { From 161326a0038f8b314bfe5ba9059dea537987c15a Mon Sep 17 00:00:00 2001 From: Jon Shier Date: Tue, 24 Oct 2023 12:21:31 -0400 Subject: [PATCH 2/4] =?UTF-8?q?Check=20whether=20we=E2=80=99ve=20stopped?= =?UTF-8?q?=20parsing=20instead.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/NIOHTTP1/HTTPDecoder.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index b12b95b9bb..0d6135e2ae 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -493,7 +493,7 @@ extension HTTPDecoder: WriteObservingByteToMessageDecoder { self.parser.requestHeads.append(head) } else if Self.self == HTTPRequestDecoder.self, case let .head(head) = data as? HTTPServerResponsePart { - if head.isKeepAlive, self.isUpgrade == true, head.status != .switchingProtocols { + if head.isKeepAlive, head.status != .switchingProtocols, self.stopParsing { self.stopParsing = false } } From 09d9992c557cc0075ce15caeb52d749a6a40a92b Mon Sep 17 00:00:00 2001 From: Jon Shier Date: Thu, 26 Oct 2023 09:38:10 -0400 Subject: [PATCH 3/4] Check kind instead. --- Sources/NIOHTTP1/HTTPDecoder.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index 0d6135e2ae..2dfb5f4500 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -488,11 +488,11 @@ extension HTTPDecoder: WriteObservingByteToMessageDecoder { public typealias OutboundIn = Out public func write(data: Out) { - if Self.self == HTTPResponseDecoder.self, + if kind == .request, case .head(let head) = (data as? HTTPClientRequestPart) { self.parser.requestHeads.append(head) - } else if Self.self == HTTPRequestDecoder.self, - case let .head(head) = data as? HTTPServerResponsePart { + } else if kind == .response, + case let .head(head) = data as? HTTPServerResponsePart { if head.isKeepAlive, head.status != .switchingProtocols, self.stopParsing { self.stopParsing = false } From b45e567b00a309fba1a8f77c1fd41e9464162ed5 Mon Sep 17 00:00:00 2001 From: Jon Shier Date: Fri, 27 Oct 2023 13:19:05 -0400 Subject: [PATCH 4/4] Add tests. --- Sources/NIOHTTP1/HTTPDecoder.swift | 6 +-- Tests/NIOHTTP1Tests/HTTPDecoderTest.swift | 59 +++++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index 2dfb5f4500..87a6f22625 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -488,11 +488,9 @@ extension HTTPDecoder: WriteObservingByteToMessageDecoder { public typealias OutboundIn = Out public func write(data: Out) { - if kind == .request, - case .head(let head) = (data as? HTTPClientRequestPart) { + if kind == .response, case .head(let head) = data as? HTTPClientRequestPart { self.parser.requestHeads.append(head) - } else if kind == .response, - case let .head(head) = data as? HTTPServerResponsePart { + } else if kind == .request, case let .head(head) = data as? HTTPServerResponsePart { if head.isKeepAlive, head.status != .switchingProtocols, self.stopParsing { self.stopParsing = false } diff --git a/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift b/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift index 326c741482..1e827aae90 100644 --- a/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPDecoderTest.swift @@ -500,6 +500,65 @@ class HTTPDecoderTest: XCTestCase { status: .ok)), try channel.readInbound())) } + func testHTTPRequestParsingReenabledAfterFailedUpgrade() { + let channel = EmbeddedChannel() + defer { + XCTAssertNoThrow(try channel.finish()) + } + var inBuffer = channel.allocator.buffer(capacity: 128) + inBuffer.writeStaticString("GET / HTTP/1.1\r\nHost: localhost\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n") + + XCTAssertNoThrow(try channel.pipeline.addHandler(ByteToMessageHandler(HTTPRequestDecoder(leftOverBytesStrategy: .fireError))).wait()) + // Server receives an upgrade request. + XCTAssertNoThrow(try channel.writeInbound(inBuffer)) + XCTAssertNoThrow(XCTAssertEqual(HTTPServerRequestPart.head(.init(version: .http1_1, + method: .GET, uri: "/", + headers: .init([("Host", "localhost"), + ("Connection", "Upgrade"), + ("Upgrade", "websocket")]))), + try channel.readInbound())) + XCTAssertNoThrow(XCTAssertEqual(HTTPServerRequestPart.end(nil), try channel.readInbound())) + // Server sees a non upgrade response come through. + XCTAssertNoThrow(try channel.writeOutbound(HTTPServerResponsePart.head(.init(version: .http1_1, status: .internalServerError)))) + inBuffer.clear() + // Server receives another request. + inBuffer.writeStaticString("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n") + XCTAssertNoThrow(try channel.writeInbound(inBuffer)) + // Server should properly parse that request. + XCTAssertNoThrow(XCTAssertEqual(HTTPServerRequestPart.head(.init(version: .http1_1, + method: .GET, uri: "/", + headers: .init([("Host", "localhost")]))), + try channel.readInbound())) + } + + func testHTTPRequestParsingStopsAfterSuccessfulUpgrade() { + let channel = EmbeddedChannel() + defer { + XCTAssertNoThrow(try channel.finish()) + } + var inBuffer = channel.allocator.buffer(capacity: 128) + inBuffer.writeStaticString("GET / HTTP/1.1\r\nHost: localhost\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n") + + XCTAssertNoThrow(try channel.pipeline.addHandler(ByteToMessageHandler(HTTPRequestDecoder(leftOverBytesStrategy: .fireError))).wait()) + // Server receives an upgrade request. + XCTAssertNoThrow(try channel.writeInbound(inBuffer)) + XCTAssertNoThrow(XCTAssertEqual(HTTPServerRequestPart.head(.init(version: .http1_1, + method: .GET, uri: "/", + headers: .init([("Host", "localhost"), + ("Connection", "Upgrade"), + ("Upgrade", "websocket")]))), + try channel.readInbound())) + XCTAssertNoThrow(XCTAssertEqual(HTTPServerRequestPart.end(nil), try channel.readInbound())) + // Server sees successful upgrade response come through. + XCTAssertNoThrow(try channel.writeOutbound(HTTPServerResponsePart.head(.init(version: .http1_1, status: .switchingProtocols)))) + inBuffer.clear() + // Server receives another request. + inBuffer.writeStaticString("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n") + XCTAssertNoThrow(try channel.writeInbound(inBuffer)) + // Server should not parse that request. + XCTAssertNoThrow(XCTAssertEqual(nil, try channel.readInbound(as: HTTPServerRequestPart.self))) + } + func testBasicVerifications() { let byteBufferContainingJustAnX = ByteBuffer(string: "X") let expectedInOuts: [(String, [HTTPServerRequestPart])] = [