count headers, URL and start lines separately

Counting headers, URL and start lines separately because
it was counting everything as headers before the headers_done state,
throwing HPE_HEADER_OVERFLOW errors even when what really caused
the overflow was the request URL being too long

Creating an HPE_URL_OVERFLOW err to make sure embedders
still have a request line limit, but now differentiating what
went over such limit.

Refs: nodejs/node#26553

Author: caiolrm

http_parser.c

@@ -26,6 +26,7 @@
 #include <limits.h>
 
 static uint32_t max_header_size = HTTP_MAX_HEADER_SIZE;
+static uint32_t max_url_size = HTTP_MAX_URL_SIZE;
 
 #ifndef ULLONG_MAX
 # define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */
@@ -160,6 +161,24 @@ do {                                                                 \
   }                                                                  \
 } while (0)
 
+/* Don't allow the total size of the request uri to exceed 
+ * the max_url_size. If we don't check the request uri size and 
+ * it exceeds the max_header_size, the embedder will get an 
+ * HPE_HEADER_OVERFLOW, which is ambiguous in situations where
+ * what caused the overflow matters, e.g. servers deciding to 
+ * reply either 414 or 431 status.
+ */
+#include <stdio.h>
+#define COUNT_URL_SIZE(V, O)                                         \
+do {                                                                 \
+  nread += (uint32_t)(V);                                            \
+  url_offset = (uint8_t)(O);                                         \
+  if (UNLIKELY(nread > url_offset &&                                 \
+      (nread - url_offset) > max_url_size)) {                        \
+    SET_ERRNO(HPE_URL_OVERFLOW);                                     \
+    goto error;                                                      \
+  }                                                                  \
+} while(0)                                                           \
 
 #define PROXY_CONNECTION "proxy-connection"
 #define CONNECTION "connection"
@@ -173,12 +192,17 @@ do {                                                                 \
 
 static const char *method_strings[] =
   {
-#define XX(num, name, string) #string,
+#define XX(num, name, string, length) #string,
   HTTP_METHOD_MAP(XX)
 #undef XX
   };
 
-
+static const uint8_t method_lengths[] =
+  {
+#define XX(num, name, string, length) length,
+  HTTP_METHOD_MAP(XX)
+#undef XX
+  };
 /* Tokens as defined by rfc 2616. Also lowercases them.
  *        token       = 1*<any CHAR except CTLs or separators>
  *     separators     = "(" | ")" | "<" | ">" | "@"
@@ -358,9 +382,9 @@ enum state
   , s_message_done
   };
 
-
-#define PARSING_HEADER(state) (state <= s_headers_done)
-
+#define PARSING_HEADER(state) (state > s_req_http_end && state <= s_headers_done)
+#define PARSING_URL(state) (state > s_req_spaces_before_url && state < s_req_http_start)
+#define PARSING_START_LINE(state) (state <= s_req_spaces_before_url || (state >= s_req_http_start && state <= s_req_http_end))
 
 enum header_states
   { h_general = 0
@@ -642,6 +666,8 @@ size_t http_parser_execute (http_parser *parser,
 {
   char c, ch;
   int8_t unhex_val;
+  uint8_t url_offset;
+  uint32_t spaces_before_url = 0;
   const char *p = data;
   const char *header_field_mark = 0;
   const char *header_value_mark = 0;
@@ -707,9 +733,14 @@ size_t http_parser_execute (http_parser *parser,
   for (p=data; p != data + len; p++) {
     ch = *p;
 
-    if (PARSING_HEADER(CURRENT_STATE()))
+    if (PARSING_HEADER(CURRENT_STATE())) {
       COUNT_HEADER_SIZE(1);
-
+    } else if (PARSING_URL(CURRENT_STATE())) {
+      COUNT_URL_SIZE(1, method_lengths[parser->method] + spaces_before_url);
+    } else if (PARSING_START_LINE(CURRENT_STATE())) {
+      nread++;
+    }
+    
 reexecute:
     switch (CURRENT_STATE()) {
 
@@ -1015,7 +1046,10 @@ size_t http_parser_execute (http_parser *parser,
 
       case s_req_spaces_before_url:
       {
-        if (ch == ' ') break;
+        if (ch == ' ') {
+          ++spaces_before_url;
+          break;
+        }
 
         MARK(url);
         if (parser->method == HTTP_CONNECT) {
@@ -2499,3 +2533,8 @@ void
 http_parser_set_max_header_size(uint32_t size) {
   max_header_size = size;
 }
+
+void
+http_parser_set_max_uri_size(uint32_t size) {
+  max_url_size = size;
+}

http_parser.h

@@ -63,6 +63,17 @@ typedef unsigned __int64 uint64_t;
 # define HTTP_MAX_HEADER_SIZE (80*1024)
 #endif
 
+/* Maximium uri size allowed. If the macro is not defined
+ * before including this header then the default is used. To
+ * change the maximum uri size, define the macro in the build
+ * environment (e.g. -DHTTP_MAX_URL_SIZE=<value>). To remove
+ * the effective limit on the size of the uri, define the macro
+ * to a very large number (e.g. -DHTTP_MAX_URL_SIZE=0x7fffffff)
+ */
+#ifndef HTTP_MAX_URL_SIZE
+# define HTTP_MAX_URL_SIZE (80*1024)
+#endif
+
 typedef struct http_parser http_parser;
 typedef struct http_parser_settings http_parser_settings;
 
@@ -160,53 +171,53 @@ enum http_status
 
 
 /* Request Methods */
-#define HTTP_METHOD_MAP(XX)         \
-  XX(0,  DELETE,      DELETE)       \
-  XX(1,  GET,         GET)          \
-  XX(2,  HEAD,        HEAD)         \
-  XX(3,  POST,        POST)         \
-  XX(4,  PUT,         PUT)          \
-  /* pathological */                \
-  XX(5,  CONNECT,     CONNECT)      \
-  XX(6,  OPTIONS,     OPTIONS)      \
-  XX(7,  TRACE,       TRACE)        \
-  /* WebDAV */                      \
-  XX(8,  COPY,        COPY)         \
-  XX(9,  LOCK,        LOCK)         \
-  XX(10, MKCOL,       MKCOL)        \
-  XX(11, MOVE,        MOVE)         \
-  XX(12, PROPFIND,    PROPFIND)     \
-  XX(13, PROPPATCH,   PROPPATCH)    \
-  XX(14, SEARCH,      SEARCH)       \
-  XX(15, UNLOCK,      UNLOCK)       \
-  XX(16, BIND,        BIND)         \
-  XX(17, REBIND,      REBIND)       \
-  XX(18, UNBIND,      UNBIND)       \
-  XX(19, ACL,         ACL)          \
-  /* subversion */                  \
-  XX(20, REPORT,      REPORT)       \
-  XX(21, MKACTIVITY,  MKACTIVITY)   \
-  XX(22, CHECKOUT,    CHECKOUT)     \
-  XX(23, MERGE,       MERGE)        \
-  /* upnp */                        \
-  XX(24, MSEARCH,     M-SEARCH)     \
-  XX(25, NOTIFY,      NOTIFY)       \
-  XX(26, SUBSCRIBE,   SUBSCRIBE)    \
-  XX(27, UNSUBSCRIBE, UNSUBSCRIBE)  \
-  /* RFC-5789 */                    \
-  XX(28, PATCH,       PATCH)        \
-  XX(29, PURGE,       PURGE)        \
-  /* CalDAV */                      \
-  XX(30, MKCALENDAR,  MKCALENDAR)   \
-  /* RFC-2068, section 19.6.1.2 */  \
-  XX(31, LINK,        LINK)         \
-  XX(32, UNLINK,      UNLINK)       \
-  /* icecast */                     \
-  XX(33, SOURCE,      SOURCE)       \
+#define HTTP_METHOD_MAP(XX)            \
+  XX(0,  DELETE,      DELETE,  6)      \
+  XX(1,  GET,         GET,  3)         \
+  XX(2,  HEAD,        HEAD,  4)        \
+  XX(3,  POST,        POST,  4)        \
+  XX(4,  PUT,         PUT,  3)         \
+  /* pathological */                   \
+  XX(5,  CONNECT,     CONNECT,  7)     \
+  XX(6,  OPTIONS,     OPTIONS,  7)     \
+  XX(7,  TRACE,       TRACE,  5)       \
+  /* WebDAV */                         \
+  XX(8,  COPY,        COPY,  4)        \
+  XX(9,  LOCK,        LOCK,  4)        \
+  XX(10, MKCOL,       MKCOL, 5)        \
+  XX(11, MOVE,        MOVE, 4)         \
+  XX(12, PROPFIND,    PROPFIND, 8)     \
+  XX(13, PROPPATCH,   PROPPATCH, 9)    \
+  XX(14, SEARCH,      SEARCH, 6)       \
+  XX(15, UNLOCK,      UNLOCK, 6)       \
+  XX(16, BIND,        BIND, 4)         \
+  XX(17, REBIND,      REBIND, 6)       \
+  XX(18, UNBIND,      UNBIND, 6)       \
+  XX(19, ACL,         ACL, 3)          \
+  /* subversion */                     \
+  XX(20, REPORT,      REPORT, 6)       \
+  XX(21, MKACTIVITY,  MKACTIVITY, 10)  \
+  XX(22, CHECKOUT,    CHECKOUT, 8)     \
+  XX(23, MERGE,       MERGE, 5)        \
+  /* upnp */                           \
+  XX(24, MSEARCH,     M-SEARCH, 7)     \
+  XX(25, NOTIFY,      NOTIFY, 6)       \
+  XX(26, SUBSCRIBE,   SUBSCRIBE, 9)    \
+  XX(27, UNSUBSCRIBE, UNSUBSCRIBE, 11) \
+  /* RFC-5789 */                       \
+  XX(28, PATCH,       PATCH, 5)        \
+  XX(29, PURGE,       PURGE, 5)        \
+  /* CalDAV */                         \
+  XX(30, MKCALENDAR,  MKCALENDAR, 10)  \
+  /* RFC-2068, section 19.6.1.2 */     \
+  XX(31, LINK,        LINK, 4)         \
+  XX(32, UNLINK,      UNLINK, 6)       \
+  /* icecast */                        \
+  XX(33, SOURCE,      SOURCE, 6)       \
 
 enum http_method
   {
-#define XX(num, name, string) HTTP_##name = num,
+#define XX(num, name, string, length) HTTP_##name = num,
   HTTP_METHOD_MAP(XX)
 #undef XX
   };
@@ -252,6 +263,8 @@ enum flags
   XX(INVALID_EOF_STATE, "stream ended at an unexpected time")        \
   XX(HEADER_OVERFLOW,                                                \
      "too many header bytes seen; overflow detected")                \
+  XX(URL_OVERFLOW,                                                   \
+     "too many url bytes seen; overflow detected")                   \
   XX(CLOSED_CONNECTION,                                              \
      "data received after completed connection: close message")      \
   XX(INVALID_VERSION, "invalid HTTP version")                        \
@@ -433,6 +446,9 @@ int http_body_is_final(const http_parser *parser);
 /* Change the maximum header size provided at compile time. */
 void http_parser_set_max_header_size(uint32_t size);
 
+/* Change the maximum uri size provided at compile time. */
+void http_parser_set_max_uri_size(uint32_t size);
+
 #ifdef __cplusplus
 }
 #endif

test.c

@@ -3756,6 +3756,32 @@ test_header_overflow_error (int req)
   abort();
 }
 
+void
+test_URL_OVERFLOW_error ()
+{
+  http_parser parser;
+  http_parser_init(&parser, HTTP_REQUEST);
+  size_t parsed;
+  const char *buf;
+  buf = "GET /";
+  parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf));
+  assert(parsed == strlen(buf));
+
+  buf = "a";
+  size_t buflen = strlen(buf);
+
+  int i;
+  for (i = 0; i < HTTP_MAX_URL_SIZE - 1; i++) { //-1 because it started with a slash
+    parsed = http_parser_execute(&parser, &settings_null, buf, buflen);
+    if (parsed != buflen) {
+      assert(HTTP_PARSER_ERRNO(&parser) == HPE_URL_OVERFLOW);
+      return;
+    }
+  }
+
+  fprintf(stderr, "\n*** Error expected but none in uri overflow test ***\n");
+  abort();
+}
 
 void
 test_header_nread_value ()
@@ -4159,6 +4185,8 @@ main (void)
   //// OVERFLOW CONDITIONS
   test_no_overflow_parse_url();
 
+  test_URL_OVERFLOW_error();
+
   test_header_overflow_error(HTTP_REQUEST);
   test_no_overflow_long_body(HTTP_REQUEST, 1000);
   test_no_overflow_long_body(HTTP_REQUEST, 100000);