Skip to content

Commit fe7c01a

Browse files
Fix cork() closing a WebSocket that was upgraded asynchronously (#1929)
cork() detected an in-handler upgrade with `this != newCorkedSocket`, but us_socket_context_adopt_socket() reallocs the poll and returns the same pointer when the WebSocket extension is not larger than the HttpResponse extension. The upgrade then went undetected and cork() ran its HTTP connection-close logic on the now-WebSocket socket, closing it right after open. Also compare the socket context, which always changes on upgrade.
1 parent 8a1f360 commit fe7c01a

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

src/HttpResponse.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,9 @@ struct HttpResponse : public AsyncSocket<SSL> {
532532
HttpResponse *cork(MoveOnlyFunction<void()> &&handler) {
533533
if (!Super::isCorked() && Super::canCork()) {
534534
LoopData *loopData = Super::getLoopData();
535+
/* Remember our socket context so we can detect a WebSocket upgrade in the
536+
* handler even when the poll realloc kept our address (see below). */
537+
struct us_socket_context_t *preCorkContext = us_socket_context(SSL, (struct us_socket_t *) this);
535538
Super::cork();
536539
handler();
537540

@@ -552,7 +555,11 @@ struct HttpResponse : public AsyncSocket<SSL> {
552555

553556
/* If we are no longer an HTTP socket then early return the new "this".
554557
* We don't want to even overwrite timeout as it is set in upgrade already. */
555-
if (this != newCorkedSocket) {
558+
/* The pointer check alone is not enough: us_socket_context_adopt_socket() can
559+
* realloc the poll in place, leaving the upgraded WebSocket at our old address
560+
* (this == newCorkedSocket). The socket context always changes on upgrade. */
561+
if (this != newCorkedSocket ||
562+
us_socket_context(SSL, (struct us_socket_t *) newCorkedSocket) != preCorkContext) {
556563
return static_cast<HttpResponse *>(newCorkedSocket);
557564
}
558565

0 commit comments

Comments
 (0)