search

apple / swift-nio-http2

HTTP/2 support for SwiftNIO
https://swiftpackageindex.com/apple/swift-nio-http2/main/documentation/niohttp2
Apache License 2.0
462 stars 82 forks source link

Fix parent channel `read()` call on `HTTP2StreamChannel` initialization causing incorrect order of inbound `HTTP2Frame`s #413

Closed qusc closed 1 year ago

qusc commented 1 year ago

Motivation:

Make sure the order of HTTP2Frames 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.

Lukasa commented 1 year ago

@swift-server-bot test this please

Lukasa commented 1 year ago

@swift-server-bot test this please

Lukasa commented 1 year ago

Locally your test crashes for me: 

Test Case '-[NIOHTTP2Tests.HTTP2StreamMultiplexerTests testMultiplexerFiresInitialFramesInCorrectOrder]' started.
NIOCore/NIOAny.swift:200: Fatal error: tried to decode as type HTTP2Frame but found FramePayload with contents other(NIOHTTP2.HTTP2Frame.FramePayload.headers(NIOHTTP2.HTTP2Frame.FramePayload.Headers(headers: [], priorityData: nil, endStream: false, _paddingBytes: nil)))
qusc commented 1 year ago

Locally your test crashes for me:

Test Case '-[NIOHTTP2Tests.HTTP2StreamMultiplexerTests testMultiplexerFiresInitialFramesInCorrectOrder]' started.
NIOCore/NIOAny.swift:200: Fatal error: tried to decode as type HTTP2Frame but found FramePayload with contents other(NIOHTTP2.HTTP2Frame.FramePayload.headers(NIOHTTP2.HTTP2Frame.FramePayload.Headers(headers: [], priorityData: nil, endStream: false, _paddingBytes: nil)))

@Lukasa looking into this...

qusc commented 1 year ago

@Lukasa Apparently caused by the switch to the non-deprecated init of HTTP2StreamMultiplexer. That implicitly switches the elements being sent down the pipeline to HTTP2Frame.Payload to exclude the stream id? But the InboundOut type of HTTP2StreamMultiplexer is technically still HTTP2Frame? Edit: this is about the type of elements fired on the child channel, I see...

qusc commented 1 year ago

@swift-server-bot test this please if I'm allowed to trigger that

Lukasa commented 1 year ago

@swift-server-bot add to allowlist