]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib-http: Restructured the header, message, request, and response parsers to have...
authorStephan Bosch <stephan.bosch@dovecot.fi>
Tue, 11 Jul 2017 12:18:47 +0000 (14:18 +0200)
committerStephan Bosch <stephan.bosch@dovecot.fi>
Thu, 27 Jul 2017 13:07:29 +0000 (15:07 +0200)
Extended the test suites with a few cases that test parsing with and without the STRICT flag.

14 files changed:
src/lib-http/http-client-connection.c
src/lib-http/http-header-parser.c
src/lib-http/http-header-parser.h
src/lib-http/http-message-parser.c
src/lib-http/http-message-parser.h
src/lib-http/http-request-parser.c
src/lib-http/http-request-parser.h
src/lib-http/http-response-parser.c
src/lib-http/http-response-parser.h
src/lib-http/http-server-connection.c
src/lib-http/http-transfer-chunked.c
src/lib-http/test-http-header-parser.c
src/lib-http/test-http-request-parser.c
src/lib-http/test-http-response-parser.c

index 153287be9fa0fa102622433066c5ba50883a88f3..8e69ae4ff7bbe19521135a78cac2b61ebd69122e 100644 (file)
@@ -1204,7 +1204,7 @@ http_client_connection_ready(struct http_client_connection *conn)
 
        /* start protocol I/O */
        conn->http_parser = http_response_parser_init
-               (conn->conn.input, &conn->client->set.response_hdr_limits);
+               (conn->conn.input, &conn->client->set.response_hdr_limits, 0);
        o_stream_set_flush_callback(conn->conn.output,
     http_client_connection_output, conn);
 }
index 2b729dc786a8f86c236cd3f8a5837f3fb6fa233d..aecf636ef491176e835c33af8595e650476f5d45 100644 (file)
@@ -26,6 +26,8 @@ struct http_header_parser {
        struct istream *input;
 
        struct http_header_limits limits;
+       enum http_header_parse_flags flags;
+
        uoff_t size, field_size;
        unsigned int field_count;
 
@@ -36,21 +38,17 @@ struct http_header_parser {
        buffer_t *value_buf;
 
        enum http_header_parse_state state;
-
-       unsigned int lenient:1;
 };
 
 struct http_header_parser *
 http_header_parser_init(struct istream *input,
-       const struct http_header_limits *limits, bool lenient)
+       const struct http_header_limits *limits,
+       enum http_header_parse_flags flags)
 {
        struct http_header_parser *parser;
 
        parser = i_new(struct http_header_parser, 1);
        parser->input = input;
-       parser->lenient = lenient;
-       parser->name = str_new(default_pool, 128);
-       parser->value_buf = buffer_create_dynamic(default_pool, 4096);
 
        if (limits != NULL)
                parser->limits = *limits;
@@ -62,6 +60,11 @@ http_header_parser_init(struct istream *input,
        if (parser->limits.max_fields == 0)
                parser->limits.max_fields = (unsigned int)-1;
 
+       parser->flags = flags;
+
+       parser->name = str_new(default_pool, 128);
+       parser->value_buf = buffer_create_dynamic(default_pool, 4096);
+
        return parser;
 }
 
@@ -131,7 +134,7 @@ static int http_header_parse_content(struct http_header_parser *parser)
                }
                buffer_append(parser->value_buf, first, parser->cur-first);
 
-               if (!parser->lenient)
+               if ((parser->flags & HTTP_HEADER_PARSE_FLAG_STRICT) != 0)
                        break;
 
                /* We'll be lenient here to accommodate for some bad servers. We just
index 838d05661dd9b1b2ff88e9c0416fc69751e00af8..a7465209eaa900cb424d53c994b4ee8f82a814ab 100644 (file)
@@ -4,9 +4,15 @@
 struct http_header_limits;
 struct http_header_parser;
 
+enum http_header_parse_flags {
+       /* Strictly adhere to the HTTP protocol specification */
+       HTTP_HEADER_PARSE_FLAG_STRICT = BIT(0)
+};
+
 struct http_header_parser *
 http_header_parser_init(struct istream *input,
-       const struct http_header_limits *limits, bool lenient);
+       const struct http_header_limits *limits,
+       enum http_header_parse_flags flags);
 void http_header_parser_deinit(struct http_header_parser **_parser);
 
 void http_header_parser_reset(struct http_header_parser *parser);
index c6271bb8cc1a77854f9bee2fda20e8196ea0884d..6082f02181d113cd8a23df84d599240548338bc8 100644 (file)
@@ -16,7 +16,7 @@
 
 void http_message_parser_init(struct http_message_parser *parser,
        struct istream *input, const struct http_header_limits *hdr_limits,
-       uoff_t max_payload_size, bool lenient)
+       uoff_t max_payload_size, enum http_message_parse_flags flags)
 {
        i_zero(parser);
        parser->input = input;
@@ -24,7 +24,7 @@ void http_message_parser_init(struct http_message_parser *parser,
        if (hdr_limits != NULL)
                parser->header_limits = *hdr_limits;
        parser->max_payload_size = max_payload_size;
-       parser->lenient = lenient;
+       parser->flags = flags;
 }
 
 void http_message_parser_deinit(struct http_message_parser *parser)
@@ -45,8 +45,12 @@ void http_message_parser_restart(struct http_message_parser *parser,
        i_assert(parser->payload == NULL);
 
        if (parser->header_parser == NULL) {
+               enum http_header_parse_flags hdr_flags = 0;
+
+               if ((parser->flags & HTTP_MESSAGE_PARSE_FLAG_STRICT) != 0)
+                       hdr_flags |= HTTP_HEADER_PARSE_FLAG_STRICT;
                parser->header_parser = http_header_parser_init
-                               (parser->input, &parser->header_limits, parser->lenient);
+                       (parser->input, &parser->header_limits, hdr_flags);
        } else {
                http_header_parser_reset(parser->header_parser);
        }
index 7cef0ba743b3b14185f0d6d790e1e23270df9b1d..d2d2da727a8ffe8cec6f532c14dcd8064a667603 100644 (file)
@@ -17,6 +17,11 @@ enum http_message_parse_error {
                                                      (fatal) */
 };
 
+enum http_message_parse_flags {
+       /* Strictly adhere to the HTTP protocol specification */
+       HTTP_MESSAGE_PARSE_FLAG_STRICT = BIT(0)
+};
+
 struct http_message {
        pool_t pool;
 
@@ -40,6 +45,7 @@ struct http_message_parser {
 
        struct http_header_limits header_limits;
        uoff_t max_payload_size;
+       enum http_message_parse_flags flags;
 
        const unsigned char *cur, *end;
 
@@ -51,13 +57,12 @@ struct http_message_parser {
 
        pool_t msg_pool;
        struct http_message msg;
-
-       unsigned int lenient:1;
 };
 
 void http_message_parser_init(struct http_message_parser *parser,
        struct istream *input, const struct http_header_limits *hdr_limits,
-       uoff_t max_payload_size, bool lenient) ATTR_NULL(3);
+       uoff_t max_payload_size, enum http_message_parse_flags flags)
+       ATTR_NULL(3);
 void http_message_parser_deinit(struct http_message_parser *parser);
 void http_message_parser_restart(struct http_message_parser *parser,
        pool_t pool);
index 95e9371cb43b16ee0a3ddfe4fd5f1b32879cd072..aab38e460770509dac875b24cce431d9107b5485 100644 (file)
@@ -38,11 +38,13 @@ struct http_request_parser {
 
 struct http_request_parser *
 http_request_parser_init(struct istream *input,
-       const struct http_request_limits *limits)
+       const struct http_request_limits *limits,
+       enum http_request_parse_flags flags)
 {
        struct http_request_parser *parser;
        struct http_header_limits hdr_limits;
        uoff_t max_payload_size;
+       enum http_message_parse_flags msg_flags = 0;
 
        parser = i_new(struct http_request_parser, 1);
        if (limits != NULL) {
@@ -65,8 +67,10 @@ http_request_parser_init(struct istream *input,
        if (max_payload_size == 0)
                max_payload_size = HTTP_REQUEST_DEFAULT_MAX_PAYLOAD_SIZE;
 
-       http_message_parser_init
-               (&parser->parser, input, &hdr_limits, max_payload_size, FALSE);
+       if ((flags & HTTP_REQUEST_PARSE_FLAG_STRICT) != 0)
+               msg_flags |= HTTP_MESSAGE_PARSE_FLAG_STRICT;
+       http_message_parser_init(&parser->parser, input,
+               &hdr_limits, max_payload_size, msg_flags);
        return parser;
 }
 
index ba16dc20cd4aeb3cb68387b9c17775e1ee3ede0d..841e28ab671e6007d4fb4b7132b4a62c504c8354 100644 (file)
@@ -17,9 +17,15 @@ enum http_request_parse_error {
        HTTP_REQUEST_PARSE_ERROR_PAYLOAD_TOO_LARGE   /* payload too large (fatal) */
 };
 
+enum http_request_parse_flags {
+       /* Strictly adhere to the HTTP protocol specification */
+       HTTP_REQUEST_PARSE_FLAG_STRICT = BIT(0)
+};
+
 struct http_request_parser *
 http_request_parser_init(struct istream *input,
-       const struct http_request_limits *limits) ATTR_NULL(2);
+       const struct http_request_limits *limits,
+       enum http_request_parse_flags flags) ATTR_NULL(2);
 void http_request_parser_deinit(struct http_request_parser **_parser);
 
 int http_request_parse_finish_payload(
index a6fca1d3b3a909cefb3fe3d57a4045274b217f87..b4676f5a66085aa2bfe8c229f2797fdd29172d58 100644 (file)
@@ -32,13 +32,18 @@ struct http_response_parser {
 
 struct http_response_parser *
 http_response_parser_init(struct istream *input,
-       const struct http_header_limits *hdr_limits)
+       const struct http_header_limits *hdr_limits,
+       enum http_response_parse_flags flags)
 {
        struct http_response_parser *parser;
+       enum http_message_parse_flags msg_flags = 0;
 
        /* FIXME: implement status line limit */
+       if ((flags & HTTP_RESPONSE_PARSE_FLAG_STRICT) != 0)
+               msg_flags |= HTTP_MESSAGE_PARSE_FLAG_STRICT;
        parser = i_new(struct http_response_parser, 1);
-       http_message_parser_init(&parser->parser, input, hdr_limits, 0, TRUE);
+       http_message_parser_init(&parser->parser,
+               input, hdr_limits, 0, msg_flags);
        return parser;
 }
 
index f98d4e6f2c3700c23395866dc0e6744d929e39f1..6bcdeba9cc2293fc53a3140c89d444c6abdc359f 100644 (file)
@@ -6,9 +6,15 @@
 struct http_header_limits;
 struct http_response_parser;
 
+enum http_response_parse_flags {
+       /* Strictly adhere to the HTTP protocol specification */
+       HTTP_RESPONSE_PARSE_FLAG_STRICT = BIT(0)
+};
+
 struct http_response_parser *
 http_response_parser_init(struct istream *input,
-       const struct http_header_limits *hdr_limits) ATTR_NULL(2);
+       const struct http_header_limits *hdr_limits,
+       enum http_response_parse_flags flags) ATTR_NULL(2);
 void http_response_parser_deinit(struct http_response_parser **_parser);
 
 int http_response_parse_next(struct http_response_parser *parser,
index 5947c5a9fdff6aa728b84a28e97a63ad67038fab..60318ffde31df99264d326693858c71b33274b5a 100644 (file)
@@ -173,7 +173,8 @@ static void http_server_connection_ready(struct http_server_connection *conn)
        }
 
        conn->http_parser = http_request_parser_init
-               (conn->conn.input, &conn->server->set.request_limits);
+               (conn->conn.input, &conn->server->set.request_limits,
+                       HTTP_REQUEST_PARSE_FLAG_STRICT);
        o_stream_set_flush_callback(conn->conn.output,
     http_server_connection_output, conn);
 }
index 5872664ce6e16e2f8d3c28922a72f1d6565877ac..624fd352bb520d7929cd5968f3f20c8f2d4198ab 100644 (file)
@@ -434,7 +434,7 @@ static int http_transfer_chunked_parse_trailer(
                /* NOTE: trailer is currently ignored */
                /* FIXME: limit trailer size */
                tcstream->header_parser =
-                       http_header_parser_init(tcstream->istream.parent, NULL, TRUE);
+                       http_header_parser_init(tcstream->istream.parent, NULL, 0);
        }
 
        while ((ret=http_header_parse_next_field(tcstream->header_parser,
index 4ab5052cfcce4d8a54e3ad6aba705b65b3664674..4ccdbb66020ee0a33a2aa3de29799f938f8ec722 100644 (file)
@@ -17,6 +17,7 @@ struct http_header_parse_result {
 struct http_header_parse_test {
        const char *header;
        struct http_header_limits limits;
+       enum http_header_parse_flags flags;
        const struct http_header_parse_result *fields;
 };
 
@@ -194,7 +195,8 @@ static void test_http_header_parse_valid(void)
                header_len = strlen(header);
                limits = &valid_header_parse_tests[i].limits;
                input = test_istream_create_data(header, header_len);
-               parser = http_header_parser_init(input, limits, TRUE);
+               parser = http_header_parser_init(input, limits,
+                       valid_header_parse_tests[i].flags);
 
                test_begin(t_strdup_printf("http header valid [%d]", i));
 
@@ -262,12 +264,14 @@ static const struct http_header_parse_test invalid_header_parse_tests[] = {
                        "Host: p5-lrqzb4yavu4l7nagydw-428649-i2-v6exp3-ds.metric.example.com\n"
                        "User-Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0)\n"
                        "Accept:\t\timage/png,image/*;q=0.8,*/\1;q=0.5\n"
-                       "\n"
+                       "\n",
+               .flags = HTTP_HEADER_PARSE_FLAG_STRICT
        },{
                .header = 
                        "Date: Sat, 06 Oct 2012 17:18:22 GMT\r\n"
                        "Server: Apache/2.2.3\177 (CentOS)\r\n"
-                       "\r\n"
+                       "\r\n",
+               .flags = HTTP_HEADER_PARSE_FLAG_STRICT
        },{
                .header = 
                        "Date: Sat, 06 Oct 2012 17:12:37 GMT\r\n"
@@ -349,7 +353,8 @@ static void test_http_header_parse_invalid(void)
                header = invalid_header_parse_tests[i].header;
                limits = &invalid_header_parse_tests[i].limits;
                input = i_stream_create_from_data(header, strlen(header));
-               parser = http_header_parser_init(input, limits, FALSE);
+               parser = http_header_parser_init(input, limits,
+                       invalid_header_parse_tests[i].flags);
 
                test_begin(t_strdup_printf("http header invalid [%d]", i));
 
index f728a7d3e9e3bd0f87407146eefe837d789c2250..948ad86946b4cb81d220d14f223ab2df99531f45 100644 (file)
@@ -30,6 +30,8 @@ struct http_request_valid_parse_test {
        const char *payload;
        bool connection_close;
        bool expect_100_continue;
+
+       enum http_request_parse_flags flags;
 };
 
 static const struct http_request_valid_parse_test
@@ -177,7 +179,7 @@ static void test_http_request_parse_valid(void)
                request_text = test->request;
                request_text_len = strlen(request_text);
                input = test_istream_create_data(request_text, request_text_len);
-               parser = http_request_parser_init(input, NULL);
+               parser = http_request_parser_init(input, NULL, test->flags);
 
                test_begin(t_strdup_printf("http request valid [%d]", i));
 
@@ -293,6 +295,8 @@ static void test_http_request_parse_valid(void)
 
 struct http_request_invalid_parse_test {
        const char *request;
+       enum http_request_parse_flags flags;
+
        enum http_request_parse_error error_code;
 };
 
@@ -354,12 +358,6 @@ invalid_request_parse_tests[] = {
        // FIXME: test request limits
 };
 
-static const unsigned char invalid_request_with_nuls[] =
-       "GET / HTTP/1.1\r\n"
-       "Host: example.com\r\n"
-       "Null: text\0server\r\n"
-       "\r\n";
-
 static unsigned int invalid_request_parse_test_count =
        N_ELEMENTS(invalid_request_parse_tests);
 
@@ -404,7 +402,7 @@ static void test_http_request_parse_invalid(void)
                test = &invalid_request_parse_tests[i];
                request_text = test->request;
                input = i_stream_create_from_data(request_text, strlen(request_text));
-               parser = http_request_parser_init(input, NULL);
+               parser = http_request_parser_init(input, NULL, test->flags);
                i_stream_unref(&input);
 
                test_begin(t_strdup_printf("http request invalid [%d]", i));
@@ -420,13 +418,34 @@ static void test_http_request_parse_invalid(void)
                test_end();
                http_request_parser_deinit(&parser);
        } T_END;
+}
+
+/*
+ * Bad request tests
+ */
+
+static const unsigned char bad_request_with_nuls[] =
+       "GET / HTTP/1.1\r\n"
+       "Host: example.com\r\n"
+       "User-Agent: text\0client\r\n"
+       "\r\n";
+
+static void test_http_request_parse_bad(void)
+{
+       struct http_request_parser *parser;
+       struct http_request request;
+       enum http_request_parse_error error_code;
+       const char *header, *error;
+       struct istream *input;
+       int ret;
 
        /* parse failure guarantees http_request_header.size equals
           strlen(http_request_header.value) */
-       test_begin("http request with NULs");
-       input = i_stream_create_from_data(invalid_request_with_nuls,
-                                         sizeof(invalid_request_with_nuls)-1);
-       parser = http_request_parser_init(input, NULL);
+       test_begin("http request with NULs (strict)");
+       input = i_stream_create_from_data(bad_request_with_nuls,
+                                         sizeof(bad_request_with_nuls)-1);
+       parser = http_request_parser_init(input, NULL,
+               HTTP_REQUEST_PARSE_FLAG_STRICT);
        i_stream_unref(&input);
 
        while ((ret=http_request_parse_next
@@ -434,6 +453,28 @@ static void test_http_request_parse_invalid(void)
        test_assert(ret < 0);
        http_request_parser_deinit(&parser);
        test_end();
+
+       /* even when lenient, bad characters like NUL must not be returned */
+       test_begin("http request with NULs (lenient)");
+       input = i_stream_create_from_data(bad_request_with_nuls,
+                                         sizeof(bad_request_with_nuls)-1);
+       parser = http_request_parser_init(input, NULL, 0);
+       i_stream_unref(&input);
+
+       ret = http_request_parse_next
+               (parser, NULL, &request, &error_code, &error);
+       test_out("parse success", ret > 0);
+       header = http_request_header_get(&request, "user-agent");
+       test_out("header present", header != NULL);
+       if (header != NULL) {
+               test_out(t_strdup_printf("header User-Agent: %s", header),
+                       strcmp(header, "textclient") == 0);
+       }
+       ret = http_request_parse_next
+               (parser, NULL, &request, &error_code, &error);
+       test_out("parse end", ret == 0);
+       http_request_parser_deinit(&parser);
+       test_end();
 }
 
 int main(void)
@@ -441,6 +482,7 @@ int main(void)
        static void (*test_functions[])(void) = {
                test_http_request_parse_valid,
                test_http_request_parse_invalid,
+               test_http_request_parse_bad,
                NULL
        };
        return test_run(test_functions);
index 0b6b55c92d91ca99e59bef7246966656fa946d8a..982da97878284db37cc7d6f8032248c468472e37 100644 (file)
@@ -21,6 +21,7 @@ struct valid_parse_test_response {
 
 struct valid_parse_test {
        const char *input;
+       enum http_response_parse_flags flags;
 
        const struct valid_parse_test_response *responses;
        unsigned int responses_count;
@@ -140,7 +141,8 @@ static void test_http_response_parse_valid(void)
                input_text = test->input;
                input_text_len = strlen(input_text);
                input = test_istream_create_data(input_text, input_text_len);
-               parser = http_response_parser_init(input, NULL);
+               parser = http_response_parser_init(input, NULL,
+                       valid_response_parse_tests[i].flags);
 
                test_begin(t_strdup_printf("http response valid [%d]", i));
 
@@ -206,22 +208,38 @@ static void test_http_response_parse_valid(void)
  * Invalid response tests
  */
 
-static const char *invalid_response_parse_tests[] = {
-       "XMPP/1.0 302 Found\r\n"
-       "Location: http://www.example.nl/\r\n"
-       "Cache-Control: private\r\n",
-       "HTTP/1.1  302 Found\r\n"
-       "Location: http://www.example.nl/\r\n"
-       "Cache-Control: private\r\n",
-       "HTTP/1.1 ABC Found\r\n"
-       "Location: http://www.example.nl/\r\n"
-       "Cache-Control: private\r\n",
-       "HTTP/1.1 302 \177\r\n"
-       "Location: http://www.example.nl/\r\n"
-       "Cache-Control: private\r\n",
-       "HTTP/1.1 302 Found\n\r"
-       "Location: http://www.example.nl/\n\r"
-       "Cache-Control: private\n\r"
+struct invalid_parse_test {
+       const char *input;
+       enum http_response_parse_flags flags;
+};
+
+static struct invalid_parse_test invalid_response_parse_tests[] = {
+       {
+               .input =
+                       "XMPP/1.0 302 Found\r\n"
+                       "Location: http://www.example.nl/\r\n"
+                       "Cache-Control: private\r\n"
+       },{
+               .input =
+                       "HTTP/1.1  302 Found\r\n"
+                       "Location: http://www.example.nl/\r\n"
+                       "Cache-Control: private\r\n"
+       },{
+               .input =
+                       "HTTP/1.1 ABC Found\r\n"
+                       "Location: http://www.example.nl/\r\n"
+                       "Cache-Control: private\r\n"
+       },{
+               .input =
+                       "HTTP/1.1 302 \177\r\n"
+                       "Location: http://www.example.nl/\r\n"
+                       "Cache-Control: private\r\n"
+       },{
+               .input =
+                       "HTTP/1.1 302 Found\n\r"
+                       "Location: http://www.example.nl/\n\r"
+                       "Cache-Control: private\n\r"
+       }
 };
 
 static const unsigned int invalid_response_parse_test_count =
@@ -239,10 +257,11 @@ static void test_http_response_parse_invalid(void)
        for (i = 0; i < invalid_response_parse_test_count; i++) T_BEGIN {
                const char *test;
 
-               test = invalid_response_parse_tests[i];
+               test = invalid_response_parse_tests[i].input;
                response_text = test;
                input = i_stream_create_from_data(response_text, strlen(response_text));
-               parser = http_response_parser_init(input, NULL);
+               parser = http_response_parser_init(input, NULL,
+                       invalid_response_parse_tests[i].flags);
                i_stream_unref(&input);
 
                test_begin(t_strdup_printf("http response invalid [%d]", i));
@@ -274,11 +293,27 @@ static void test_http_response_parse_bad(void)
 
        /* parse failure guarantees http_response_header.size equals
           strlen(http_response_header.value) */
-       test_begin("http response with NULs");
+
+       test_begin("http response with NULs (strict)");
+       input = i_stream_create_from_data(bad_response_with_nuls,
+                                         sizeof(bad_response_with_nuls)-1);
+       parser = http_response_parser_init(input, NULL,
+               HTTP_RESPONSE_PARSE_FLAG_STRICT);
+       i_stream_unref(&input);
+
+       while ((ret=http_response_parse_next(parser,
+               HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED, &response, &error)) > 0);
+       test_assert(ret < 0);
+       http_response_parser_deinit(&parser);
+       test_end();
+
+       /* even when lenient, bad characters like NUL must not be returned */
+       test_begin("http response with NULs (lenient)");
        input = i_stream_create_from_data(bad_response_with_nuls,
                                          sizeof(bad_response_with_nuls)-1);
-       parser = http_response_parser_init(input, NULL);
+       parser = http_response_parser_init(input, NULL, 0);
        i_stream_unref(&input);
+
        ret = http_response_parse_next(parser,
                HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED, &response, &error);
        test_out("parse success", ret > 0);
@@ -291,8 +326,8 @@ static void test_http_response_parse_bad(void)
        ret = http_response_parse_next(parser,
                HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED, &response, &error);
        test_out("parse end", ret == 0);
-       test_end();
        http_response_parser_deinit(&parser);
+       test_end();
 }
 
 int main(void)