Mercurial > dovecot > core-2.2
changeset 22389:36a38929734d
lib-http: Restructured the header, message, request, and response parsers to have an extensible and consistent API using flags.
Extended the test suites with a few cases that test parsing with and without the STRICT flag.
| author | Stephan Bosch <stephan.bosch@dovecot.fi> |
|---|---|
| date | Tue, 11 Jul 2017 14:18:47 +0200 |
| parents | df373becc66c |
| children | 110cf7691bba |
| files | 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 |
| diffstat | 14 files changed, 184 insertions(+), 62 deletions(-) [+] |
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-client-connection.c Tue Jul 11 14:18:47 2017 +0200 @@ -1204,7 +1204,7 @@ /* 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); }
--- a/src/lib-http/http-header-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-header-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -26,6 +26,8 @@ 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 @@ 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 @@ 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 @@ } 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
--- a/src/lib-http/http-header-parser.h Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-header-parser.h Tue Jul 11 14:18:47 2017 +0200 @@ -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);
--- a/src/lib-http/http-message-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-message-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -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 @@ 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 @@ 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); }
--- a/src/lib-http/http-message-parser.h Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-message-parser.h Tue Jul 11 14:18:47 2017 +0200 @@ -17,6 +17,11 @@ (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_header_limits header_limits; uoff_t max_payload_size; + enum http_message_parse_flags flags; const unsigned char *cur, *end; @@ -51,13 +57,12 @@ 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);
--- a/src/lib-http/http-request-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-request-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -38,11 +38,13 @@ 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 @@ 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; }
--- a/src/lib-http/http-request-parser.h Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-request-parser.h Tue Jul 11 14:18:47 2017 +0200 @@ -17,9 +17,15 @@ 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(
--- a/src/lib-http/http-response-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-response-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -32,13 +32,18 @@ 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; }
--- a/src/lib-http/http-response-parser.h Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-response-parser.h Tue Jul 11 14:18:47 2017 +0200 @@ -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,
--- a/src/lib-http/http-server-connection.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-server-connection.c Tue Jul 11 14:18:47 2017 +0200 @@ -173,7 +173,8 @@ } 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); }
--- a/src/lib-http/http-transfer-chunked.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/http-transfer-chunked.c Tue Jul 11 14:18:47 2017 +0200 @@ -434,7 +434,7 @@ /* 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,
--- a/src/lib-http/test-http-header-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/test-http-header-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -17,6 +17,7 @@ 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 @@ 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 @@ "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 @@ 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));
--- a/src/lib-http/test-http-request-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/test-http-request-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -30,6 +30,8 @@ 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 @@ 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ 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 @@ 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);
--- a/src/lib-http/test-http-response-parser.c Fri May 12 04:25:08 2017 +0200 +++ b/src/lib-http/test-http-response-parser.c Tue Jul 11 14:18:47 2017 +0200 @@ -21,6 +21,7 @@ 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 @@ 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 @@ * 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 @@ 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 @@ /* 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); + 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, 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 @@ ret = http_response_parse_next(parser, HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED, &response, &error); test_out("parse end", ret == 0); + http_response_parser_deinit(&parser); test_end(); - http_response_parser_deinit(&parser); } int main(void)