Fix parent channel read() call on HTTP2StreamChannel initializati…

…on causing incorrect order of inbound `HTTP2Frame`s (#413)

Motivation:

Make sure the order of `HTTP2Frame`s that are fired through the pipeline of a `HTTP2StreamChannel` by `HTTP2CommonInboundStreamMultiplexer` is correct when a `read()` call on the parent channel synchronously causes further frames to be processed.

Modifications:

Reorder calls in `HTTP2CommonInboundStreamMultiplexer.receivedFrame(frame:context:multiplexer)` so that initial header frame is buffered and processed first.

Result:

Resolves #410 and makes newly added test case `HTTP2StreamMultiplexerTests.testMultiplexerFiresInitialFramesInCorrectOrder()` pass.

Sources/NIOHTTP2/HTTP2CommonInboundStreamMultiplexer.swift

@@ -109,8 +109,11 @@ extension HTTP2CommonInboundStreamMultiplexer {
             // does no actual work.
             self.streamChannelContinuation?.yield(channel: channel.baseChannel)
 
-            channel.configureInboundStream(initializer: self.inboundStreamStateInitializer)
+            // Note: Firing the initial (header) frame before calling `HTTP2StreamChannel.configureInboundStream(initializer:)`
+            // is crucial to preserve frame order, since the initialization process might trigger another read on the parent
+            // channel which in turn might cause further frames to be processed synchronously.
             channel.receiveInboundFrame(frame)
+            channel.configureInboundStream(initializer: self.inboundStreamStateInitializer)
 
             if !channel.inList {
                 self.didReadChannels.append(channel)

Tests/NIOHTTP2Tests/HTTP2StreamMultiplexerTests.swift

@@ -1927,4 +1927,50 @@ final class HTTP2StreamMultiplexerTests: XCTestCase {
         XCTAssertNoThrow(try channel.finish(acceptAlreadyClosed: false))
     }
 
+    func testMultiplexerFiresInitialFramesInCorrectOrder() throws {
+        final class BufferingChannelHandler<Element>: ChannelDuplexHandler {
+            typealias InboundIn = Element
+            typealias InboundOut = Element
+            typealias OutboundIn = Element
+            typealias OutboundOut = Element
+
+            let elementsToBufferCount: Int = 1
+            var bufferedElements: CircularBuffer<Element> = []
+
+            func read(context: ChannelHandlerContext) {
+                while let bufferedElement = bufferedElements.popFirst() {
+                    context.fireChannelRead(wrapInboundOut(bufferedElement))
+                }
+
+                context.read()
+            }
+
+            func channelRead(context: ChannelHandlerContext, data: NIOAny) {
+                bufferedElements.append(unwrapInboundIn(data))
+
+                if bufferedElements.count > elementsToBufferCount {
+                    context.fireChannelRead(wrapInboundOut(bufferedElements.removeFirst()))
+                }
+            }
+        }
+
+        XCTAssertNoThrow(try self.channel.connect(to: SocketAddress(unixDomainSocketPath: "/whatever"), promise: nil))
+        let streamID = HTTP2StreamID(1)
+        let headerFrame = HTTP2Frame(streamID: streamID, payload: .headers(.init(headers: HPACKHeaders())))
+        let payloadBuffer = channel.allocator.buffer(capacity: 0)
+        let dataFrame = HTTP2Frame(streamID: streamID, payload: .data(.init(data: .byteBuffer(payloadBuffer))))
+
+        let multiplexer = HTTP2StreamMultiplexer(mode: .server, channel: self.channel) { channel in
+            channel.pipeline.addHandler(FramePayloadExpecter(expectedPayload: [headerFrame.payload, dataFrame.payload]))
+        }
+
+        XCTAssertNoThrow(try self.channel.pipeline.addHandler(BufferingChannelHandler<HTTP2Frame>()).wait())
+        XCTAssertNoThrow(try self.channel.pipeline.addHandler(multiplexer).wait())
+
+        XCTAssertNoThrow(try self.channel.writeInbound(headerFrame))
+        XCTAssertNoThrow(try self.channel.writeInbound(dataFrame))
+        self.activateStream(streamID)
+        (self.channel.eventLoop as! EmbeddedEventLoop).run()
+        XCTAssertNoThrow(try self.channel.finish())
+    }
 }