HTTP parser: allowed more characters in header field names.

Previously, all requests that contained in header field names characters other
than alphanumeric, or "-", or "_" were rejected with a 400 "Bad Request" error
response.

Now, the parser allows the same set of characters as specified in RFC 7230,
including: "!", "#", "$", "%", "&", "'", "*", "+", ".", "^", "`", "|", and "~".
Header field names that contain only these characters are considered valid.

Also, there's a new option introduced: "discard_unsafe_fields".  It accepts
boolean value and it is set to "true" by default.

When this option is "true", all header field names that contain characters
in valid range, but other than alphanumeric or "-" are skipped during parsing.
When the option is "false", these header fields aren't skipped.

Requests with non-valid characters in header field names according to
RFC 7230 are rejected regardless of "discard_unsafe_fields" setting.

This closes #422 issue on GitHub.

src/nxt_conf_validation.c

@@ -273,6 +273,9 @@ static nxt_conf_vldt_object_t  nxt_conf_vldt_http_members[] = {
     }, {
         .name       = nxt_string("body_temp_path"),
         .type       = NXT_CONF_VLDT_STRING,
+    }, {
+        .name       = nxt_string("discard_unsafe_fields"),
+        .type       = NXT_CONF_VLDT_BOOLEAN,
     }, {
         .name       = nxt_string("websocket"),
         .type       = NXT_CONF_VLDT_OBJECT,

src/nxt_h1proto.c

@@ -467,6 +467,7 @@ nxt_h1p_conn_request_init(nxt_task_t *task, void *obj, void *data)
     nxt_int_t                ret;
     nxt_conn_t               *c;
     nxt_h1proto_t            *h1p;
+    nxt_socket_conf_t        *skcf;
     nxt_http_request_t       *r;
     nxt_socket_conf_joint_t  *joint;
 
@@ -503,11 +504,14 @@ nxt_h1p_conn_request_init(nxt_task_t *task, void *obj, void *data)
             joint->count++;
 
             r->conf = joint;
+            skcf = joint->socket_conf;
 
             if (c->local == NULL) {
-                c->local = joint->socket_conf->sockaddr;
+                c->local = skcf->sockaddr;
             }
 
+            h1p->parser.discard_unsafe_fields = skcf->discard_unsafe_fields;
+
             nxt_h1p_conn_request_header_parse(task, c, h1p);
             return;
         }

src/nxt_http_parse.c

@@ -288,11 +288,13 @@ continue_target:
         case NXT_HTTP_TARGET_SPACE:
             rp->target_end = p;
             goto space_after_target;
-
+#if 0
         case NXT_HTTP_TARGET_QUOTE_MARK:
             rp->quoted_target = 1;
             goto rest_of_target;
-
+#else
+        case NXT_HTTP_TARGET_QUOTE_MARK:
+#endif
         case NXT_HTTP_TARGET_HASH:
             rp->complex_target = 1;
             goto rest_of_target;
@@ -378,7 +380,7 @@ space_after_target:
             }
         }
 
-        rp->space_in_target = 1;
+        //rp->space_in_target = 1;
 
         if (rest) {
             goto rest_of_target;
@@ -397,7 +399,7 @@ space_after_target:
             goto space_after_target;
         }
 
-        rp->space_in_target = 1;
+        //rp->space_in_target = 1;
 
         if (rest) {
             goto rest_of_target;
@@ -432,7 +434,12 @@ space_after_target:
             *pos = p + 10;
         }
 
-        if (rp->complex_target != 0 || rp->quoted_target != 0) {
+        if (rp->complex_target != 0
+#if 0
+            || rp->quoted_target != 0
+#endif
+           )
+        {
             rc = nxt_http_parse_complex_target(rp);
 
             if (nxt_slow_path(rc != NXT_OK)) {
@@ -518,11 +525,13 @@ nxt_http_parse_field_name(nxt_http_request_parse_t *rp, u_char **pos,
 
     static const u_char  normal[256]  nxt_aligned(64) =
         "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
-        "\0\0\0\0\0\0\0\0\0\0\0\0\0-\0\0" "0123456789\0\0\0\0\0\0"
+    /*   \s ! " # $ % & ' ( ) * + ,        . /                 : ; < = > ?   */
+        "\0\1\0\1\1\1\1\1\0\0\1\1\0" "-" "\1\0" "0123456789" "\0\0\0\0\0\0"
 
-        /* These 64 bytes should reside in one cache line. */
+    /*    @                                 [ \ ] ^ _                        */
-        "\0abcdefghijklmnopqrstuvwxyz\0\0\0\0_"
+        "\0" "abcdefghijklmnopqrstuvwxyz" "\0\0\0\1\1"
-        "\0abcdefghijklmnopqrstuvwxyz\0\0\0\0\0"
+    /*    `                                 { | } ~                          */
+        "\1" "abcdefghijklmnopqrstuvwxyz" "\0\1\0\1\0"
 
         "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
         "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
@@ -538,9 +547,14 @@ nxt_http_parse_field_name(nxt_http_request_parse_t *rp, u_char **pos,
                                                                               \
         c = normal[ch];                                                       \
                                                                               \
-        if (nxt_slow_path(c == '\0')) {                                       \
+        if (nxt_slow_path(c <= '\1')) {                                       \
-            p = &(ch);                                                        \
+            if (c == '\0') {                                                  \
-            goto name_end;                                                    \
+                p = &(ch);                                                    \
+                goto name_end;                                                \
+            }                                                                 \
+                                                                              \
+            rp->skip_field = rp->discard_unsafe_fields;                       \
+            c = ch;                                                           \
         }                                                                     \
                                                                               \
         hash = nxt_http_field_hash_char(hash, c);
@@ -777,21 +791,26 @@ nxt_http_parse_field_end(nxt_http_request_parse_t *rp, u_char **pos,
         *pos = p + 1;
 
         if (rp->field_name.length != 0) {
-            field = nxt_list_add(rp->fields);
+            if (rp->skip_field) {
+                rp->skip_field = 0;
 
-            if (nxt_slow_path(field == NULL)) {
+            } else {
-                return NXT_ERROR;
+                field = nxt_list_add(rp->fields);
+
+                if (nxt_slow_path(field == NULL)) {
+                    return NXT_ERROR;
+                }
+
+                field->hash = nxt_http_field_hash_end(rp->field_hash);
+                field->skip = 0;
+                field->hopbyhop = 0;
+
+                field->name_length = rp->field_name.length;
+                field->value_length = rp->field_value.length;
+                field->name = rp->field_name.start;
+                field->value = rp->field_value.start;
             }
 
-            field->hash = nxt_http_field_hash_end(rp->field_hash);
-            field->skip = 0;
-            field->hopbyhop = 0;
-
-            field->name_length = rp->field_name.length;
-            field->value_length = rp->field_value.length;
-            field->name = rp->field_name.start;
-            field->value = rp->field_value.start;
-
             rp->field_hash = NXT_HTTP_FIELD_HASH_INIT;
 
             rp->field_name.length = 0;
@@ -1023,7 +1042,7 @@ nxt_http_parse_complex_target(nxt_http_request_parse_t *rp)
             break;
 
         case sw_quoted:
-            rp->quoted_target = 1;
+            //rp->quoted_target = 1;
 
             if (ch >= '0' && ch <= '9') {
                 high = (u_char) (ch - '0');

src/nxt_http_parse.h

@@ -55,15 +55,19 @@ struct nxt_http_request_parse_s {
 
     uint32_t                  field_hash;
 
-    /* target with "/." */
+    uint8_t                   skip_field;             /* 1 bit */
-    uint8_t                   complex_target;   /* 1 bit */
+    uint8_t                   discard_unsafe_fields;  /* 1 bit */
-    /* target with "%" */
-    uint8_t                   quoted_target;    /* 1 bit */
-    /* target with " " */
-    uint8_t                   space_in_target;  /* 1 bit */
 
+    /* target with "/." */
+    uint8_t                   complex_target;         /* 1 bit */
+#if 0
+    /* target with "%" */
+    uint8_t                   quoted_target;          /* 1 bit */
+    /* target with " " */
+    uint8_t                   space_in_target;        /* 1 bit */
+#endif
     /* Preserve encoded '/' (%2F) and '%' (%25). */
-    uint8_t                   encoded_slashes;  /* 1 bit */
+    uint8_t                   encoded_slashes;        /* 1 bit */
 };
 
 

src/nxt_router.c

@@ -1262,6 +1262,12 @@ static nxt_conf_map_t  nxt_router_http_conf[] = {
         NXT_CONF_MAP_STR,
         offsetof(nxt_socket_conf_t, body_temp_path),
     },
+
+    {
+        nxt_string("discard_unsafe_fields"),
+        NXT_CONF_MAP_INT8,
+        offsetof(nxt_socket_conf_t, discard_unsafe_fields),
+    },
 };
 
 
@@ -1649,6 +1655,7 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf,
             skcf->header_buffer_size = 2048;
             skcf->large_header_buffer_size = 8192;
             skcf->large_header_buffers = 4;
+            skcf->discard_unsafe_fields = 1;
             skcf->body_buffer_size = 16 * 1024;
             skcf->max_body_size = 8 * 1024 * 1024;
             skcf->proxy_header_buffer_size = 64 * 1024;

src/nxt_router.h

@@ -191,6 +191,8 @@ typedef struct {
 
     nxt_str_t              body_temp_path;
 
+    uint8_t                discard_unsafe_fields;  /* 1 bit */
+
 #if (NXT_TLS)
     nxt_tls_conf_t         *tls;
 #endif

src/test/nxt_http_parse_test.c

@@ -23,9 +23,15 @@ typedef struct {
 } nxt_http_parse_test_request_line_t;
 
 
+typedef struct {
+    nxt_int_t  result;
+    unsigned   discard_unsafe_fields:1;
+} nxt_http_parse_test_fields_t;
+
+
 typedef union {
     void                                *pointer;
-    nxt_int_t                           result;
+    nxt_http_parse_test_fields_t        fields;
     nxt_http_parse_test_request_line_t  request_line;
 } nxt_http_parse_test_data_t;
 
@@ -324,10 +330,11 @@ static nxt_http_parse_test_case_t  nxt_http_test_cases[] = {
     {
         nxt_string("GET / HTTP/1.1\r\n"
                    "X-Unknown-Header: value\r\n"
-                   "X-Good-Header: value\r\n\r\n"),
+                   "X-Good-Header: value\r\n"
+                   "!#$%&'*+.^_`|~: skipped\r\n\r\n"),
         NXT_DONE,
         &nxt_http_parse_test_fields,
-        { .result = NXT_OK }
+        { .fields = { NXT_OK, 1 } }
     },
     {
         nxt_string("GET / HTTP/1.1\r\n"
@@ -336,7 +343,14 @@ static nxt_http_parse_test_case_t  nxt_http_test_cases[] = {
                    "X-Bad-Header: value\r\n\r\n"),
         NXT_DONE,
         &nxt_http_parse_test_fields,
-        { .result = NXT_ERROR }
+        { .fields = { NXT_ERROR, 1 } }
+    },
+    {
+        nxt_string("GET / HTTP/1.1\r\n"
+                   "!#$%&'*+.^_`|~: allowed\r\n\r\n"),
+        NXT_DONE,
+        &nxt_http_parse_test_fields,
+        { .fields = { NXT_ERROR, 0 } }
     },
 };
 
@@ -349,6 +363,10 @@ static nxt_http_field_proc_t  nxt_http_test_fields[] = {
     { nxt_string("X-Good-Header"),
       &nxt_http_test_header_return,
       NXT_OK },
+
+    { nxt_string("!#$%&'*+.^_`|~"),
+      &nxt_http_test_header_return,
+      NXT_ERROR },
 };
 
 
@@ -540,6 +558,10 @@ nxt_http_parse_test(nxt_thread_t *thr)
             return NXT_ERROR;
         }
 
+        if (test->handler == &nxt_http_parse_test_fields) {
+            rp.discard_unsafe_fields = test->data.fields.discard_unsafe_fields;
+        }
+
         rc = nxt_http_parse_test_run(&rp, &test->request);
 
         if (rc != test->result) {
@@ -740,7 +762,7 @@ nxt_http_parse_test_request_line(nxt_http_request_parse_t *rp,
         return NXT_ERROR;
     }
 
-    if (rp->complex_target != test->complex_target) {
+    if (rp->complex_target != (test->complex_target | test->quoted_target)) {
         nxt_log_alert(log, "http parse test case failed:\n"
                            " - request:\n\"%V\"\n"
                            " - complex_target: %d (expected: %d)",
@@ -748,6 +770,7 @@ nxt_http_parse_test_request_line(nxt_http_request_parse_t *rp,
         return NXT_ERROR;
     }
 
+#if 0
     if (rp->quoted_target != test->quoted_target) {
         nxt_log_alert(log, "http parse test case failed:\n"
                            " - request:\n\"%V\"\n"
@@ -763,6 +786,7 @@ nxt_http_parse_test_request_line(nxt_http_request_parse_t *rp,
                            request, rp->space_in_target, test->space_in_target);
         return NXT_ERROR;
     }
+#endif
 
     return NXT_OK;
 }
@@ -776,11 +800,11 @@ nxt_http_parse_test_fields(nxt_http_request_parse_t *rp,
 
     rc = nxt_http_fields_process(rp->fields, &nxt_http_test_fields_hash, NULL);
 
-    if (rc != data->result) {
+    if (rc != data->fields.result) {
         nxt_log_alert(log, "http parse test hash failed:\n"
                            " - request:\n\"%V\"\n"
                            " - result: %i (expected: %i)",
-                           request, rc, data->result);
+                           request, rc, data->fields.result);
         return NXT_ERROR;
     }