apple / swift-nio

Event-driven network application framework for high performance protocol servers & clients, non-blocking.
https://swiftpackageindex.com/apple/swift-nio/documentation
Apache License 2.0
7.98k stars 652 forks source link

Failed WebSocket upgrade does not consume initial request `.end` part #2632

Open adam-fowler opened 9 months ago

adam-fowler commented 9 months ago

Expected behavior

When a HTTP channel is configured with a NIOTypedWebSocketServerUpgrader WebSocket upgrade and a connection fails because shouldUpgrade returned nil the upgrade process should consume all parts of the initial request.

Actual behavior

The .end part is passed onto the HTTP channel pipeline, confusing HTTP request processing as the first part it receives is not a .head.

Steps to reproduce

This can be easily reproduced using the WebSocket samples that come with SwiftNIO

  1. Edit the shouldUpgrade closure of NIOTypedWebSocketServerUpgrader in NIOWebSocketServer/Server.swift:82 to be
    channel.eventLoop.makeSucceededFuture(nil)`.
  2. Run NIOWebSocketServer sample
  3. Add break point on line NIOWebSocketServer/Server.swift:214 to catch HTTP part.
    for try await requestPart in inbound {
  4. Run NIOWebSocketClient sample
  5. When you hit the breakpoint, check contents of requestPart.

SwiftNIO version/commit hash

Swift version 5.10-dev (LLVM e8e5be8d5b39f46, Swift 99e9db868aefd99) Target: x86_64-unknown-linux-gnu

adam-fowler commented 9 months ago

Should the server be closing the channel at this point?

FranzBusch commented 9 months ago

I will take a look. Thanks for filing this!

adam-fowler commented 7 months ago

Additional information. If your shouldUpgrade function doesn't complete immediately then the .end is consumed prior to the NIOAsyncChannel is added to the pipeline. And your example websocket server channel will hang waiting for a HTTP part that never appears.