diff --git a/doc/api/http.md b/doc/api/http.md index e7d4d3b4ac9604..700dce3b94f660 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -471,8 +471,9 @@ added: v0.1.94 * `head` {Buffer} Emitted each time a server responds to a request with an upgrade. If this -event is not being listened for, clients receiving an upgrade header will have -their connections closed. +event is not being listened for and the response status code is 101 Switching +Protocols, clients receiving an upgrade header will have their connections +closed. A client server pair demonstrating how to listen for the `'upgrade'` event. @@ -886,6 +887,11 @@ per connection (in the case of HTTP Keep-Alive connections). ### Event: 'upgrade' * `request` {http.IncomingMessage} Arguments for the HTTP request, as it is in @@ -893,9 +899,8 @@ added: v0.1.94 * `socket` {net.Socket} Network socket between the server and client * `head` {Buffer} The first packet of the upgraded stream (may be empty) -Emitted each time a client requests an HTTP upgrade. If this event is not -listened for, then clients requesting an upgrade will have their connections -closed. +Emitted each time a client requests an HTTP upgrade. Listening to this event +is optional and clients cannot insist on a protocol change. After this event is emitted, the request's socket will not have a `'data'` event listener, meaning it will need to be bound in order to handle data diff --git a/lib/_http_client.js b/lib/_http_client.js index 5be1632fe59433..c3ef9de20498df 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -426,7 +426,7 @@ function socketOnData(d) { req.socket._hadError = true; req.emit('error', ret); } else if (parser.incoming && parser.incoming.upgrade) { - // Upgrade or CONNECT + // Upgrade (if status code 101) or CONNECT var bytesParsed = ret; var res = parser.incoming; req.res = res; @@ -453,7 +453,7 @@ function socketOnData(d) { req.emit(eventName, res, socket, bodyHead); req.emit('close'); } else { - // Got Upgrade header or CONNECT method, but have no handler. + // Requested Upgrade or used CONNECT method, but have no handler. socket.destroy(); } } else if (parser.incoming && parser.incoming.complete && @@ -492,6 +492,10 @@ function parserOnIncomingClient(res, shouldKeepAlive) { } req.res = res; + // Skip body and treat as Upgrade. + if (res.upgrade) + return 2; + // Responses to CONNECT request is handled as Upgrade. const method = req.method; if (method === 'CONNECT') { diff --git a/lib/_http_common.js b/lib/_http_common.js index b101c11911fa1e..bc65443eb3c845 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -83,6 +83,7 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, parser.incoming.httpVersionMinor = versionMinor; parser.incoming.httpVersion = `${versionMajor}.${versionMinor}`; parser.incoming.url = url; + parser.incoming.upgrade = upgrade; var n = headers.length; @@ -101,19 +102,6 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, parser.incoming.statusMessage = statusMessage; } - if (upgrade && parser.outgoing !== null && !parser.outgoing.upgrading) { - // The client made non-upgrade request, and server is just advertising - // supported protocols. - // - // See RFC7230 Section 6.7 - upgrade = false; - } - - parser.incoming.upgrade = upgrade; - - if (upgrade) - return 2; // Skip body and treat as Upgrade. - return parser.onIncoming(parser.incoming, shouldKeepAlive); } diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index adc4481347f71a..b62dea4a24dd2f 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -98,7 +98,6 @@ function OutgoingMessage() { this.writable = true; this._last = false; - this.upgrading = false; this.chunkedEncoding = false; this.shouldKeepAlive = true; this.useChunkedEncodingByDefault = true; @@ -304,7 +303,6 @@ function _storeHeader(firstLine, headers) { // in the case of response it is: 'HTTP/1.1 200 OK\r\n' var state = { connection: false, - connUpgrade: false, contLen: false, te: false, date: false, @@ -366,10 +364,6 @@ function _storeHeader(firstLine, headers) { } } - // Are we upgrading the connection? - if (state.connUpgrade && state.upgrade) - this.upgrading = true; - // Date header if (this.sendDate && !state.date) { state.header += 'Date: ' + utcDate() + CRLF; @@ -460,8 +454,6 @@ function matchConnValue(self, state, value) { while (m) { if (m[0].length === 5) sawClose = true; - else - state.connUpgrade = true; m = RE_CONN_VALUES.exec(value); } if (sawClose) diff --git a/lib/_http_server.js b/lib/_http_server.js index cba70bd896058c..a8389dbda62f0d 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -530,14 +530,14 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { parser = null; var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; - if (server.listenerCount(eventName) > 0) { + if (eventName === 'upgrade' || server.listenerCount(eventName) > 0) { debug('SERVER have listener for %s', eventName); var bodyHead = d.slice(bytesParsed, d.length); socket.readableFlowing = null; server.emit(eventName, req, socket, bodyHead); } else { - // Got upgrade header or CONNECT method, but have no handler. + // Got CONNECT method, but have no handler. socket.destroy(); } } @@ -592,6 +592,13 @@ function resOnFinish(req, res, socket, state, server) { function parserOnIncoming(server, socket, state, req, keepAlive) { resetSocketTimeout(server, socket, state); + if (req.upgrade) { + req.upgrade = req.method === 'CONNECT' || + server.listenerCount('upgrade') > 0; + if (req.upgrade) + return 2; + } + state.incoming.push(req); // If the writable end isn't consuming, then stop reading diff --git a/test/parallel/test-http-upgrade-advertise.js b/test/parallel/test-http-upgrade-advertise.js index 99a3e8fd353015..6d033ba36a5d60 100644 --- a/test/parallel/test-http-upgrade-advertise.js +++ b/test/parallel/test-http-upgrade-advertise.js @@ -8,7 +8,9 @@ const tests = [ { headers: {}, expected: 'regular' }, { headers: { upgrade: 'h2c' }, expected: 'regular' }, { headers: { connection: 'upgrade' }, expected: 'regular' }, - { headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'upgrade' } + { headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'upgrade' }, + { headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'destroy' }, + { headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'regular' }, ]; function fire() { @@ -32,10 +34,17 @@ function fire() { done('regular'); }); - req.on('upgrade', function onUpgrade(res, socket) { - socket.destroy(); - done('upgrade'); - }); + if (test.expected === 'destroy') { + req.on('socket', () => req.socket.on('close', () => { + server.removeAllListeners('upgrade'); + done('destroy'); + })); + } else { + req.on('upgrade', function onUpgrade(res, socket) { + socket.destroy(); + done('upgrade'); + }); + } req.end(); } diff --git a/test/parallel/test-http-upgrade-server.js b/test/parallel/test-http-upgrade-server.js index 71a075831603da..d8a193c6edb4ec 100644 --- a/test/parallel/test-http-upgrade-server.js +++ b/test/parallel/test-http-upgrade-server.js @@ -121,8 +121,6 @@ function test_upgrade_with_listener() { /*----------------------------------------------- connection: Upgrade, no listener -----------------------------------------------*/ -let test_upgrade_no_listener_ended = false; - function test_upgrade_no_listener() { const conn = net.createConnection(server.address().port); conn.setEncoding('utf8'); @@ -135,8 +133,9 @@ function test_upgrade_no_listener() { '\r\n'); }); - conn.on('end', function() { - test_upgrade_no_listener_ended = true; + conn.once('data', (data) => { + assert.strictEqual(typeof data, 'string'); + assert.strictEqual(data.substr(0, 12), 'HTTP/1.1 200'); conn.end(); }); @@ -182,5 +181,4 @@ server.listen(0, function() { process.on('exit', function() { assert.strictEqual(3, requests_recv); assert.strictEqual(3, requests_sent); - assert.ok(test_upgrade_no_listener_ended); });