diff --git a/doc/api/http.md b/doc/api/http.md index 6f6adecacf43..b65974493f65 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1631,7 +1631,8 @@ added: v0.7.0 Emitted each time a client requests an HTTP `CONNECT` method. If this event is not listened for, then clients requesting a `CONNECT` method will have their -connections closed. +connections closed. If an earlier response is still pending on the same +connection, the connection is closed and this event is not emitted. This event is guaranteed to be passed an instance of the {net.Socket} class, a subclass of {stream.Duplex}, unless the user specifies a socket @@ -1738,7 +1739,9 @@ change. If an upgrade is accepted by `shouldUpgradeCallback` but no event handler is registered then the socket will be destroyed, resulting in an immediate -connection closure for the client. +connection closure for the client. If an earlier response is still pending on +the same connection, the connection is also closed and this event is not +emitted. In the uncommon case that the incoming request has a body, this body will be parsed as normal, separate to the upgrade stream, and the raw stream data will diff --git a/lib/_http_server.js b/lib/_http_server.js index 43af4fff0a4a..6740ab15917a 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -1144,6 +1144,13 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { debug('SERVER upgrade or connect', req.method); const eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; + // Switching protocols while an earlier response is still pending would + // allow the upgrade response to be written out of order. + if (socket._httpMessage || state.outgoing.length > 0) { + socket.destroy(); + return; + } + let upgradeStream; if (req.complete) { d ||= parser.getCurrentBuffer(); diff --git a/test/parallel/test-http-pipeline-upgrade-response-order.mjs b/test/parallel/test-http-pipeline-upgrade-response-order.mjs new file mode 100644 index 000000000000..1f7115bc6005 --- /dev/null +++ b/test/parallel/test-http-pipeline-upgrade-response-order.mjs @@ -0,0 +1,97 @@ +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import { once } from 'node:events'; +import http from 'node:http'; +import net from 'node:net'; + +const CRLF = '\r\n'; +const firstRequest = `GET /first HTTP/1.1${CRLF}Host: localhost${CRLF}${CRLF}`; + +function switchRequest(eventName) { + if (eventName === 'connect') { + return `CONNECT localhost:443 HTTP/1.1${CRLF}` + + `Host: localhost:443${CRLF}${CRLF}`; + } + + return `GET /second HTTP/1.1${CRLF}` + + `Host: localhost${CRLF}` + + `Connection: Upgrade${CRLF}` + + `Upgrade: websocket${CRLF}${CRLF}`; +} + +function switchResponse(eventName) { + if (eventName === 'connect') { + return `HTTP/1.1 200 Connection Established${CRLF}${CRLF}SWITCHED`; + } + + return `HTTP/1.1 101 Switching Protocols${CRLF}` + + `Connection: Upgrade${CRLF}` + + `Upgrade: websocket${CRLF}${CRLF}SWITCHED`; +} + +async function listen(server) { + server.listen(0); + await once(server, 'listening'); +} + +async function close(server) { + await new Promise((resolve) => server.close(resolve)); +} + +async function testPipelinedSwitchIsRejected(eventName) { + const server = http.createServer(common.mustCall((req) => { + assert.strictEqual(req.url, '/first'); + })); + + server.on(eventName, common.mustNotCall()); + await listen(server); + + const client = net.connect(server.address().port); + let received = ''; + client.setEncoding('latin1'); + client.on('data', (data) => received += data); + client.on('error', () => {}); + const closed = new Promise((resolve) => client.once('close', resolve)); + client.end(firstRequest + switchRequest(eventName)); + + await closed; + assert.strictEqual(received, ''); + await close(server); +} + +async function testSwitchAfterResponseIsAccepted(eventName) { + const responseFinished = Promise.withResolvers(); + const server = http.createServer(common.mustCall((req, res) => { + assert.strictEqual(req.url, '/first'); + res.on('finish', responseFinished.resolve); + res.end('FIRST'); + })); + + server.on(eventName, common.mustCall((req, socket) => { + socket.end(switchResponse(eventName)); + })); + await listen(server); + + const client = net.connect(server.address().port); + let received = ''; + client.setEncoding('latin1'); + client.on('data', (data) => received += data); + client.on('error', () => {}); + client.write(firstRequest); + + await responseFinished.promise; + client.end(switchRequest(eventName)); + await once(client, 'close'); + + const firstIndex = received.indexOf('FIRST'); + const switchedIndex = received.indexOf('SWITCHED'); + assert.notStrictEqual(firstIndex, -1); + assert.notStrictEqual(switchedIndex, -1); + assert.ok(firstIndex < switchedIndex); + await close(server); +} + +for (const eventName of ['upgrade', 'connect']) { + await testPipelinedSwitchIsRejected(eventName); + await testSwitchAfterResponseIsAccepted(eventName); +}