http: skip body and next message of CONNECT res

deps/http_parser/Makefile

@@ -22,14 +22,14 @@ PLATFORM ?= $(shell sh -c 'uname -s | tr "[A-Z]" "[a-z]"')
 HELPER ?=
 BINEXT ?=
 ifeq (darwin,$(PLATFORM))
-SONAME ?= libhttp_parser.2.6.2.dylib
+SONAME ?= libhttp_parser.2.7.0.dylib
 SOEXT ?= dylib
 else ifeq (wine,$(PLATFORM))
 CC = winegcc
 BINEXT = .exe.so
 HELPER = wine
 else
-SONAME ?= libhttp_parser.so.2.6.2
+SONAME ?= libhttp_parser.so.2.7.0
 SOEXT ?= so
 endif
 

deps/http_parser/http_parser.c

@@ -1812,6 +1812,9 @@ size_t http_parser_execute (http_parser *parser,
             case 0:
               break;
 
+            case 2:
+              parser->upgrade = 1;
+
             case 1:
               parser->flags |= F_SKIPBODY;
               break;

deps/http_parser/http_parser.h

@@ -26,8 +26,8 @@ extern "C" {
 
 /* Also update SONAME in the Makefile whenever you change these. */
 #define HTTP_PARSER_VERSION_MAJOR 2
-#define HTTP_PARSER_VERSION_MINOR 6
-#define HTTP_PARSER_VERSION_PATCH 2
+#define HTTP_PARSER_VERSION_MINOR 7
+#define HTTP_PARSER_VERSION_PATCH 0
 
 #include <sys/types.h>
 #if defined(_WIN32) && !defined(__MINGW32__) && \
@@ -77,6 +77,11 @@ typedef struct http_parser_settings http_parser_settings;
  * HEAD request which may contain 'Content-Length' or 'Transfer-Encoding:
  * chunked' headers that indicate the presence of a body.
  *
+ * Returning `2` from on_headers_complete will tell parser that it should not
+ * expect neither a body nor any futher responses on this connection. This is
+ * useful for handling responses to a CONNECT request which may not contain
+ * `Upgrade` or `Connection: upgrade` headers.
+ *
  * http_data_cb does not return data chunks. It will be called arbitrarily
  * many times for each string. E.G. you might get 10 callbacks for "on_url"
  * each providing just a few characters more data.

deps/http_parser/test.c

@@ -2173,6 +2173,20 @@ pause_chunk_complete_cb (http_parser *p)
   return chunk_complete_cb(p);
 }
 
+int
+connect_headers_complete_cb (http_parser *p)
+{
+  headers_complete_cb(p);
+  return 1;
+}
+
+int
+connect_message_complete_cb (http_parser *p)
+{
+  messages[num_messages].should_keep_alive = http_should_keep_alive(parser);
+  return message_complete_cb(p);
+}
+
 static http_parser_settings settings_pause =
   {.on_message_begin = pause_message_begin_cb
   ,.on_header_field = pause_header_field_cb
@@ -2212,6 +2226,19 @@ static http_parser_settings settings_count_body =
   ,.on_chunk_complete = chunk_complete_cb
   };
 
+static http_parser_settings settings_connect =
+  {.on_message_begin = message_begin_cb
+  ,.on_header_field = header_field_cb
+  ,.on_header_value = header_value_cb
+  ,.on_url = request_url_cb
+  ,.on_status = response_status_cb
+  ,.on_body = dontcall_body_cb
+  ,.on_headers_complete = connect_headers_complete_cb
+  ,.on_message_complete = connect_message_complete_cb
+  ,.on_chunk_header = chunk_header_cb
+  ,.on_chunk_complete = chunk_complete_cb
+  };
+
 static http_parser_settings settings_null =
   {.on_message_begin = 0
   ,.on_header_field = 0
@@ -2275,6 +2302,14 @@ size_t parse_pause (const char *buf, size_t len)
   return nparsed;
 }
 
+size_t parse_connect (const char *buf, size_t len)
+{
+  size_t nparsed;
+  currently_parsing_eof = (len == 0);
+  nparsed = http_parser_execute(parser, &settings_connect, buf, len);
+  return nparsed;
+}
+
 static inline int
 check_str_eq (const struct message *m,
               const char *prop,
@@ -2331,7 +2366,7 @@ do {                                                                 \
 } while(0)
 
 int
-message_eq (int index, const struct message *expected)
+message_eq (int index, int connect, const struct message *expected)
 {
   int i;
   struct message *m = &messages[index];
@@ -2346,8 +2381,10 @@ message_eq (int index, const struct message *expected)
     MESSAGE_CHECK_STR_EQ(expected, m, response_status);
   }
 
-  MESSAGE_CHECK_NUM_EQ(expected, m, should_keep_alive);
-  MESSAGE_CHECK_NUM_EQ(expected, m, message_complete_on_eof);
+  if (!connect) {
+    MESSAGE_CHECK_NUM_EQ(expected, m, should_keep_alive);
+    MESSAGE_CHECK_NUM_EQ(expected, m, message_complete_on_eof);
+  }
 
   assert(m->message_begin_cb_called);
   assert(m->headers_complete_cb_called);
@@ -2385,16 +2422,22 @@ message_eq (int index, const struct message *expected)
     MESSAGE_CHECK_NUM_EQ(expected, m, port);
   }
 
-  if (expected->body_size) {
+  if (connect) {
+    check_num_eq(m, "body_size", 0, m->body_size);
+  } else if (expected->body_size) {
     MESSAGE_CHECK_NUM_EQ(expected, m, body_size);
   } else {
     MESSAGE_CHECK_STR_EQ(expected, m, body);
   }
 
-  assert(m->num_chunks == m->num_chunks_complete);
-  MESSAGE_CHECK_NUM_EQ(expected, m, num_chunks_complete);
-  for (i = 0; i < m->num_chunks && i < MAX_CHUNKS; i++) {
-    MESSAGE_CHECK_NUM_EQ(expected, m, chunk_lengths[i]);
+  if (connect) {
+    check_num_eq(m, "num_chunks_complete", 0, m->num_chunks_complete);
+  } else {
+    assert(m->num_chunks == m->num_chunks_complete);
+    MESSAGE_CHECK_NUM_EQ(expected, m, num_chunks_complete);
+    for (i = 0; i < m->num_chunks && i < MAX_CHUNKS; i++) {
+      MESSAGE_CHECK_NUM_EQ(expected, m, chunk_lengths[i]);
+    }
   }
 
   MESSAGE_CHECK_NUM_EQ(expected, m, num_headers);
@@ -3201,7 +3244,7 @@ test_message (const struct message *message)
       abort();
     }
 
-    if(!message_eq(0, message)) abort();
+    if(!message_eq(0, 0, message)) abort();
 
     parser_free();
   }
@@ -3238,7 +3281,7 @@ test_message_count_body (const struct message *message)
     abort();
   }
 
-  if(!message_eq(0, message)) abort();
+  if(!message_eq(0, 0, message)) abort();
 
   parser_free();
 }
@@ -3589,9 +3632,9 @@ test_multiple3 (const struct message *r1, const struct message *r2, const struct
     abort();
   }
 
-  if (!message_eq(0, r1)) abort();
-  if (message_count > 1 && !message_eq(1, r2)) abort();
-  if (message_count > 2 && !message_eq(2, r3)) abort();
+  if (!message_eq(0, 0, r1)) abort();
+  if (message_count > 1 && !message_eq(1, 0, r2)) abort();
+  if (message_count > 2 && !message_eq(2, 0, r3)) abort();
 
   parser_free();
 }
@@ -3687,17 +3730,17 @@ test_scan (const struct message *r1, const struct message *r2, const struct mess
           goto error;
         }
 
-        if (!message_eq(0, r1)) {
+        if (!message_eq(0, 0, r1)) {
           fprintf(stderr, "\n\nError matching messages[0] in test_scan.\n");
           goto error;
         }
 
-        if (message_count > 1 && !message_eq(1, r2)) {
+        if (message_count > 1 && !message_eq(1, 0, r2)) {
           fprintf(stderr, "\n\nError matching messages[1] in test_scan.\n");
           goto error;
         }
 
-        if (message_count > 2 && !message_eq(2, r3)) {
+        if (message_count > 2 && !message_eq(2, 0, r3)) {
           fprintf(stderr, "\n\nError matching messages[2] in test_scan.\n");
           goto error;
         }
@@ -3796,7 +3839,29 @@ test_message_pause (const struct message *msg)
     abort();
   }
 
-  if(!message_eq(0, msg)) abort();
+  if(!message_eq(0, 0, msg)) abort();
+
+  parser_free();
+}
+
+/* Verify that body and next message won't be parsed in responses to CONNECT */
+void
+test_message_connect (const struct message *msg)
+{
+  char *buf = (char*) msg->raw;
+  size_t buflen = strlen(msg->raw);
+  size_t nread;
+
+  parser_init(msg->type);
+
+  nread = parse_connect(buf, buflen);
+
+  if (num_messages != 1) {
+    printf("\n*** num_messages != 1 after testing '%s' ***\n\n", msg->name);
+    abort();
+  }
+
+  if(!message_eq(0, 1, msg)) abort();
 
   parser_free();
 }
@@ -3867,6 +3932,10 @@ main (void)
     test_message_pause(&responses[i]);
   }
 
+  for (i = 0; i < response_count; i++) {
+    test_message_connect(&responses[i]);
+  }
+
   for (i = 0; i < response_count; i++) {
     if (!responses[i].should_keep_alive) continue;
     for (j = 0; j < response_count; j++) {

lib/_http_client.js

@@ -432,7 +432,7 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
   // Responses to CONNECT request is handled as Upgrade.
   if (req.method === 'CONNECT') {
     res.upgrade = true;
-    return true; // skip body
+    return 2; // skip body, and the rest
   }
 
   // Responses to HEAD requests are crazy.

lib/_http_common.js

@@ -96,7 +96,7 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
 
   parser.incoming.upgrade = upgrade;
 
-  var skipBody = false; // response to HEAD or CONNECT
+  var skipBody = 0; // response to HEAD or CONNECT
 
   if (!upgrade) {
     // For upgraded connections and CONNECT method request, we'll emit this
@@ -105,7 +105,10 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
     skipBody = parser.onIncoming(parser.incoming, shouldKeepAlive);
   }
 
-  return skipBody;
+  if (typeof skipBody !== 'number')
+    return skipBody ? 1 : 0;
+  else
+    return skipBody;
 }
 
 // XXX This is a mess.

src/node_http_parser.cc

@@ -300,7 +300,7 @@ class Parser : public AsyncWrap {
       return -1;
     }
 
-    return head_response->IsTrue() ? 1 : 0;
+    return head_response->IntegerValue();
   }
 
 

test/parallel/test-http-connect-req-res.js

@@ -0,0 +1,72 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(function(req, res) {
+  assert(false);
+});
+server.on('connect', common.mustCall(function(req, socket, firstBodyChunk) {
+  assert.equal(req.method, 'CONNECT');
+  assert.equal(req.url, 'example.com:443');
+  console.error('Server got CONNECT request');
+
+  // It is legal for the server to send some data intended for the client
+  // along with the CONNECT response
+  socket.write(
+    'HTTP/1.1 200 Connection established\r\n' +
+    'Date: Tue, 15 Nov 1994 08:12:31 GMT\r\n' +
+    '\r\n' +
+    'Head'
+  );
+
+  var data = firstBodyChunk.toString();
+  socket.on('data', function(buf) {
+    data += buf.toString();
+  });
+  socket.on('end', function() {
+    socket.end(data);
+  });
+}));
+server.listen(common.PORT, common.mustCall(function() {
+  const req = http.request({
+    port: common.PORT,
+    method: 'CONNECT',
+    path: 'example.com:443'
+  }, function(res) {
+    assert(false);
+  });
+
+  req.on('close', common.mustCall(function() { }));
+
+  req.on('connect', common.mustCall(function(res, socket, firstBodyChunk) {
+    console.error('Client got CONNECT request');
+
+    // Make sure this request got removed from the pool.
+    const name = 'localhost:' + common.PORT;
+    assert(!http.globalAgent.sockets.hasOwnProperty(name));
+    assert(!http.globalAgent.requests.hasOwnProperty(name));
+
+    // Make sure this socket has detached.
+    assert(!socket.ondata);
+    assert(!socket.onend);
+    assert.equal(socket.listeners('connect').length, 0);
+    assert.equal(socket.listeners('data').length, 0);
+
+    var data = firstBodyChunk.toString();
+
+    // test that the firstBodyChunk was not parsed as HTTP
+    assert.equal(data, 'Head');
+
+    socket.on('data', function(buf) {
+      data += buf.toString();
+    });
+    socket.on('end', function() {
+      assert.equal(data, 'HeadRequestEnd');
+      server.close();
+    });
+    socket.end('End');
+  }));
+
+  req.end('Request');
+}));