http: create an option for setting a maximum size for uri parsing

doc/api/http.md

@@ -1959,6 +1959,17 @@ added: v11.6.0
 Read-only property specifying the maximum allowed size of HTTP headers in bytes.
 Defaults to 8KB. Configurable using the [`--max-http-header-size`][] CLI option.
 
+## http.maxUriSize
+<!-- YAML
+added: REPLACEME
+-->
+
+* {number}
+
+Read-only property specifying the maximum allowed size of HTTP request uri
+in bytes. Defaults to 7KB. Configurable using the
+[`--max-http-uri-size`][] CLI option.
+
 ## http.request(options[, callback])
 ## http.request(url[, options][, callback])
 <!-- YAML

lib/_http_server.js

@@ -509,6 +509,10 @@ const badRequestResponse = Buffer.from(
   `HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
 );
+const uriTooLongResponse = Buffer.from(
+  `HTTP/1.1 414 ${STATUS_CODES[414]}${CRLF}` +
+  `Connection: close${CRLF}${CRLF}`, 'ascii'
+);
 const requestHeaderFieldsTooLargeResponse = Buffer.from(
   `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
@@ -520,9 +524,16 @@ function socketOnError(e) {
 
   if (!this.server.emit('clientError', e, this)) {
     if (this.writable) {
-      const response = e.code === 'HPE_HEADER_OVERFLOW' ?
-        requestHeaderFieldsTooLargeResponse : badRequestResponse;
-      this.write(response);
+      switch (e.code) {
+        case 'HPE_URI_OVERFLOW':
+          this.write(uriTooLongResponse);
+          break;
+        case 'HPE_HEADER_OVERFLOW':
+          this.write(requestHeaderFieldsTooLargeResponse);
+          break;
+        default:
+          this.write(badRequestResponse);
+      }
     }
     this.destroy(e);
   }

lib/http.js

@@ -33,6 +33,7 @@ const {
   ServerResponse
 } = require('_http_server');
 let maxHeaderSize;
+let maxUriSize;
 
 function createServer(opts, requestListener) {
   return new Server(opts, requestListener);
@@ -76,6 +77,19 @@ Object.defineProperty(module.exports, 'maxHeaderSize', {
   }
 });
 
+Object.defineProperty(module.exports, 'maxUriSize', {
+  configurable: true,
+  enumerable: true,
+  get() {
+    if (maxUriSize === undefined) {
+      const { getOptionValue } = require('internal/options');
+      maxUriSize = getOptionValue('--max-http-uri-size');
+    }
+
+    return maxUriSize;
+  }
+});
+
 Object.defineProperty(module.exports, 'globalAgent', {
   configurable: true,
   enumerable: true,

src/node_http_parser_impl.h

@@ -180,7 +180,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
 
   int on_url(const char* at, size_t length) {
-    int rv = TrackHeader(length);
+    int rv = TrackURI(length);
     if (rv != 0) {
       return rv;
     }
@@ -251,6 +251,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
   int on_headers_complete() {
 #ifdef NODE_EXPERIMENTAL_HTTP
+    uri_nread_ = 0;
     header_nread_ = 0;
 #endif  /* NODE_EXPERIMENTAL_HTTP */
 
@@ -813,6 +814,7 @@ class Parser : public AsyncWrap, public StreamListener {
   void Init(parser_type_t type) {
 #ifdef NODE_EXPERIMENTAL_HTTP
     llhttp_init(&parser_, type, &settings);
+    uri_nread_ = 0;
     header_nread_ = 0;
 #else  /* !NODE_EXPERIMENTAL_HTTP */
     http_parser_init(&parser_, type);
@@ -825,6 +827,17 @@ class Parser : public AsyncWrap, public StreamListener {
     got_exception_ = false;
   }
 
+  int TrackURI(size_t len) {
+#ifdef NODE_EXPERIMENTAL_HTTP
+    uri_nread_ += len;
+    if (uri_nread_ >= per_process::cli_options->max_http_uri_size) {
+      llhttp_set_error_reason(&parser_,
+                              "HPE_URI_OVERFLOW:URI overflow");
+      return HPE_USER;
+    }
+#endif  /* NODE_EXPERIMENTAL_HTTP */
+    return 0;
+  }
 
   int TrackHeader(size_t len) {
 #ifdef NODE_EXPERIMENTAL_HTTP
@@ -869,6 +882,7 @@ class Parser : public AsyncWrap, public StreamListener {
 #ifdef NODE_EXPERIMENTAL_HTTP
   unsigned int execute_depth_ = 0;
   bool pending_pause_ = false;
+  uint64_t uri_nread_ = 0;
   uint64_t header_nread_ = 0;
 #endif  /* NODE_EXPERIMENTAL_HTTP */
 

src/node_options.cc

@@ -426,6 +426,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
             "set the maximum size of HTTP headers (default: 8KB)",
             &PerProcessOptions::max_http_header_size,
             kAllowedInEnvironment);
+  AddOption("--max-http-uri-size",
+            "set the maximum size of HTTP request uri (default: 7KB)",
+            &PerProcessOptions::max_http_uri_size,
+            kAllowedInEnvironment);
   AddOption("--v8-pool-size",
             "set V8's thread pool size",
             &PerProcessOptions::v8_thread_pool_size,

src/node_options.h

@@ -157,6 +157,7 @@ class PerProcessOptions : public Options {
   std::string trace_event_categories;
   std::string trace_event_file_pattern = "node_trace.${rotation}.log";
   uint64_t max_http_header_size = 8 * 1024;
+  uint64_t max_http_uri_size = 7 * 1024;
   int64_t v8_thread_pool_size = 4;
   bool zero_fill_all_buffers = false;
   bool debug_arraybuffer_allocations = false;

test/parallel/test-http-header-overflow.js

@@ -1,18 +1,34 @@
+// Flags: --expose-internals
 'use strict';
+const { expectsError, mustCall } = require('../common');
 const assert = require('assert');
 const { createServer, maxHeaderSize } = require('http');
 const { createConnection } = require('net');
-const { expectsError, mustCall } = require('../common');
+
+const { getOptionValue } = require('internal/options');
+
+const currentParser = getOptionValue('--http-parser');
+
+const sumOfLengths = (strings) => strings
+  .map((string) => string.length)
+  .reduce((a, b) => a + b);
+
+const getBreakingLength = () => {
+  if (currentParser === 'llhttp') {
+    return maxHeaderSize - HEADER_NAME.length + 1;
+  } else {
+    return maxHeaderSize - HEADER_NAME.length - HEADER_SEPARATOR -
+      (2 * CRLF.length) + 1;
+  }
+};
 
 const CRLF = '\r\n';
-const DUMMY_HEADER_NAME = 'Cookie: ';
-const DUMMY_HEADER_VALUE = 'a'.repeat(
-  // Plus one is to make it 1 byte too big
-  maxHeaderSize - DUMMY_HEADER_NAME.length - (2 * CRLF.length) + 1
-);
+const HEADER_NAME = 'Cookie';
+const HEADER_SEPARATOR = ': ';
+const HEADER_VALUE = 'a'.repeat(getBreakingLength());
 const PAYLOAD_GET = 'GET /blah HTTP/1.1';
 const PAYLOAD = PAYLOAD_GET + CRLF +
-  DUMMY_HEADER_NAME + DUMMY_HEADER_VALUE + CRLF.repeat(2);
+  HEADER_NAME + HEADER_SEPARATOR + HEADER_VALUE + CRLF.repeat(2);
 
 const server = createServer();
 
@@ -21,7 +37,13 @@ server.on('connection', mustCall((socket) => {
     type: Error,
     message: 'Parse Error',
     code: 'HPE_HEADER_OVERFLOW',
-    bytesParsed: maxHeaderSize + PAYLOAD_GET.length,
+    bytesParsed: sumOfLengths([
+      PAYLOAD_GET,
+      CRLF,
+      HEADER_NAME,
+      HEADER_SEPARATOR,
+      HEADER_VALUE
+    ]) + 1,
     rawPacket: Buffer.from(PAYLOAD)
   }));
 }));

test/parallel/test-http-uri-overflow.js

@@ -0,0 +1,46 @@
+'use strict';
+const { expectsError, mustCall } = require('../common');
+const assert = require('assert');
+const { createServer, maxUriSize } = require('http');
+const { createConnection } = require('net');
+
+const CRLF = '\r\n';
+const REQUEST_METHOD_AND_SPACE = 'GET ';
+const DUMMY_URI = '/' + 'a'.repeat(
+  maxUriSize
+); // The slash makes it just 1 byte too long.
+
+const PAYLOAD = REQUEST_METHOD_AND_SPACE + DUMMY_URI + CRLF;
+
+const server = createServer();
+
+server.on('connection', mustCall((socket) => {
+  socket.on('error', expectsError({
+    type: Error,
+    message: 'Parse Error',
+    code: 'HPE_URI_OVERFLOW',
+    bytesParsed: REQUEST_METHOD_AND_SPACE.length + DUMMY_URI.length,
+    rawPacket: Buffer.from(PAYLOAD)
+  }));
+}));
+
+server.listen(0, mustCall(() => {
+  const c = createConnection(server.address().port);
+  let received = '';
+
+  c.on('connect', mustCall(() => {
+    c.write(PAYLOAD);
+  }));
+  c.on('data', mustCall((data) => {
+    received += data.toString();
+  }));
+  c.on('end', mustCall(() => {
+    assert.strictEqual(
+      received,
+      'HTTP/1.1 414 URI Too Long\r\n' +
+      'Connection: close\r\n\r\n'
+    );
+    c.end();
+  }));
+  c.on('close', mustCall(() => server.close()));
+}));

test/sequential/test-http-max-http-headers.js

@@ -122,8 +122,8 @@ const test2 = common.mustCall(() => {
     'Agent: nod2\r\n' +
     'X-CRASH: ';
 
-  // /, Host, localhost, Agent, node, X-CRASH, a...
-  const currentSize = 1 + 4 + 9 + 5 + 4 + 7;
+  // Host, localhost, Agent, node, X-CRASH, a...
+  const currentSize = 4 + 9 + 5 + 4 + 7;
   headers = fillHeaders(headers, currentSize);
 
   const server = http.createServer(common.mustNotCall());