commit f21102c06974a8db9d31819a8caf7f34f267a863 Author: Nick Mathewson nickm@torproject.org Date: Wed Sep 27 21:45:55 2017 -0400
Let's get 100% coverage for proto_http while we're at it.
There was some coverage before, but it was mostly accidental. --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_proto_http.c | 213 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 216 insertions(+)
diff --git a/src/test/include.am b/src/test/include.am index 95efe4e79..4cc2fae4c 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -137,6 +137,7 @@ src_test_test_SOURCES = \ src/test/test_options.c \ src/test/test_policy.c \ src/test/test_procmon.c \ + src/test/test_proto_http.c \ src/test/test_proto_misc.c \ src/test/test_protover.c \ src/test/test_pt.c \ diff --git a/src/test/test.c b/src/test/test.c index 27c776b30..368d874be 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1215,6 +1215,7 @@ struct testgroup_t testgroups[] = { { "options/", options_tests }, { "policy/" , policy_tests }, { "procmon/", procmon_tests }, + { "proto/http/", proto_http_tests }, { "proto/misc/", proto_misc_tests }, { "protover/", protover_tests }, { "pt/", pt_tests }, diff --git a/src/test/test.h b/src/test/test.h index ba7b767c3..634f3e7b0 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -226,6 +226,7 @@ extern struct testcase_t oos_tests[]; extern struct testcase_t options_tests[]; extern struct testcase_t policy_tests[]; extern struct testcase_t procmon_tests[]; +extern struct testcase_t proto_http_tests[]; extern struct testcase_t proto_misc_tests[]; extern struct testcase_t protover_tests[]; extern struct testcase_t pubsub_tests[]; diff --git a/src/test/test_proto_http.c b/src/test/test_proto_http.c new file mode 100644 index 000000000..2f36fbccd --- /dev/null +++ b/src/test/test_proto_http.c @@ -0,0 +1,213 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_proto_http.c + * \brief Tests for our HTTP protocol parser code + */ + +#include "or.h" +#include "test.h" +#include "buffers.h" +#include "proto_http.h" +#include "log_test_helpers.h" + +#define S(str) str, sizeof(str)-1 + +static void +test_proto_http_peek(void *arg) +{ + (void) arg; + const struct { + int is_http; + const char *message; + size_t len; + } cases[] = { + { 1, S("GET /index HTTP/1.0\r\n") }, + { 1, S("GET /index HTTP/1.1\r\n") }, + { 1, S("GET ") }, + { 0, S("GIT ") }, + { 0, S("GET") }, + { 0, S("get ") }, + { 0, S("GETAWAY") }, + }; + unsigned i; + buf_t *buf = buf_new(); + for (i = 0; i < ARRAY_LENGTH(cases); ++i) { + TT_BLATHER(("Trying case %u", i)); + buf_add(buf, cases[i].message, cases[i].len); + tt_int_op(cases[i].is_http, OP_EQ, peek_buf_has_http_command(buf)); + buf_clear(buf); + } + done: + buf_free(buf); +} + +static void +test_proto_http_valid(void *arg) +{ + (void) arg; + const struct { + const char *message; + size_t len; + const char *headers; + const char *body; + size_t bodylen; + int should_detect_truncated; + int bytes_left_over; + } cases[] = { + { S("GET /index.html HTTP/1.0\r\n\r\n"), + "GET /index.html HTTP/1.0\r\n\r\n", + S(""), + 1, 0, + }, + { S("PUT /tor/foo HTTP/1.1\r\n" + "Content-Length: 51\r\n\r\n" + "this is a test of the http parsing system . test te"), + "PUT /tor/foo HTTP/1.1\r\n" "Content-Length: 51\r\n\r\n", + S("this is a test of the http parsing system . test te"), + 1, 0, + }, + { S("PUT /tor/foo HTTP/1.1\r\n" + "Content-Length: 5\r\n\r\n" + "there are more than 5 characters in this body."), + "PUT /tor/foo HTTP/1.1\r\n" "Content-Length: 5\r\n\r\n", + S("there"), + 0, 41, + }, + { S("PUT /tor/bar HTTP/1.1\r\n\r\n" + "this is another \x00test"), + "PUT /tor/bar HTTP/1.1\r\n\r\n", + S("this is another \x00test"), + 0, 0, + } + }; + unsigned i; + buf_t *buf = buf_new(); + char *h = NULL, *b = NULL; + + for (i = 0; i < ARRAY_LENGTH(cases); ++i) { + TT_BLATHER(("Trying case %u", i)); + size_t bl = 0; + // truncate by 2 chars + buf_add(buf, cases[i].message, cases[i].len - 2); + + if (cases[i].should_detect_truncated) { + tt_int_op(0, OP_EQ, fetch_from_buf_http(buf, &h, 1024*16, + &b, &bl, 1024*16, 0)); + tt_ptr_op(h, OP_EQ, NULL); + tt_ptr_op(b, OP_EQ, NULL); + tt_u64_op(bl, OP_EQ, 0); + tt_int_op(buf_datalen(buf), OP_EQ, cases[i].len - 2); + } + + // add the rest. + buf_add(buf, cases[i].message+cases[i].len-2, 2); + tt_int_op(1, OP_EQ, fetch_from_buf_http(buf, &h, 1024*16, + &b, &bl, 1024*16, 0)); + tt_str_op(h, OP_EQ, cases[i].headers); + tt_u64_op(bl, OP_EQ, cases[i].bodylen); + tt_mem_op(b, OP_EQ, cases[i].body, bl); + tt_int_op(buf_datalen(buf), OP_EQ, cases[i].bytes_left_over); + + buf_clear(buf); + tor_free(h); + tor_free(b); + } + done: + tor_free(h); + tor_free(b); + buf_free(buf); +} + +static void +test_proto_http_invalid(void *arg) +{ + (void) arg; + const struct { + const char *message; + size_t len; + const char *expect; + } cases[] = { + /* Overlong headers, headers not finished. */ + { S("GET /index.xhml HTTP/1.0\r\n" + "X-My-headers-are-too-long: yes indeed they are. They might be\r\n" + "X-My-headers-are-too-long: normal under other circumstances, but\r\n" + "X-My-headers-are-too-long: the 128-byte limit makes them bad\r\n"), + "headers too long." }, + /* Overlong finished headers. */ + { S("GET /index.xhml HTTP/1.0\r\n" + "X-My-headers-are-too-long: yes indeed they are. They might be\r\n" + "X-My-headers-are-too-long: normal under other circumstances, but\r\n" + "X-My-headers-are-too-long: the 128-byte limit makes them bad\r\n" + "\r\n"), + "headers too long." }, + /* Exactly too long finished headers. */ + { S("GET /index.xhml HTTP/1.0\r\n" + "X-My-headers-are-too-long: yes indeed they are. They might be\r\n" + "X-My-headers-are-too-long: normal un\r\n\r\n"), + "headerlen 129 larger than 127. Failing." }, + /* Body too long, with content-length */ + { S("GET /index.html HTTP/1.0\r\n" + "Content-Length: 129\r\n\r\n" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxx"), + "bodylen 129 larger than 127" }, + /* Body too long, with content-length lying */ + { S("GET /index.html HTTP/1.0\r\n" + "Content-Length: 99999\r\n\r\n" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxx"), + "bodylen 138 larger than 127" }, + /* Body too long, no content-length. */ + { S("GET /index.html HTTP/1.0\r\n\r\n" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxz"), + "bodylen 139 larger than 127" }, + /* Content-Length is junk. */ + { S("GET /index.html HTTP/1.0\r\n" + "Content-Length: Cheese\r\n\r\n" + "foo"), + "Content-Length is bogus; maybe someone is trying to crash us." }, + }; + unsigned i; + buf_t *buf = buf_new(); + char *h = NULL, *b = NULL; + setup_capture_of_logs(LOG_DEBUG); + + for (i = 0; i < ARRAY_LENGTH(cases); ++i) { + TT_BLATHER(("Trying case %u", i)); + size_t bl = 0; + buf_add(buf, cases[i].message, cases[i].len); + + /* Use low body limits here so we can force over-sized object warnings */ + tt_int_op(-1, OP_EQ, fetch_from_buf_http(buf, &h, 128, + &b, &bl, 128, 0)); + tt_ptr_op(h, OP_EQ, NULL); + tt_ptr_op(b, OP_EQ, NULL); + tt_u64_op(bl, OP_EQ, 0); + expect_log_msg_containing(cases[i].expect); + + buf_clear(buf); + tor_free(h); + tor_free(b); + mock_clean_saved_logs(); + } + done: + tor_free(h); + tor_free(b); + buf_free(buf); + teardown_capture_of_logs(); +} + +struct testcase_t proto_http_tests[] = { + { "peek", test_proto_http_peek, 0, NULL, NULL }, + { "valid", test_proto_http_valid, 0, NULL, NULL }, + { "invalid", test_proto_http_invalid, 0, NULL, NULL }, + + END_OF_TESTCASES +}; +