Skip to content

Commit 36918b2

Browse files
committed
fix(http): don't reset shared req/resp while async response is in flight (#424)
HttpHandler::Reset() reuses the same HttpRequest/HttpResponse objects. On a keep-alive connection, if a new request arrives while an async handler on a worker thread is still producing/sending the previous response, FeedRecvData called Reset() unconditionally -- racing the worker's use of resp/writer and risking a crash (reported in #424; also reachable via HTTP pipelining or a malicious peer). Guard it: when a new request arrives and the handler isn't back at WANT_RECV, only Reset() if the response has already been handed off (writer->end == SEND_END). If the async response is still in flight, reject the early/pipelined data (ERR_REQUEST) so the connection is closed instead of corrupting the in-use objects. The completion signal is the writer's end state, not HttpHandler::state: an async writer writes straight to the socket and never advances the handler's send-state machine, so HttpHandler::state stays HANDLE_CONTINUE. Verified: sync and async keep-alive connection reuse both work (single connection, no spurious close); the pipeline-during-async case is rejected without crashing and the server keeps serving; make check passes (~658k keep-alive reqs OK). Alternative to #814 that keeps req/resp reuse instead of reallocating per request.
1 parent 1965221 commit 36918b2

1 file changed

Lines changed: 12 additions & 0 deletions

File tree

http/server/HttpHandler.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,18 @@ int HttpHandler::FeedRecvData(const char* data, size_t len) {
777777
case HttpHandler::HTTP_V1:
778778
case HttpHandler::HTTP_V2:
779779
if (state != WANT_RECV) {
780+
// A new request arrived on this connection before the previous one
781+
// finished. Reset() reuses the same req/resp objects, so if an async
782+
// handler on another thread is still producing/sending the previous
783+
// response (writer not yet End()ed), resetting here races that thread
784+
// and can crash. In that case reject the pipelined/early data and let
785+
// the caller close the connection. Otherwise (response already sent)
786+
// it is safe to reset for the next keep-alive request.
787+
if (writer && writer->end != hv::HttpResponseWriter::SEND_END) {
788+
hloge("[%s:%d] new request while previous async response is still in flight", ip, port);
789+
error = ERR_REQUEST;
790+
return -1;
791+
}
780792
Reset();
781793
}
782794
nfeed = parser->FeedRecvData(data, len);

0 commit comments

Comments
 (0)