diff --git a/lib/_http_client.js b/lib/_http_client.js index b7311639f80db3..1b637c94209f67 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -490,7 +490,6 @@ function parserOnIncomingClient(res, shouldKeepAlive) { var socket = this.socket; var req = socket._httpMessage; - // propagate "domain" setting... if (req.domain && !res.domain) { debug('setting "res.domain"'); @@ -503,29 +502,22 @@ function parserOnIncomingClient(res, shouldKeepAlive) { // We already have a response object, this means the server // sent a double response. socket.destroy(); - return; + return 0; // No special treatment. } req.res = res; // Responses to CONNECT request is handled as Upgrade. - if (req.method === 'CONNECT') { + const method = req.method; + if (method === 'CONNECT') { res.upgrade = true; - return 2; // skip body, and the rest + return 2; // Skip body and treat as Upgrade. } - // Responses to HEAD requests are crazy. - // HEAD responses aren't allowed to have an entity-body - // but *can* have a content-length which actually corresponds - // to the content-length of the entity-body had the request - // been a GET. - var isHeadResponse = req.method === 'HEAD'; - debug('AGENT isHeadResponse', isHeadResponse); - if (res.statusCode === 100) { // restart the parser, as this is a continue message. req.res = null; // Clear res so that we don't hit double-responses. req.emit('continue'); - return true; + return 1; // Skip body but don't treat as Upgrade. } if (req.shouldKeepAlive && !shouldKeepAlive && !req.upgradeOrConnect) { @@ -535,7 +527,6 @@ function parserOnIncomingClient(res, shouldKeepAlive) { req.shouldKeepAlive = false; } - DTRACE_HTTP_CLIENT_RESPONSE(socket, req); LTTNG_HTTP_CLIENT_RESPONSE(socket, req); COUNTER_HTTP_CLIENT_RESPONSE(); @@ -553,7 +544,10 @@ function parserOnIncomingClient(res, shouldKeepAlive) { if (!handled) res._dump(); - return isHeadResponse; + if (method === 'HEAD') + return 1; // Skip body but don't treat as Upgrade. + + return 0; // No special treatment. } // client diff --git a/lib/_http_common.js b/lib/_http_common.js index 6ad13104772ede..b4caf5939e5afc 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -106,19 +106,10 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, parser.incoming.upgrade = upgrade; - var skipBody = 0; // response to HEAD or CONNECT + if (upgrade) + return 2; // Skip body and treat as Upgrade. - if (!upgrade) { - // For upgraded connections and CONNECT method request, we'll emit this - // after parser.execute so that we can capture the first part of the new - // protocol. - skipBody = parser.onIncoming(parser.incoming, shouldKeepAlive); - } - - if (typeof skipBody !== 'number') - return skipBody ? 1 : 0; - else - return skipBody; + return parser.onIncoming(parser.incoming, shouldKeepAlive); } // XXX This is a mess. diff --git a/lib/_http_server.js b/lib/_http_server.js index 5857e43d79c787..4a9733202de562 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -617,7 +617,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) { } else { server.emit('request', req, res); } - return false; // Not a HEAD response. (Not even a response!) + return 0; // No special treatment. } function resetSocketTimeout(server, socket, state) { diff --git a/test/parallel/test-http-upgrade-binary.js b/test/parallel/test-http-upgrade-binary.js new file mode 100644 index 00000000000000..002ac9c564ad1e --- /dev/null +++ b/test/parallel/test-http-upgrade-binary.js @@ -0,0 +1,28 @@ +'use strict'; +const { mustCall } = require('../common'); +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +// https://github.com/nodejs/node/issues/17789 - a connection upgrade response +// that has a Transfer-Encoding header and a body whose first byte is > 127 +// triggers a bug where said byte is skipped over. +net.createServer(mustCall(function(conn) { + conn.write('HTTP/1.1 101 Switching Protocols\r\n' + + 'Connection: upgrade\r\n' + + 'Transfer-Encoding: chunked\r\n' + + 'Upgrade: websocket\r\n' + + '\r\n' + + '\u0080', 'latin1'); + this.close(); +})).listen(0, mustCall(function() { + http.get({ + host: this.address().host, + port: this.address().port, + headers: { 'Connection': 'upgrade', 'Upgrade': 'websocket' }, + }).on('upgrade', mustCall((res, conn, head) => { + assert.strictEqual(head.length, 1); + assert.strictEqual(head[0], 128); + conn.destroy(); + })); +}));