mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix: ensure TLS handshake callback fires before HTTP request handler (#25525)
## Summary Fixes a flaky test (`test-http-url.parse-https.request.js`) where `request.socket._secureEstablished` was intermittently `false` when the HTTP request handler was called on HTTPS servers. ## Root Cause The `isAuthorized` flag was stored in `HttpContextData::flags.isAuthorized`, which is **shared across all sockets** in the same context. This meant multiple concurrent TLS connections could overwrite each other's authorization state, and the value could be stale when read. ## Fix Moved the `isAuthorized` flag from the context-level `HttpContextData` to the per-socket `AsyncSocketData` base class. This ensures each socket has its own authorization state that is set correctly during its TLS handshake callback. ## Changes - **`AsyncSocketData.h`**: Added per-socket `bool isAuthorized` field - **`HttpContext.h`**: Updated handshake callback to set per-socket flag instead of context-level flag - **`JSNodeHTTPServerSocket.cpp`**: Updated `isAuthorized()` to read from per-socket `AsyncSocketData` (via `HttpResponseData` which inherits from it) ## Testing Ran the flaky test 50+ times with 100% pass rate. Also verified gRPC and HTTP2 tests still pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -84,6 +84,7 @@ struct AsyncSocketData {
|
||||
/* Or empty */
|
||||
AsyncSocketData() = default;
|
||||
bool isIdle = false;
|
||||
bool isAuthorized = false; // per-socket TLS authorization status
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
@@ -124,15 +124,16 @@ private:
|
||||
// if we are closing or already closed, we don't need to do anything
|
||||
if (!us_socket_is_closed(SSL, s)) {
|
||||
HttpContextData<SSL> *httpContextData = getSocketContextDataS(s);
|
||||
httpContextData->flags.isAuthorized = success;
|
||||
// Set per-socket authorization status
|
||||
auto *httpResponseData = reinterpret_cast<HttpResponseData<SSL> *>(us_socket_ext(SSL, s));
|
||||
if(httpContextData->flags.rejectUnauthorized) {
|
||||
if(!success || verify_error.error != 0) {
|
||||
// we failed to handshake, close the socket
|
||||
us_socket_close(SSL, s, 0, nullptr);
|
||||
return;
|
||||
}
|
||||
httpContextData->flags.isAuthorized = true;
|
||||
}
|
||||
httpResponseData->isAuthorized = success;
|
||||
|
||||
/* Any connected socket should timeout until it has a request */
|
||||
((HttpResponse<SSL> *) s)->resetTimeout();
|
||||
|
||||
@@ -72,13 +72,11 @@ bool JSNodeHTTPServerSocket::isAuthorized() const
|
||||
// is secure means that tls was established successfully
|
||||
if (!is_ssl || !socket)
|
||||
return false;
|
||||
auto* context = us_socket_context(is_ssl, socket);
|
||||
if (!context)
|
||||
// Read from per-socket HttpResponseData instead of context-level data
|
||||
auto* httpResponseData = reinterpret_cast<uWS::HttpResponseData<true>*>(us_socket_ext(is_ssl, socket));
|
||||
if (!httpResponseData)
|
||||
return false;
|
||||
auto* data = (uWS::HttpContextData<true>*)us_socket_context_ext(is_ssl, context);
|
||||
if (!data)
|
||||
return false;
|
||||
return data->flags.isAuthorized;
|
||||
return httpResponseData->isAuthorized;
|
||||
}
|
||||
|
||||
JSNodeHTTPServerSocket::~JSNodeHTTPServerSocket()
|
||||
|
||||
Reference in New Issue
Block a user