lib-http: Added option to the header parser to make it lenient towards illegal charac...

The offending characters are then skipped without error. This is required
for the http client as a workaround for bugs in certain HTTP servers. This
behavior is explicitly not enabled for the http-request-parser as used by
our own HTTP server implementation.

diff --git a/src/lib-http/http-header-parser.c b/src/lib-http/http-header-parser.c
index 729ccf49a5c0ca2551e4ff3c56e4a1cae601c34f..88da94926fbfcdf8cf5eedec270eb564684ca51a 100644 (file)
--- a/src/lib-http/http-header-parser.c
+++ b/src/lib-http/http-header-parser.c
@@ -37,16 +37,19 @@ 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)
+       const struct http_header_limits *limits, bool lenient)
 {
        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);
 
@@ -116,14 +119,27 @@ static int http_header_parse_ows(struct http_header_parser *parser)
 
 static int http_header_parse_content(struct http_header_parser *parser)
 {
-       const unsigned char *first = parser->cur;
+       const unsigned char *first;
 
        /* field-content  = *( HTAB / SP / VCHAR / obs-text )
         */
-       while (parser->cur < parser->end && http_char_is_text(*parser->cur))
-               parser->cur++;
+       do {
+               first = parser->cur;
+               while (parser->cur < parser->end && http_char_is_text(*parser->cur)) {
+                       parser->cur++;
+               }
+               buffer_append(parser->value_buf, first, parser->cur-first);
+
+               if (!parser->lenient)
+                       break;
 
-       buffer_append(parser->value_buf, first, parser->cur-first);
+               /* We'll be lenient here to accommodate for some bad servers. We just
+                  drop offending characters */
+               while (parser->cur < parser->end && !http_char_is_text(*parser->cur) &&
+                       (*parser->cur != '\r' && *parser->cur != '\n'))
+                       parser->cur++;
+       } while (parser->cur < parser->end &&
+               (*parser->cur != '\r' && *parser->cur != '\n'));
 
        if (parser->cur == parser->end)
                return 0;

diff --git a/src/lib-http/http-header-parser.h b/src/lib-http/http-header-parser.h
index 7138caff2a2751499fdb5e7c775ab97ad81c4504..838d05661dd9b1b2ff88e9c0416fc69751e00af8 100644 (file)
--- a/src/lib-http/http-header-parser.h
+++ b/src/lib-http/http-header-parser.h
@@ -6,7 +6,7 @@ struct http_header_parser;
 
 struct http_header_parser *
 http_header_parser_init(struct istream *input,
-       const struct http_header_limits *limits);
+       const struct http_header_limits *limits, bool lenient);
 void http_header_parser_deinit(struct http_header_parser **_parser);
 
 void http_header_parser_reset(struct http_header_parser *parser);

diff --git a/src/lib-http/http-message-parser.c b/src/lib-http/http-message-parser.c
index 9df19f492f1e46528d187e92e0d2b1f1fbf2bba3..d2bad277bb649ee6869a4c6d5cd8ffd61d55e7d1 100644 (file)
--- a/src/lib-http/http-message-parser.c
+++ b/src/lib-http/http-message-parser.c
@@ -14,13 +14,14 @@
 
 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)
+       uoff_t max_payload_size, bool lenient)
 {
        memset(parser, 0, sizeof(*parser));
        parser->input = input;
        if (hdr_limits != NULL)
                parser->header_limits = *hdr_limits;
        parser->max_payload_size = max_payload_size;
+       parser->lenient = lenient;
 }
 
 void http_message_parser_deinit(struct http_message_parser *parser)
@@ -39,8 +40,8 @@ void http_message_parser_restart(struct http_message_parser *parser,
        i_assert(parser->payload == NULL);
 
        if (parser->header_parser == NULL) {
-               parser->header_parser =
-                       http_header_parser_init(parser->input, &parser->header_limits);
+               parser->header_parser = http_header_parser_init
+                               (parser->input, &parser->header_limits, parser->lenient);
        } else {
                http_header_parser_reset(parser->header_parser);
        }

diff --git a/src/lib-http/http-message-parser.h b/src/lib-http/http-message-parser.h
index 879c094b4fb30d6361ffaf5239ac8e39da247718..f9b39ef41479b82e88b7cbc9f3545d4775b74bfa 100644 (file)
--- a/src/lib-http/http-message-parser.h
+++ b/src/lib-http/http-message-parser.h
@@ -51,11 +51,13 @@ 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) ATTR_NULL(3);
+       uoff_t max_payload_size, bool lenient) 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);

diff --git a/src/lib-http/http-request-parser.c b/src/lib-http/http-request-parser.c
index 7151146238343081901afbc66bc6d07c23d3af1e..0c64dede91d78944b2f1b89e8003cee943f1d69b 100644 (file)
--- a/src/lib-http/http-request-parser.c
+++ b/src/lib-http/http-request-parser.c
@@ -66,7 +66,7 @@ http_request_parser_init(struct istream *input,
                max_payload_size = HTTP_REQUEST_DEFAULT_MAX_PAYLOAD_SIZE;
 
        http_message_parser_init
-               (&parser->parser, input, &hdr_limits, max_payload_size);
+               (&parser->parser, input, &hdr_limits, max_payload_size, FALSE);
        return parser;
 }
 

diff --git a/src/lib-http/http-response-parser.c b/src/lib-http/http-response-parser.c
index 87d5b0ff45b2cbbe17e7605f28278e9998ddb9fb..1d5bea08c65d022354b8444740c9f28513b75926 100644 (file)
--- a/src/lib-http/http-response-parser.c
+++ b/src/lib-http/http-response-parser.c
@@ -37,7 +37,7 @@ http_response_parser_init(struct istream *input,
 
        /* FIXME: implement status line limit */
        parser = i_new(struct http_response_parser, 1);
-       http_message_parser_init(&parser->parser, input, hdr_limits, 0);
+       http_message_parser_init(&parser->parser, input, hdr_limits, 0, TRUE);
        return parser;
 }
 

diff --git a/src/lib-http/http-transfer-chunked.c b/src/lib-http/http-transfer-chunked.c
index 414027849d6505083f7837606f2a74254f95dc95..ed0a83b4b134c53b92ed2df608f076be312d82a0 100644 (file)
--- a/src/lib-http/http-transfer-chunked.c
+++ b/src/lib-http/http-transfer-chunked.c
@@ -434,9 +434,10 @@ static int http_transfer_chunked_parse_trailer(
        int ret;
 
        if (tcstream->header_parser == NULL) {
+               /* NOTE: trailer is currently ignored */
                /* FIXME: limit trailer size */
                tcstream->header_parser =
-                       http_header_parser_init(tcstream->istream.parent, 0);
+                       http_header_parser_init(tcstream->istream.parent, 0, TRUE);
        }
 
        while ((ret=http_header_parse_next_field(tcstream->header_parser,

diff --git a/src/lib-http/test-http-header-parser.c b/src/lib-http/test-http-header-parser.c
index d9e64cf16e5640c71b3c61d4b99ccefbe64507ce..eb21986edf8a9c223e71ce76bc9f627ef57a31fc 100644 (file)
--- a/src/lib-http/test-http-header-parser.c
+++ b/src/lib-http/test-http-header-parser.c
@@ -1,6 +1,7 @@
 /* Copyright (c) 2013 Dovecot authors, see the included COPYING file */
 
 #include "test-lib.h"
+#include "str-sanitize.h"
 #include "istream.h"
 #include "test-common.h"
 #include "http-response.h"
@@ -83,6 +84,16 @@ static struct http_header_parse_result valid_header_parse_result5[] = {
        { NULL, NULL }
 };
 
+static struct http_header_parse_result valid_header_parse_result6[] = {
+       { "X-Frop", "This text\x80 contains obs-text\x81 characters" },
+       { NULL, NULL }
+};
+
+static struct http_header_parse_result valid_header_parse_result7[] = {
+       { "X-Frop", "This text contains invalid characters" },
+       { NULL, NULL }
+};
+
 static const struct http_header_parse_test valid_header_parse_tests[] = {
        { .header = 
                        "Date: Sat, 06 Oct 2012 16:01:44 GMT\r\n"
@@ -150,6 +161,16 @@ static const struct http_header_parse_test valid_header_parse_tests[] = {
                .header =
                        "\r\n",
                .fields = valid_header_parse_result5
+       },{
+               .header =
+                       "X-Frop: This text\x80 contains obs-text\x81 characters\r\n"
+                       "\r\n",
+               .fields = valid_header_parse_result6
+       },{
+               .header =
+                       "X-Frop: This text\x01 contains invalid\x7f characters\r\n"
+                       "\r\n",
+               .fields = valid_header_parse_result7
        }
 };
 
@@ -173,7 +194,7 @@ 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);
+               parser = http_header_parser_init(input, limits, TRUE);
 
                test_begin(t_strdup_printf("http header valid [%d]", i));
 
@@ -196,15 +217,16 @@ static void test_http_header_parse_valid(void)
                        field_value = t_strndup(field_data, field_size);
 
                        if (result->name == NULL) {
-                               test_out_reason("valid", FALSE,
-                                       t_strdup_printf("%s: %s", field_name, field_value));
+                               test_out_reason("valid", FALSE, t_strdup_printf
+                                       ("%s: %s", field_name, str_sanitize(field_value, 100)));
                                break;
                        }
 
                        test_out_reason("valid",
                                strcmp(result->name, field_name) == 0 &&
-                               strcmp(result->value, field_value) == 0,
-                               t_strdup_printf("%s: %s", field_name, field_value));
+                                       strcmp(result->value, field_value) == 0,
+                               t_strdup_printf("%s: %s", field_name,
+                                       str_sanitize(field_value, 100)));
                        j++;
                }
 
@@ -238,8 +260,13 @@ static const struct http_header_parse_test invalid_header_parse_tests[] = {
                .header = 
                        "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,*/\177;q=0.5\n"
+                       "Accept:\t\timage/png,image/*;q=0.8,*/\1;q=0.5\n"
                        "\n"
+       },{
+               .header = 
+                       "Date: Sat, 06 Oct 2012 17:18:22 GMT\r\n"
+                       "Server: Apache/2.2.3\177 (CentOS)\r\n"
+                       "\r\n"
        },{
                .header = 
                        "Date: Sat, 06 Oct 2012 17:12:37 GMT\r\n"
@@ -247,6 +274,9 @@ static const struct http_header_parse_test invalid_header_parse_tests[] = {
                        "Suhosin-Patch proxy_html/3.0.1 mod_python/3.3.1 Python/2.6.6\r\n"
                        "mod_ssl/2.2.16 OpenSSL/0.9.8o mod_perl/2.0.4 Perl/v5.10.1\r\n"
                        "\r\n"
+       },{
+               .header = 
+                       "Date: Sat, 06 Oct 2012 17:12:37 GMT\r\n"
        },{
                .header = 
                        "Age: 58        \r\n"
@@ -318,7 +348,7 @@ 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);
+               parser = http_header_parser_init(input, limits, FALSE);
 
                test_begin(t_strdup_printf("http header invalid [%d]", i));
 

diff --git a/src/lib-http/test-http-response-parser.c b/src/lib-http/test-http-response-parser.c
index d75627a00d4a80175252005dedaabd93da5c55b8..dbc3b93d7f96c3968fc6fc0f7fb30922954f89ca 100644 (file)
--- a/src/lib-http/test-http-response-parser.c
+++ b/src/lib-http/test-http-response-parser.c
@@ -20,7 +20,7 @@ struct http_response_parse_test {
        const char *payload;
 };
 
-/* Valid header tests */
+/* Valid response tests */
 
 static const struct http_response_parse_test
 valid_response_parse_tests[] = {
@@ -39,7 +39,7 @@ valid_response_parse_tests[] = {
                        "This is a piece of stupid text.\r\n",
                .status = 200,
                .payload = "This is a piece of stupid text.\r\n"
-       },{ 
+       },{
                .response =
                        "HTTP/1.1 200 OK\r\n"
                        "Date: Sun, 07 Oct 2012 13:02:27 GMT\r\n"
@@ -134,7 +134,7 @@ static void test_http_response_parse_valid(void)
                }
 
                test_out("parse success", ret == 0);
-               
+
                if (ret == 0) {
                        /* verify last response only */
                        test_out(t_strdup_printf("response->status = %d",test->status),
@@ -156,6 +156,10 @@ static void test_http_response_parse_valid(void)
        buffer_free(&payload_buffer);
 }
 
+/*
+ * Invalid response tests
+ */
+
 static const char *invalid_response_parse_tests[] = {
        "XMPP/1.0 302 Found\r\n"
        "Location: http://www.example.nl/\r\n"
@@ -174,11 +178,6 @@ static const char *invalid_response_parse_tests[] = {
        "Cache-Control: private\n\r"
 };
 
-static unsigned char invalid_response_with_nuls[] =
-       "HTTP/1.1 200 OK\r\n"
-       "Server: text\0server\r\n"
-       "\r\n";
-
 unsigned int invalid_response_parse_test_count =
        N_ELEMENTS(invalid_response_parse_tests);
 
@@ -207,15 +206,39 @@ static void test_http_response_parse_invalid(void)
                test_end();
                http_response_parser_deinit(&parser);
        } T_END;
+}
+
+/*
+ * Bad response tests
+ */
+
+static unsigned char bad_response_with_nuls[] =
+       "HTTP/1.1 200 OK\r\n"
+       "Server: text\0server\r\n"
+       "\r\n";
+
+static void test_http_response_parse_bad(void)
+{
+       struct http_response_parser *parser;
+       struct http_response response;
+       const char *header, *error;
+       struct istream *input;
+       int ret;
 
        /* parse failure guarantees http_response_header.size equals
           strlen(http_response_header.value) */
        test_begin("http response with NULs");
-       input = i_stream_create_from_data(invalid_response_with_nuls,
-                                         sizeof(invalid_response_with_nuls)-1);
+       input = i_stream_create_from_data(bad_response_with_nuls,
+                                         sizeof(bad_response_with_nuls)-1);
        parser = http_response_parser_init(input, 0);
        while ((ret=http_response_parse_next(parser, FALSE, &response, &error)) > 0);
-       test_assert(ret < 0);
+       test_out("parse success", ret == 0);
+       header = http_response_header_get(&response, "server");
+       test_out("header present", header != NULL);
+       if (header != NULL) {
+               test_out(t_strdup_printf("header Server: %s", header),
+                       strcmp(header, "textserver") == 0);
+       }
        test_end();
 }
 
@@ -224,6 +247,7 @@ int main(void)
        static void (*test_functions[])(void) = {
                test_http_response_parse_valid,
                test_http_response_parse_invalid,
+               test_http_response_parse_bad,
                NULL
        };
        return test_run(test_functions);