update picohttpparser and reject multiline headers

deps/picohttpparser/.gitignore

@@ -0,0 +1 @@
+test-bin

deps/picohttpparser/picohttpparser.c

@@ -325,9 +325,21 @@ static const char *parse_headers(const char *buf, const char *buf_end, struct ph
             headers[*num_headers].name = NULL;
             headers[*num_headers].name_len = 0;
         }
-        if ((buf = get_token_to_eol(buf, buf_end, &headers[*num_headers].value, &headers[*num_headers].value_len, ret)) == NULL) {
+        const char *value;
+        size_t value_len;
+        if ((buf = get_token_to_eol(buf, buf_end, &value, &value_len, ret)) == NULL) {
             return NULL;
         }
+        /* remove trailing SPs and HTABs */
+        const char *value_end = value + value_len;
+        for (; value_end != value; --value_end) {
+            const char c = *(value_end - 1);
+            if (!(c == ' ' || c == '\t')) {
+                break;
+            }
+        }
+        headers[*num_headers].value = value;
+        headers[*num_headers].value_len = value_end - value;
     }
     return buf;
 }
@@ -350,6 +362,10 @@ static const char *parse_request(const char *buf, const char *buf_end, const cha
     ++buf;
     ADVANCE_TOKEN(*path, *path_len);
     ++buf;
+    if (*method_len == 0 || *path_len == 0) {
+        *ret = -1;
+        return NULL;
+    }
     if ((buf = parse_http_version(buf, buf_end, minor_version, ret)) == NULL) {
         return NULL;
     }

deps/picohttpparser/test.c

@@ -114,6 +114,9 @@ static void test_request(void)
     PARSE("GET /hoge HTTP/1.0\r\n\r", strlen("GET /hoge HTTP/1.0\r\n\r") - 1, -2, "slowloris (incomplete)");
     PARSE("GET /hoge HTTP/1.0\r\n\r\n", strlen("GET /hoge HTTP/1.0\r\n\r\n") - 1, 0, "slowloris (complete)");
 
+    PARSE(" / HTTP/1.0\r\n\r\n", 0, -1, "empty method");
+    PARSE("GET  HTTP/1.0\r\n\r\n", 0, -1, "empty request-target");
+
     PARSE("GET / HTTP/1.0\r\n:a\r\n\r\n", 0, -1, "empty header name");
     PARSE("GET / HTTP/1.0\r\n :a\r\n\r\n", 0, -1, "header name (space only)");
 
@@ -140,6 +143,9 @@ static void test_request(void)
 
     PARSE("GET / HTTP/1.0\r\n\x7b: 1\r\n\r\n", 0, -1, "disallow {");
 
+    PARSE("GET / HTTP/1.0\r\nfoo: a \t \r\n\r\n", 0, 0, "exclude leading and trailing spaces in header value");
+    ok(bufis(headers[0].value, headers[0].value_len, "a"));
+
 #undef PARSE
 }
 
@@ -230,6 +236,9 @@ static void test_response(void)
     PARSE("HTTP/1.2z 200 OK\r\n\r\n", 0, -1, "invalid http version 2");
     PARSE("HTTP/1.1  OK\r\n\r\n", 0, -1, "no status code");
 
+    PARSE("HTTP/1.1 200 OK\r\nbar: \t b\t \t\r\n\r\n", 0, 0, "exclude leading and trailing spaces in header value");
+    ok(bufis(headers[0].value, headers[0].value_len, "b"));
+
 #undef PARSE
 }
 

lib/common/http1client.c

@@ -265,6 +265,11 @@ static void on_head(h2o_socket_t *sock, const char *err)
 
         /* fill-in the headers */
         for (i = 0; i != num_headers; ++i) {
+            if (src_headers[i].name_len == 0) {
+                /* reject multiline header */
+                on_error_before_head(client, "line folding of header fields is not supported");
+                return;
+            }
             const h2o_token_t *token;
             char *orig_name = h2o_strdup(client->super.pool, src_headers[i].name, src_headers[i].name_len).base;
             h2o_strtolower((char *)src_headers[i].name, src_headers[i].name_len);

lib/http1.c

@@ -281,10 +281,10 @@ static int create_entity_reader(struct st_h2o_http1_conn_t *conn, const struct p
     return -1;
 }
 
-static ssize_t init_headers(h2o_mem_pool_t *pool, h2o_headers_t *headers, const struct phr_header *src, size_t len,
-                            h2o_iovec_t *connection, h2o_iovec_t *host, h2o_iovec_t *upgrade, h2o_iovec_t *expect)
+static int init_headers(h2o_mem_pool_t *pool, h2o_headers_t *headers, const struct phr_header *src, size_t len,
+                            h2o_iovec_t *connection, h2o_iovec_t *host, h2o_iovec_t *upgrade, h2o_iovec_t *expect, ssize_t *entity_header_index)
 {
-    ssize_t entity_header_index = -1;
+    *entity_header_index = -1;
 
     assert(headers->size == 0);
 
@@ -295,7 +295,9 @@ static ssize_t init_headers(h2o_mem_pool_t *pool, h2o_headers_t *headers, const
         for (i = 0; i != len; ++i) {
             const h2o_token_t *name_token;
             char orig_case[src[i].name_len];
-
+            /* reject multiline header */
+            if (src[i].name_len == 0)
+                return -1;
             /* preserve the original case */
             memcpy(orig_case, src[i].name, src[i].name_len);
             /* convert to lower-case in-place */
@@ -306,10 +308,10 @@ static ssize_t init_headers(h2o_mem_pool_t *pool, h2o_headers_t *headers, const
                         host->base = (char *)src[i].value;
                         host->len = src[i].value_len;
                     } else if (name_token == H2O_TOKEN_CONTENT_LENGTH) {
-                        if (entity_header_index == -1)
-                            entity_header_index = i;
+                        if (*entity_header_index == -1)
+                            *entity_header_index = i;
                     } else if (name_token == H2O_TOKEN_TRANSFER_ENCODING) {
-                        entity_header_index = i;
+                        *entity_header_index = i;
                     } else if (name_token == H2O_TOKEN_EXPECT) {
                         expect->base = (char *)src[i].value;
                         expect->len = src[i].value_len;
@@ -330,13 +332,12 @@ static ssize_t init_headers(h2o_mem_pool_t *pool, h2o_headers_t *headers, const
         }
     }
 
-    return entity_header_index;
+    return 0;
 }
 
-static ssize_t fixup_request(struct st_h2o_http1_conn_t *conn, struct phr_header *headers, size_t num_headers, int minor_version,
-                             h2o_iovec_t *expect)
+static int fixup_request(struct st_h2o_http1_conn_t *conn, struct phr_header *headers, size_t num_headers, int minor_version,
+                             h2o_iovec_t *expect, ssize_t *entity_header_index)
 {
-    ssize_t entity_header_index;
     h2o_iovec_t connection = {NULL, 0}, host = {NULL, 0}, upgrade = {NULL, 0};
 
     expect->base = NULL;
@@ -350,11 +351,11 @@ static ssize_t fixup_request(struct st_h2o_http1_conn_t *conn, struct phr_header
         conn->_ostr_final.super.send_informational = NULL;
 
     /* init headers */
-    entity_header_index =
-        init_headers(&conn->req.pool, &conn->req.headers, headers, num_headers, &connection, &host, &upgrade, expect);
+    if (init_headers(&conn->req.pool, &conn->req.headers, headers, num_headers, &connection, &host, &upgrade, expect, entity_header_index) != 0)
+        return -1;
 
     /* copy the values to pool, since the buffer pointed by the headers may get realloced */
-    if (entity_header_index != -1) {
+    if (*entity_header_index != -1) {
         size_t i;
         conn->req.input.method = h2o_strdup(&conn->req.pool, conn->req.input.method.base, conn->req.input.method.len);
         conn->req.input.path = h2o_strdup(&conn->req.pool, conn->req.input.path.base, conn->req.input.path.len);
@@ -392,7 +393,7 @@ static ssize_t fixup_request(struct st_h2o_http1_conn_t *conn, struct phr_header
     if (conn->req.http1_is_persistent && conn->super.ctx->shutdown_requested)
         conn->req.http1_is_persistent = 0;
 
-    return entity_header_index;
+    return 0;
 }
 
 static void on_continue_sent(h2o_socket_t *sock, const char *err)
@@ -416,10 +417,10 @@ static int contains_crlf_only(const char *s, size_t len)
     return 1;
 }
 
-static void send_bad_request(struct st_h2o_http1_conn_t *conn)
+static void send_bad_request(struct st_h2o_http1_conn_t *conn, const char *body)
 {
     h2o_socket_read_stop(conn->sock);
-    h2o_send_error_400(&conn->req, "Bad Request", "Bad Request", H2O_SEND_ERROR_HTTP1_CLOSE_CONNECTION);
+    h2o_send_error_400(&conn->req, "Bad Request", body, H2O_SEND_ERROR_HTTP1_CLOSE_CONNECTION);
 }
 
 static void handle_incoming_request(struct st_h2o_http1_conn_t *conn)
@@ -443,7 +444,12 @@ static void handle_incoming_request(struct st_h2o_http1_conn_t *conn)
     switch (reqlen) {
     default: // parse complete
         conn->_reqsize = reqlen;
-        if ((entity_body_header_index = fixup_request(conn, headers, num_headers, minor_version, &expect)) != -1) {
+        if (fixup_request(conn, headers, num_headers, minor_version, &expect, &entity_body_header_index) != 0) {
+            set_timeout(conn, 0, NULL);
+            send_bad_request(conn, "line folding of header fields is not supported");
+            return;
+        }
+        if (entity_body_header_index != -1) {
             conn->req.timestamps.request_body_begin_at = h2o_gettimeofday(conn->super.ctx->loop);
             if (expect.base != NULL) {
                 if (!h2o_lcstris(expect.base, expect.len, H2O_STRLIT("100-continue"))) {
@@ -473,7 +479,7 @@ static void handle_incoming_request(struct st_h2o_http1_conn_t *conn)
         return;
     case -2: // incomplete
         if (inreqlen == H2O_MAX_REQLEN) {
-            send_bad_request(conn);
+            send_bad_request(conn, "Bad Request");
         }
         return;
     case -1: // error
@@ -496,7 +502,7 @@ static void handle_incoming_request(struct st_h2o_http1_conn_t *conn)
         if (inreqlen <= 4 && contains_crlf_only(conn->sock->input->bytes, inreqlen)) {
             close_connection(conn, 1);
         } else {
-            send_bad_request(conn);
+            send_bad_request(conn, "Bad Request");
         }
         return;
     }

t/40bad-request.t

@@ -25,4 +25,13 @@ like $resp, qr{^HTTP/1\.1 400 .*Content-Length:\s*11.*\r\n\r\nBad Request$}is, "
 $resp = `echo "\r" | nc 127.0.0.1 $server->{port} 2>&1`;
 is $resp, "", "silent close on CRLF";
 
+$resp = `echo " / HTTP/1.1\r\n\r\n" | nc 127.0.0.1 $server->{port} 2>&1`;
+like $resp, qr{^HTTP/1\.1 400 .*Content-Length:\s*11.*\r\n\r\nBad Request$}is, "missing method";
+
+$resp = `echo "GET  HTTP/1.1\r\n\r\n" | nc 127.0.0.1 $server->{port} 2>&1`;
+like $resp, qr{^HTTP/1\.1 400 .*Content-Length:\s*11.*\r\n\r\nBad Request$}is, "missing path";
+
+$resp = `echo "GET / HTTP/1.1\r\nfoo: FOO\r\n    hoge\r\n\r\n" | nc 127.0.0.1 $server->{port} 2>&1`;
+like $resp, qr{^HTTP/1\.1 400 .*Content-Length:\s*46.*\r\n\r\nline folding of header fields is not supported$}is, "multiline header";
+
 done_testing;

t/50mruby.t

@@ -590,9 +590,6 @@ EOT
 
     (undef, $body) = $nc->('xyz');
     is $body, 'handler1, , xyz', 'no leading slash 4';
-
-    (undef, $body) = $nc->('');
-    is $body, 'handler1, , ', 'empty path';
 };
 
 subtest 'invalid response' => sub {

t/50reverse-proxy-multiline-header.t

@@ -0,0 +1,41 @@
+use strict;
+use warnings;
+use Net::EmptyPort qw(check_port empty_port);
+use Test::More;
+use t::Util;
+
+plan skip_all => 'curl not found'
+    unless prog_exists('curl');
+
+my $upstream_port = empty_port();
+my $upstream = IO::Socket::INET->new(
+    LocalHost => '127.0.0.1',
+    LocalPort => $upstream_port,
+    Proto => 'tcp',
+    Listen => 1,
+    Reuse => 1
+) or die "cannot create socket: $!";
+
+sub do_upstream {
+    my $client = $upstream->accept;
+    while (my $buf = <$client>) { last if $buf eq "\r\n" }
+    $client->send("HTTP/1.1 200 OK\r\nfoo: FOO\r\n    hoge\r\nConnection: close\r\n\r\n");
+    $client->flush;
+    close($client);
+}
+
+my $server = spawn_h2o(<< "EOT");
+hosts:
+  default:
+    paths:
+      "/":
+        proxy.reverse.url: http://127.0.0.1:$upstream_port
+EOT
+
+open(CURL, "curl --http1.1 --silent --dump-header /dev/stdout 'http://127.0.0.1:$server->{port}/' |");
+do_upstream();
+my $resp = join('', <CURL>);
+
+like $resp, qr{^HTTP/1\.1 502 .*Content-Length:\s*46.*\r\n\r\nline folding of header fields is not supported$}is;
+
+done_testing();