http: fix connection upgrade checks

This commit fixes connection upgrade checks, specifically when headers are passed as an array instead of a plain object to http.request() Fixes: #8235 PR-URL: #8238 Reviewed-By: James M Snell <[email protected]>

Author: mscdex

lib/_http_common.js

@@ -78,17 +78,11 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
 parser.incoming.statusMessage=statusMessage;
 }
 
-// The client made non-upgrade request, and server is just advertising
-// supported protocols.
-//
-// See RFC7230 Section 6.7
-//
-// NOTE: RegExp below matches `upgrade` in `Connection: abc, upgrade, def`
-// header.
-if(upgrade&&
-parser.outgoing!==null&&
-(parser.outgoing._headers.upgrade===undefined||
-!/(^|\W)upgrade(\W|$)/i.test(parser.outgoing._headers.connection))){
+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;
 }
 

lib/_http_outgoing.js

@@ -9,16 +9,18 @@ const Buffer = require('buffer').Buffer;
 constcommon=require('_http_common');
 
 constCRLF=common.CRLF;
-constchunkExpression=common.chunkExpression;
+consttrfrEncChunkExpression=common.chunkExpression;
 constdebug=common.debug;
 
-constconnectionExpression=/^Connection$/i;
+constupgradeExpression=/^Upgrade$/i;
 consttransferEncodingExpression=/^Transfer-Encoding$/i;
-constcloseExpression=/close/i;
 constcontentLengthExpression=/^Content-Length$/i;
 constdateExpression=/^Date$/i;
 constexpectExpression=/^Expect$/i;
 consttrailerExpression=/^Trailer$/i;
+constconnectionExpression=/^Connection$/i;
+constconnCloseExpression=/(^|\W)close(\W|$)/i;
+constconnUpgradeExpression=/(^|\W)upgrade(\W|$)/i;
 constlenientHttpHeaders=!!process.REVERT_CVE_2016_2216;
 
 constautomaticHeaders={
@@ -62,6 +64,7 @@ function OutgoingMessage(){
 this.writable=true;
 
 this._last=false;
+this.upgrading=false;
 this.chunkedEncoding=false;
 this.shouldKeepAlive=true;
 this.useChunkedEncodingByDefault=true;
@@ -191,11 +194,13 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers){
 // in the case of response it is: 'HTTP/1.1 200 OK\r\n'
 varstate={
 sentConnectionHeader: false,
+sentConnectionUpgrade: false,
 sentContentLengthHeader: false,
 sentTransferEncodingHeader: false,
 sentDateHeader: false,
 sentExpect: false,
 sentTrailer: false,
+sentUpgrade: false,
 messageHeader: firstLine
 };
 
@@ -224,6 +229,10 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers){
 }
 }
 
+// Are we upgrading the connection?
+if(state.sentConnectionUpgrade&&state.sentUpgrade)
+this.upgrading=true;
+
 // Date header
 if(this.sendDate===true&&state.sentDateHeader===false){
 state.messageHeader+='Date: '+utcDate()+CRLF;
@@ -313,15 +322,16 @@ function storeHeader(self, state, field, value){
 
 if(connectionExpression.test(field)){
 state.sentConnectionHeader=true;
-if(closeExpression.test(value)){
+if(connCloseExpression.test(value)){
 self._last=true;
 }else{
 self.shouldKeepAlive=true;
 }
-
+if(connUpgradeExpression.test(value))
+state.sentConnectionUpgrade=true;
 }elseif(transferEncodingExpression.test(field)){
 state.sentTransferEncodingHeader=true;
-if(chunkExpression.test(value))self.chunkedEncoding=true;
+if(trfrEncChunkExpression.test(value))self.chunkedEncoding=true;
 
 }elseif(contentLengthExpression.test(field)){
 state.sentContentLengthHeader=true;
@@ -331,6 +341,8 @@ function storeHeader(self, state, field, value){
 state.sentExpect=true;
 }elseif(trailerExpression.test(field)){
 state.sentTrailer=true;
+}elseif(upgradeExpression.test(field)){
+state.sentUpgrade=true;
 }
 }
 

test/parallel/test-http-upgrade-client.js

@@ -25,40 +25,52 @@ var srv = net.createServer(function(c){
 });
 });
 
-vargotUpgrade=false;
-
-srv.listen(0,'127.0.0.1',function(){
-
-varreq=http.get({
-port: this.address().port,
-headers: {
+srv.listen(0,'127.0.0.1',common.mustCall(function(){
+varport=this.address().port;
+varheaders=[
+{
 connection: 'upgrade',
 upgrade: 'websocket'
-}
-});
-req.on('upgrade',function(res,socket,upgradeHead){
-varrecvData=upgradeHead;
-socket.on('data',function(d){
-recvData+=d;
+},
+[
+['Host','echo.websocket.org'],
+['Connection','Upgrade'],
+['Upgrade','websocket'],
+['Origin','http://www.websocket.org']
+]
+];
+varleft=headers.length;
+headers.forEach(function(h){
+varreq=http.get({
+port: port,
+headers: h
 });
+varsawUpgrade=false;
+req.on('upgrade',common.mustCall(function(res,socket,upgradeHead){
+sawUpgrade=true;
+varrecvData=upgradeHead;
+socket.on('data',function(d){
+recvData+=d;
+});
 
-socket.on('close',common.mustCall(function(){
-assert.equal(recvData,'nurtzo');
-}));
-
-console.log(res.headers);
-varexpectedHeaders={'hello': 'world',
-'connection': 'upgrade',
-'upgrade': 'websocket'};
-assert.deepEqual(expectedHeaders,res.headers);
+socket.on('close',common.mustCall(function(){
+assert.equal(recvData,'nurtzo');
+}));
 
-socket.end();
-srv.close();
+console.log(res.headers);
+varexpectedHeaders={
+hello: 'world',
+connection: 'upgrade',
+upgrade: 'websocket'
+};
+assert.deepStrictEqual(expectedHeaders,res.headers);
 
-gotUpgrade=true;
+socket.end();
+if(--left==0)
+srv.close();
+}));
+req.on('close',common.mustCall(function(){
+assert.strictEqual(sawUpgrade,true);
+}));
 });
-});
-
-process.on('exit',function(){
-assert.ok(gotUpgrade);
-});
+}));