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.92k stars 644 forks source link

NIOHTTP1Server dynamic handlers are sticky. #450

Closed azav closed 6 years ago

azav commented 6 years ago

Expected behavior

After starting NIOHTTP1Server from the console, returned results from the published end points often return data for another end point.

Actual behavior

Return expected values.

Steps to reproduce

  1. Connect to the proper directory and run NIOHTTP1Server from the terminal using this swift run NIOHTTP1Server localhost 9990

  2. In Safari, visit the endpoints listed below http://localhost:9990 http://localhost:9990/dynamic/pid http://localhost:9990/dynamic/client-ip

  3. Now enter any of the first two end points.

Results

The expected data is not returned. All end points now return only the last valid value. Stopping and starting the server doesn't fix this. The terminal window must be closed and the server rerun to fix the issue.

SwiftNIO version/commit hash

78235f841c725a8879089bf7ab0f6f0cecff49d0

[the SwiftNIO tag/commit hash] HEAD 78235f841c725a8879089bf7ab0f6f0cecff49d0

Swift & OS version (output of swift --version && uname -a)

Apple Swift version 4.0.3 (swiftlang-900.0.74.1 clang-900.0.39.2) Target: x86_64-apple-macosx10.9 Darwin 27-iMac.local 17.5.0 Darwin Kernel Version 17.5.0: Mon Mar 5 22:24:32 PST 2018; root:xnu-4570.51.1~1/RELEASE_X86_64 x86_64

Lukasa commented 6 years ago

Yeah, this is an issue with NIOHTTP1Server that I've known for a long time.

The problem is with this code:

https://github.com/apple/swift-nio/blob/25c6cd5982ae0d83092b2bb3a6e160bdc450c72e/Sources/NIOHTTP1Server/main.swift#L396-L405

Specifically, this code will attach a dynamic handler to handle a request, and once such a handler is attached it will keep passing request data to that handler until the handler is removed. However, none of the dynamic handlers actually remove themselves. That means if the connection is re-used, you'll get the previous dynamic handler in play. This can be easily triggered making the same sequence of calls with curl (here the PID is 12805):

curl http://localhost:8888/ http://localhost:8888/dynamic/pid http://localhost:8888/dynamic/client-ip
Hello World!1280512805

A simple-enough fix would probably be to enhance completeResponse to remove the current handler, if there is one.