Skip to content

Commit 3be9e7c

Browse files
authored
Add tests for untested HTTP methods and request/response getters (#362)
* Add tests for untested HTTP methods and request/response getters - Add HEAD, OPTIONS, TRACE method handlers to complete_test_resource - Add integration tests for HEAD, OPTIONS, TRACE HTTP methods - Add request_info_resource and test for get_requestor(), get_requestor_port(), get_version() - Add content_limit_suite to test content_too_large() with content_size_limit - Add unregister_then_404 test for webserver::unregister_resource() - Create http_response unit test file with tests for response code, headers, footers, and cookies These tests increase coverage for previously untested code paths in webserver.cpp, http_request.cpp, and http_response.cpp. * Add additional coverage tests for networking and TLS features - Add IPv6 and dual-stack webserver tests - Add HTTPS webserver and TLS session getter tests with graceful failure handling - Add bind_address_ipv4 test with runtime URL building - Add shoutcast response test for MHD_ICY_FLAG verification - Add string_response constructor tests - Remove CONNECT method test (incompatible with curl's tunneling semantics) Coverage improved from ~90% to 91.8% lines, 93.3% functions. * Add print-summary to gcovr for coverage debugging Add --print-summary flag to gcovr command in CI to help diagnose the discrepancy between local coverage (91.8%) and Codecov (64.53%). * Filter gcovr to src/ directory only The coverage report was including 15,781 lines from system headers and dependencies instead of just the ~1,500 lines of libhttpserver source. Use --filter to restrict coverage to src/ directory only. * Add coverage tests for footers, PSK handlers, and ban/allow weights - Add footer/trailer tests for http_request and http_response - Add PSK handler configuration tests (setup, empty handler, no handler) - Add ban/allow IP weight comparison tests for specific-then-wildcard case - Add auth skip path tests for root path and wildcards - Add IPv6 parsing edge case tests (too many parts, wildcards, nested) - Add http_endpoint invalid regex pattern test - Add custom error handler tests (not_found, method_not_allowed) - Add request info caching tests Branch coverage: 60.4% -> 64.2% (+65 branches) Line coverage: 91.8% -> 94.9% * Add Phase 2 branch coverage tests for http_request and http_endpoint - Add null value query parameter test covering nullptr branches in build_request_args and build_request_querystring (lines 234, 248) - Add digested user caching tests for cache hit and nullptr branches (lines 293-295, 300) in http_request.cpp - Add caret prefix URL pattern tests covering line 85 in http_endpoint.cpp - Add consecutive slashes URL test covering empty parts handling (line 83) Branch coverage improvements: - http_endpoint.cpp: 66.9% -> 68.7% - http_request.cpp: 58.5% -> 59.7% * Add Phase 3 branch coverage tests for render methods and error handling - Add tests for all HTTP methods falling through to base render() - Add custom internal error resource handler test - Add get_arg_flat fallback test - Add large multipart form field test (100KB) - Add file upload with explicit content-type header test - Add http_endpoint tests for regex validation and error paths - Add tests for invalid URL parameter formats * Fix cpplint issues in test files - Replace static global strings with function-static pattern - Change 'long' to 'int64_t' for http_code variables - Add missing #include <utility> for std::move * Added PSK tests * Fix Windows test failure and add hex utility coverage - Fix file_upload tests to use subdirectory instead of /tmp for cross-platform compatibility - Extract hex validation functions (is_valid_hex, hex_char_to_val) from webserver.cpp to string_utilities for testability - Add unit tests for hex utility functions to improve coverage * Fix Windows mkdir compatibility issue On Windows/MinGW, mkdir() only takes one argument (path), while POSIX mkdir() takes two (path and mode). Add a MKDIR macro that uses the appropriate signature for each platform.
1 parent 77b089a commit 3be9e7c

17 files changed

+4735
-22
lines changed

.github/workflows/verify-build.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ jobs:
470470

471471
- name: Setup gnutls dependency (only on linux)
472472
run: |
473-
sudo apt-get install libgnutls28-dev ;
473+
sudo apt-get install libgnutls28-dev gnutls-bin ;
474474
if: ${{ matrix.os-type == 'ubuntu' }}
475475

476476
- name: Fetch libmicrohttpd from cache
@@ -668,7 +668,7 @@ jobs:
668668
- name: Generate coverage report
669669
run: |
670670
cd build
671-
gcovr --root .. --exclude test/ --xml coverage.xml --xml-pretty
671+
gcovr --root .. --filter '../src/' --xml coverage.xml --xml-pretty --print-summary
672672
if: ${{ matrix.os-type == 'ubuntu' && matrix.c-compiler == 'gcc' && matrix.debug == 'debug' && matrix.coverage == 'coverage' && success() }}
673673

674674
- name: Upload coverage to Codecov

src/httpserver/string_utilities.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ const std::string to_upper_copy(const std::string& str);
4242
const std::string to_lower_copy(const std::string& str);
4343
const std::vector<std::string> string_split(const std::string& s, char sep = ' ', bool collapse = true);
4444

45+
/**
46+
* Validate that a string contains only valid hexadecimal characters (0-9, a-f, A-F)
47+
* @param s The string to validate
48+
* @return true if string contains only valid hex characters, false otherwise
49+
*/
50+
bool is_valid_hex(const std::string& s);
51+
52+
/**
53+
* Convert a hex character to its numeric value (0-15)
54+
* @param c The hex character to convert
55+
* @return numeric value (0-15), or 0 for invalid characters
56+
*/
57+
unsigned char hex_char_to_val(char c);
58+
4559
} // namespace string_utilities
4660

4761
} // namespace httpserver

src/httpserver/webserver.hpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,12 @@ class webserver {
227227
MHD_Result complete_request(MHD_Connection* connection, struct details::modded_request* mr, const char* version, const char* method);
228228

229229
#ifdef HAVE_GNUTLS
230-
static int psk_cred_handler_func(gnutls_session_t session,
230+
// MHD_PskServerCredentialsCallback signature
231+
static int psk_cred_handler_func(void* cls,
232+
struct MHD_Connection* connection,
231233
const char* username,
232-
gnutls_datum_t* key);
234+
void** psk,
235+
size_t* psk_size);
233236
#endif // HAVE_GNUTLS
234237

235238
friend MHD_Result policy_callback(void *cls, const struct sockaddr* addr, socklen_t addrlen);

src/string_utilities.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,22 @@ const std::vector<std::string> string_split(const std::string& s, char sep, bool
5555
return result;
5656
}
5757

58+
bool is_valid_hex(const std::string& s) {
59+
for (char c : s) {
60+
if (!((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') ||
61+
(c >= 'A' && c <= 'F'))) {
62+
return false;
63+
}
64+
}
65+
return true;
66+
}
67+
68+
unsigned char hex_char_to_val(char c) {
69+
if (c >= '0' && c <= '9') return static_cast<unsigned char>(c - '0');
70+
if (c >= 'a' && c <= 'f') return static_cast<unsigned char>(c - 'a' + 10);
71+
if (c >= 'A' && c <= 'F') return static_cast<unsigned char>(c - 'A' + 10);
72+
return 0;
73+
}
74+
5875
} // namespace string_utilities
5976
} // namespace httpserver

src/webserver.cpp

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,17 @@
5858
#include "httpserver/http_resource.hpp"
5959
#include "httpserver/http_response.hpp"
6060
#include "httpserver/http_utils.hpp"
61+
#include "httpserver/string_utilities.hpp"
6162
#include "httpserver/string_response.hpp"
6263

6364
struct MHD_Connection;
6465

6566
#define _REENTRANT 1
6667

68+
#ifdef HAVE_GNUTLS
69+
#include <gnutls/gnutls.h>
70+
#endif // HAVE_GNUTLS
71+
6772
#ifndef SOCK_CLOEXEC
6873
#define SOCK_CLOEXEC 02000000
6974
#endif
@@ -425,11 +430,22 @@ void webserver::disallow_ip(const string& ip) {
425430
}
426431

427432
#ifdef HAVE_GNUTLS
428-
int webserver::psk_cred_handler_func(gnutls_session_t session,
433+
// MHD_PskServerCredentialsCallback signature:
434+
// The 'cls' parameter is our webserver pointer (passed via MHD_OPTION)
435+
// Returns 0 on success, -1 on error
436+
// The psk output should be allocated with malloc() - MHD will free it
437+
int webserver::psk_cred_handler_func(void* cls,
438+
struct MHD_Connection* connection,
429439
const char* username,
430-
gnutls_datum_t* key) {
431-
webserver* ws = static_cast<webserver*>(
432-
gnutls_session_get_ptr(session));
440+
void** psk,
441+
size_t* psk_size) {
442+
std::ignore = connection; // Not needed - we get context from cls
443+
444+
webserver* ws = static_cast<webserver*>(cls);
445+
446+
// Initialize output to safe values
447+
*psk = nullptr;
448+
*psk_size = 0;
433449

434450
if (ws == nullptr || ws->psk_cred_handler == nullptr) {
435451
return -1;
@@ -440,23 +456,28 @@ int webserver::psk_cred_handler_func(gnutls_session_t session,
440456
return -1;
441457
}
442458

443-
// Convert hex string to binary
459+
// Validate hex string before allocating memory
444460
size_t psk_len = psk_hex.size() / 2;
445-
key->data = static_cast<unsigned char*>(gnutls_malloc(psk_len));
446-
if (key->data == nullptr) {
461+
if (psk_len == 0 || (psk_hex.size() % 2 != 0) ||
462+
!string_utilities::is_valid_hex(psk_hex)) {
447463
return -1;
448464
}
449465

450-
size_t output_size = psk_len;
451-
int ret = gnutls_hex2bin(psk_hex.c_str(), psk_hex.size(),
452-
key->data, &output_size);
453-
if (ret < 0) {
454-
gnutls_free(key->data);
455-
key->data = nullptr;
466+
// Allocate with malloc - MHD will free this
467+
unsigned char* psk_data = static_cast<unsigned char*>(malloc(psk_len));
468+
if (psk_data == nullptr) {
456469
return -1;
457470
}
458471

459-
key->size = static_cast<unsigned int>(output_size);
472+
// Convert hex string to binary
473+
for (size_t i = 0; i < psk_len; i++) {
474+
psk_data[i] = static_cast<unsigned char>(
475+
(string_utilities::hex_char_to_val(psk_hex[i * 2]) << 4) |
476+
string_utilities::hex_char_to_val(psk_hex[i * 2 + 1]));
477+
}
478+
479+
*psk = psk_data;
480+
*psk_size = psk_len;
460481
return 0;
461482
}
462483
#endif // HAVE_GNUTLS

test/Makefile.am

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ LDADD += -lcurl
2626

2727
AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/
2828
METASOURCES = AUTO
29-
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource
29+
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication deferred http_resource http_response create_webserver
3030

3131
MOSTLYCLEANFILES = *.gcda *.gcno *.gcov
3232

@@ -42,6 +42,8 @@ string_utilities_SOURCES = unit/string_utilities_test.cpp
4242
http_endpoint_SOURCES = unit/http_endpoint_test.cpp
4343
nodelay_SOURCES = integ/nodelay.cpp
4444
http_resource_SOURCES = unit/http_resource_test.cpp
45+
http_response_SOURCES = unit/http_response_test.cpp
46+
create_webserver_SOURCES = unit/create_webserver_test.cpp
4547

4648
noinst_HEADERS = littletest.hpp
4749
AM_CXXFLAGS += -Wall -fPIC -Wno-overloaded-virtual

0 commit comments

Comments
 (0)