From: Stephan Bosch Date: Fri, 10 Jan 2014 20:00:28 +0000 (-0500) Subject: lib-http: Added option to the header parser to make it lenient towards illegal charac... X-Git-Tag: 2.2.11~47 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e0cf44fb802a5e7aa4cbeee5e80ef8f3f6aecdbe;p=thirdparty%2Fdovecot%2Fcore.git lib-http: Added option to the header parser to make it lenient towards illegal characters in header field content. 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 729ccf49a5..88da94926f 100644 --- 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 7138caff2a..838d05661d 100644 --- 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 9df19f492f..d2bad277bb 100644 --- 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 879c094b4f..f9b39ef414 100644 --- 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 7151146238..0c64dede91 100644 --- 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 87d5b0ff45..1d5bea08c6 100644 --- 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 414027849d..ed0a83b4b1 100644 --- 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 d9e64cf16e..eb21986edf 100644 --- 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 d75627a00d..dbc3b93d7f 100644 --- 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);