From 732a9ddb830fab6a215c555cba4c40a3299fe502 Mon Sep 17 00:00:00 2001 From: Raphael Schlarb Date: Mon, 10 Aug 2026 14:29:47 -0500 Subject: [PATCH 1/3] fix: apply handler status code to response for non-error codes - th_http_handle_request_and_write_response only set response->code on client/server error paths, so success/redirect/informational codes returned from a handler were silently dropped in favor of the default 200 OK - add tests covering the TH_HTTP_CODE_TYPE_INFORMATIONAL path (including the HTTP/1.0 downgrade-to-400 behavior) and a handler returning an unrelated system error mapped through th_http_error --- src/th_http.c | 1 + src/th_http_test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/th_http.c b/src/th_http.c index f08f8c9..1d01942 100644 --- a/src/th_http.c +++ b/src/th_http.c @@ -166,6 +166,7 @@ th_http_handle_request_and_write_response(th_http* http) th_response* response = &http->response; th_http_prehandle_request(http); th_err err = th_http_error(th_http_handle_route(http->router, &http->request, &http->response)); + th_response_set_code(response, TH_ERR_CODE(err)); switch (th_http_code_get_type(TH_ERR_CODE(err))) { case TH_HTTP_CODE_TYPE_INFORMATIONAL: if (request->version == 0) { diff --git a/src/th_http_test.c b/src/th_http_test.c index cd2fea0..2040a07 100644 --- a/src/th_http_test.c +++ b/src/th_http_test.c @@ -1,6 +1,7 @@ #include "th_conn_tracker.h" #include "th_fmt.h" #include "th_http.h" +#include "th_system_error.h" #include "th_test.h" #include "th_utility.h" @@ -175,6 +176,24 @@ th_test_handler(void* user_data, const th_request* req, th_response* resp) return TH_ERR_OK; } +static th_err +th_test_informational_handler(void* user_data, const th_request* req, th_response* resp) +{ + (void)user_data; + (void)req; + (void)resp; + return TH_ERR_HTTP(100); // Continue +} + +static th_err +th_test_system_error_handler(void* user_data, const th_request* req, th_response* resp) +{ + (void)user_data; + (void)req; + (void)resp; + return TH_ERR_SYSTEM(TH_ENOENT); +} + TH_TEST_BEGIN(http) { th_conn_tracker tracker; @@ -183,6 +202,8 @@ TH_TEST_BEGIN(http) th_router_init(&router, th_default_allocator_get()); TH_EXPECT(th_router_add_route(&router, TH_METHOD_GET, TH_STR("/test"), th_test_handler, NULL) == TH_ERR_OK); TH_EXPECT(th_router_add_route(&router, TH_METHOD_POST, TH_STR("/test"), th_test_handler, NULL) == TH_ERR_OK); + TH_EXPECT(th_router_add_route(&router, TH_METHOD_GET, TH_STR("/informational"), th_test_informational_handler, NULL) == TH_ERR_OK); + TH_EXPECT(th_router_add_route(&router, TH_METHOD_GET, TH_STR("/system-error"), th_test_system_error_handler, NULL) == TH_ERR_OK); th_http_upgrader upgrader; th_http_upgrader_init(&upgrader, &tracker, &router, NULL, NULL, th_default_allocator_get()); th_fake_conn conn; @@ -408,6 +429,46 @@ TH_TEST_BEGIN(http) TH_EXPECT(conn.destroyed); } TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(http_writes_informational_response_for_1_1) + { + th_fake_conn_set_request(&conn, TH_STR("GET /informational HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n")); + + th_conn_upgrader_upgrade(&upgrader.base, &conn.base); + while (!conn.destroyed && conn.callback != NULL) + th_fake_conn_run(&conn); + + TH_EXPECT(th_buf_starts_with(conn.written, conn.written_len, "HTTP/1.1 100 ")); + TH_EXPECT(conn.destroyed); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(http_rejects_informational_response_for_1_0) + { + // HTTP/1.0 clients can't handle 1xx responses, so this is + // downgraded to a 400 Bad Request instead. + th_fake_conn_set_request(&conn, TH_STR("GET /informational HTTP/1.0\r\nHost: example.com\r\nConnection: close\r\n\r\n")); + + th_conn_upgrader_upgrade(&upgrader.base, &conn.base); + while (!conn.destroyed && conn.callback != NULL) + th_fake_conn_run(&conn); + + TH_EXPECT(th_buf_starts_with(conn.written, conn.written_len, "HTTP/1.1 400 Bad Request\r\n")); + TH_EXPECT(conn.destroyed); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(http_maps_unrelated_system_error_to_404) + { + // A handler returning a plain system error (not an HTTP error) gets + // translated by th_http_error, e.g. ENOENT maps to 404. + th_fake_conn_set_request(&conn, TH_STR("GET /system-error HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n")); + + th_conn_upgrader_upgrade(&upgrader.base, &conn.base); + while (!conn.destroyed && conn.callback != NULL) + th_fake_conn_run(&conn); + + TH_EXPECT(th_buf_starts_with(conn.written, conn.written_len, "HTTP/1.1 404 Not Found\r\n")); + TH_EXPECT(conn.destroyed); + } + TH_TEST_CASE_END th_router_deinit(&router); th_conn_tracker_deinit(&tracker); From 881c0610b0d2d64c2e507594f0b213fd17e32dc8 Mon Sep 17 00:00:00 2001 From: Raphael Schlarb Date: Mon, 10 Aug 2026 14:55:15 -0500 Subject: [PATCH 2/3] test: add th_request unit tests, fix prefix false-match in th_find_* - add src/th_request_test.c covering setters/getters, find/iterate for headers, cookies, query/form/path vars and multipart parts, url-decode behavior, and reset - fix th_find_header/th_find_cookie/th_find_queryvar/th_find_formvar/ th_find_pathvar/th_find_part: strncmp used the stored key's length, so a stored key that's a prefix of the search term (e.g. "Ho" vs "Host") incorrectly matched; switched to strcmp --- CMakeLists.txt | 1 + src/th_request.c | 13 +-- src/th_request_test.c | 262 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 269 insertions(+), 7 deletions(-) create mode 100644 src/th_request_test.c diff --git a/CMakeLists.txt b/CMakeLists.txt index bae587b..4011919 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,6 +231,7 @@ if (NOT TH_DISABLE_TESTS) src/th_tcp_conn_test.c src/th_router_test.c src/th_allocator_test.c + src/th_request_test.c src/th_request_parser_test.c src/th_cookie_parser_test.c src/th_multipart_parser_test.c diff --git a/src/th_request.c b/src/th_request.c index 8df2ffa..9685b9d 100644 --- a/src/th_request.c +++ b/src/th_request.c @@ -354,7 +354,7 @@ th_find_header(const th_request* req, const char* key) { size_t num = th_hstr_vec_size(&req->headers); for (size_t i = 0; i < num; i++) { - if (strncmp(key, th_string_data(&req->headers.data[i].key), th_string_len(&req->headers.data[i].key)) == 0) { + if (strcmp(key, th_string_data(&req->headers.data[i].key)) == 0) { return th_string_data(&req->headers.data[i].value); } } @@ -376,7 +376,7 @@ th_find_cookie(const th_request* req, const char* key) { size_t num = th_hstr_vec_size(&req->cookies); for (size_t i = 0; i < num; i++) { - if (strncmp(key, th_string_data(&req->cookies.data[i].key), th_string_len(&req->cookies.data[i].key)) == 0) { + if (strcmp(key, th_string_data(&req->cookies.data[i].key)) == 0) { return th_string_data(&req->cookies.data[i].value); } } @@ -398,7 +398,7 @@ th_find_queryvar(const th_request* req, const char* key) { size_t num = th_hstr_vec_size(&req->queryvars); for (size_t i = 0; i < num; i++) { - if (strncmp(key, th_string_data(&req->queryvars.data[i].key), th_string_len(&req->queryvars.data[i].key)) == 0) { + if (strcmp(key, th_string_data(&req->queryvars.data[i].key)) == 0) { return th_string_data(&req->queryvars.data[i].value); } } @@ -420,7 +420,7 @@ th_find_formvar(const th_request* req, const char* key) { size_t num = th_hstr_vec_size(&req->formvars); for (size_t i = 0; i < num; i++) { - if (strncmp(key, th_string_data(&req->formvars.data[i].key), th_string_len(&req->formvars.data[i].key)) == 0) { + if (strcmp(key, th_string_data(&req->formvars.data[i].key)) == 0) { return th_string_data(&req->formvars.data[i].value); } } @@ -442,7 +442,7 @@ th_find_pathvar(const th_request* req, const char* key) { size_t num = th_hstr_vec_size(&req->pathvars); for (size_t i = 0; i < num; i++) { - if (strncmp(key, th_string_data(&req->pathvars.data[i].key), th_string_len(&req->pathvars.data[i].key)) == 0) { + if (strcmp(key, th_string_data(&req->pathvars.data[i].key)) == 0) { return th_string_data(&req->pathvars.data[i].value); } } @@ -464,8 +464,7 @@ th_find_part(const th_request* req, const char* name) { size_t num = th_part_vec_size(&req->parts); for (size_t i = 0; i < num; i++) { - if (strncmp(name, th_string_data(&req->parts.data[i].name), th_string_len(&req->parts.data[i].name)) - == 0) { + if (strcmp(name, th_string_data(&req->parts.data[i].name)) == 0) { return th_part_vec_cat(&req->parts, i); } } diff --git a/src/th_request_test.c b/src/th_request_test.c new file mode 100644 index 0000000..ff84394 --- /dev/null +++ b/src/th_request_test.c @@ -0,0 +1,262 @@ +#include "th_request.h" +#include "th_test.h" + +#include + +TH_TEST_BEGIN(request) +{ + th_request request; + th_request_init(&request, th_default_allocator_get()); + + TH_TEST_CASE_BEGIN(request_init_is_empty) + { + TH_EXPECT(strcmp(th_get_path(&request), "") == 0); + TH_EXPECT(strcmp(th_get_query(&request), "") == 0); + TH_EXPECT(th_get_body(&request).len == 0); + TH_EXPECT(th_find_header(&request, "Host") == NULL); + TH_EXPECT(th_find_cookie(&request, "session") == NULL); + TH_EXPECT(th_find_queryvar(&request, "q") == NULL); + TH_EXPECT(th_find_formvar(&request, "f") == NULL); + TH_EXPECT(th_find_pathvar(&request, "id") == NULL); + TH_EXPECT(th_find_part(&request, "file") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_set_uri_path_and_query) + { + TH_EXPECT(th_request_set_uri_path(&request, TH_STR("/a/b")) == TH_ERR_OK); + TH_EXPECT(th_request_set_uri_query(&request, TH_STR("x=1")) == TH_ERR_OK); + TH_EXPECT(strcmp(th_get_path(&request), "/a/b") == 0); + TH_EXPECT(strcmp(th_get_query(&request), "x=1") == 0); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_set_method_and_version) + { + th_request_set_method(&request, TH_METHOD_POST); + th_request_set_version(&request, TH_HTTP_1_0); + TH_EXPECT(th_get_method(&request) == TH_METHOD_POST); + TH_EXPECT(th_get_version(&request) == TH_HTTP_1_0); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_set_body) + { + th_request_set_body(&request, TH_STR("hello")); + th_buffer body = th_get_body(&request); + TH_EXPECT(body.len == 5); + TH_EXPECT(memcmp(body.ptr, "hello", 5) == 0); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_header) + { + TH_EXPECT(th_request_add_header(&request, TH_STR("Host"), TH_STR("example.com")) == TH_ERR_OK); + const char* value = th_find_header(&request, "Host"); + TH_EXPECT(value != NULL); + TH_EXPECT(strcmp(value, "example.com") == 0); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_find_header_missing_returns_null) + { + TH_EXPECT(th_request_add_header(&request, TH_STR("Host"), TH_STR("example.com")) == TH_ERR_OK); + TH_EXPECT(th_find_header(&request, "Accept") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_find_header_does_not_match_key_that_is_only_a_prefix_of_the_search_term) + { + TH_EXPECT(th_request_add_header(&request, TH_STR("Ho"), TH_STR("wrong")) == TH_ERR_OK); + TH_EXPECT(th_find_header(&request, "Host") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_header_iter_visits_all_headers) + { + TH_EXPECT(th_request_add_header(&request, TH_STR("Host"), TH_STR("example.com")) == TH_ERR_OK); + TH_EXPECT(th_request_add_header(&request, TH_STR("Accept"), TH_STR("*/*")) == TH_ERR_OK); + + th_iter it = th_header_iter(&request); + size_t count = 0; + bool found_host = false; + bool found_accept = false; + do { + count++; + if (strcmp(th_key(&it), "Host") == 0 && strcmp(th_cval(&it), "example.com") == 0) + found_host = true; + if (strcmp(th_key(&it), "Accept") == 0 && strcmp(th_cval(&it), "*/*") == 0) + found_accept = true; + } while (th_next(&it)); + + TH_EXPECT(count == 2); + TH_EXPECT(found_host); + TH_EXPECT(found_accept); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_header_iter_on_empty_request_yields_no_elements) + { + th_iter it = th_header_iter(&request); + TH_EXPECT(it.ptr == it.end); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_cookie) + { + TH_EXPECT(th_request_add_cookie(&request, TH_STR("session"), TH_STR("abc123")) == TH_ERR_OK); + const char* value = th_find_cookie(&request, "session"); + TH_EXPECT(value != NULL); + TH_EXPECT(strcmp(value, "abc123") == 0); + TH_EXPECT(th_find_cookie(&request, "other") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_cookie_iter_visits_all_cookies) + { + TH_EXPECT(th_request_add_cookie(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_cookie(&request, TH_STR("b"), TH_STR("2")) == TH_ERR_OK); + + th_iter it = th_cookie_iter(&request); + size_t count = 0; + do { + count++; + } while (th_next(&it)); + TH_EXPECT(count == 2); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_queryvar_url_decodes) + { + // th_request_add_queryvar url-decodes both key and value. + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("na%20me"), TH_STR("john+doe")) == TH_ERR_OK); + const char* value = th_find_queryvar(&request, "na me"); + TH_EXPECT(value != NULL); + TH_EXPECT(strcmp(value, "john doe") == 0); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_queryvar_rejects_invalid_percent_encoding) + { + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("key"), TH_STR("bad%")) != TH_ERR_OK); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_queryvar_iter_visits_all_queryvars) + { + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("b"), TH_STR("2")) == TH_ERR_OK); + + th_iter it = th_queryvar_iter(&request); + size_t count = 0; + do { + count++; + } while (th_next(&it)); + TH_EXPECT(count == 2); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_clear_queryvars_removes_all) + { + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + th_request_clear_queryvars(&request); + TH_EXPECT(th_find_queryvar(&request, "a") == NULL); + th_iter it = th_queryvar_iter(&request); + TH_EXPECT(it.ptr == it.end); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_formvar_url_decodes) + { + TH_EXPECT(th_request_add_formvar(&request, TH_STR("a+b"), TH_STR("c%20d")) == TH_ERR_OK); + const char* value = th_find_formvar(&request, "a b"); + TH_EXPECT(value != NULL); + TH_EXPECT(strcmp(value, "c d") == 0); + TH_EXPECT(th_find_formvar(&request, "missing") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_formvar_iter_visits_all_formvars) + { + TH_EXPECT(th_request_add_formvar(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_formvar(&request, TH_STR("b"), TH_STR("2")) == TH_ERR_OK); + + th_iter it = th_formvar_iter(&request); + size_t count = 0; + do { + count++; + } while (th_next(&it)); + TH_EXPECT(count == 2); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_pathvar_does_not_url_decode) + { + // Unlike query/form vars, pathvars are stored verbatim. + TH_EXPECT(th_request_add_pathvar(&request, TH_STR("id"), TH_STR("a%20b")) == TH_ERR_OK); + const char* value = th_find_pathvar(&request, "id"); + TH_EXPECT(value != NULL); + TH_EXPECT(strcmp(value, "a%20b") == 0); + TH_EXPECT(th_find_pathvar(&request, "missing") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_pathvar_iter_visits_all_pathvars) + { + TH_EXPECT(th_request_add_pathvar(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_pathvar(&request, TH_STR("b"), TH_STR("2")) == TH_ERR_OK); + + th_iter it = th_pathvar_iter(&request); + size_t count = 0; + do { + count++; + } while (th_next(&it)); + TH_EXPECT(count == 2); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_add_and_find_part) + { + TH_EXPECT( + th_request_add_part(&request, TH_STR("file content"), TH_STR("upload"), TH_STR("a.txt"), TH_STR("text/plain")) + == TH_ERR_OK); + + const th_part* part = th_find_part(&request, "upload"); + TH_EXPECT(part != NULL); + TH_EXPECT(strcmp(th_part_name(part), "upload") == 0); + TH_EXPECT(strcmp(th_part_filename(part), "a.txt") == 0); + TH_EXPECT(strcmp(th_part_content_type(part), "text/plain") == 0); + th_buffer content = th_part_content(part); + TH_EXPECT(content.len == 12); + TH_EXPECT(memcmp(content.ptr, "file content", 12) == 0); + TH_EXPECT(th_find_part(&request, "missing") == NULL); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_part_iter_visits_all_parts) + { + TH_EXPECT(th_request_add_part(&request, TH_STR("a"), TH_STR("one"), TH_STR(""), TH_STR("")) == TH_ERR_OK); + TH_EXPECT(th_request_add_part(&request, TH_STR("b"), TH_STR("two"), TH_STR(""), TH_STR("")) == TH_ERR_OK); + + th_iter it = th_part_iter(&request); + size_t count = 0; + do { + count++; + } while (th_next(&it)); + TH_EXPECT(count == 2); + } + TH_TEST_CASE_END + TH_TEST_CASE_BEGIN(request_reset_clears_everything) + { + TH_EXPECT(th_request_set_uri_path(&request, TH_STR("/a")) == TH_ERR_OK); + TH_EXPECT(th_request_set_uri_query(&request, TH_STR("x=1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_header(&request, TH_STR("Host"), TH_STR("example.com")) == TH_ERR_OK); + TH_EXPECT(th_request_add_cookie(&request, TH_STR("session"), TH_STR("abc")) == TH_ERR_OK); + TH_EXPECT(th_request_add_queryvar(&request, TH_STR("a"), TH_STR("1")) == TH_ERR_OK); + TH_EXPECT(th_request_add_formvar(&request, TH_STR("b"), TH_STR("2")) == TH_ERR_OK); + TH_EXPECT(th_request_add_pathvar(&request, TH_STR("c"), TH_STR("3")) == TH_ERR_OK); + TH_EXPECT(th_request_add_part(&request, TH_STR("data"), TH_STR("f"), TH_STR(""), TH_STR("")) == TH_ERR_OK); + th_request_set_body(&request, TH_STR("body")); + th_request_set_method(&request, TH_METHOD_POST); + th_request_set_version(&request, TH_HTTP_1_1); + request.close = true; + + th_request_reset(&request); + + TH_EXPECT(strcmp(th_get_path(&request), "") == 0); + TH_EXPECT(strcmp(th_get_query(&request), "") == 0); + TH_EXPECT(th_get_body(&request).len == 0); + TH_EXPECT(th_get_version(&request) == TH_HTTP_1_0); + TH_EXPECT(request.close == false); + TH_EXPECT(th_find_header(&request, "Host") == NULL); + TH_EXPECT(th_find_cookie(&request, "session") == NULL); + TH_EXPECT(th_find_queryvar(&request, "a") == NULL); + TH_EXPECT(th_find_formvar(&request, "b") == NULL); + TH_EXPECT(th_find_pathvar(&request, "c") == NULL); + TH_EXPECT(th_find_part(&request, "f") == NULL); + } + TH_TEST_CASE_END + + th_request_deinit(&request); +} +TH_TEST_END From 00d680c2e3a8b860286f180399d8f37e88fc2a84 Mon Sep 17 00:00:00 2001 From: Raphael Schlarb Date: Mon, 10 Aug 2026 15:00:00 -0500 Subject: [PATCH 3/3] style: clean up TH_EXPECT failure message --- src/th_test.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/th_test.h b/src/th_test.h index 13c7a00..e344919 100644 --- a/src/th_test.h +++ b/src/th_test.h @@ -10,10 +10,10 @@ typedef enum { TH_TEST_FAILURE = -1, } th_test_result; -#define TH_EXPECT(x) \ - if ((x) == 0) { \ - printf("Test failed: %s, at %s:%d\n", #x, __FILE__, __LINE__); \ - return TH_TEST_FAILURE; \ +#define TH_EXPECT(x) \ + if ((x) == 0) { \ + printf(" failed\n Case %s failed at %s:%d\n", #x, __FILE__, __LINE__); \ + return TH_TEST_FAILURE; \ } /** th_test_setup