Skip to content

Commit 37d2f93

Browse files
etrclaude
andcommitted
Add regression unit test for null key in post_iterator (#375)
White-box unit test that drives webserver::post_iterator directly with a null key on both the continuation (off > 0) and initial chunks. Without the guard the test process terminates (std::logic_error from std::string(nullptr) escaping the callback); with it the chunk is accepted and skipped. Adds happy-path coverage (initial store and large-field continuation append) to pin that the guard does not regress normal form handling. post_iterator and http_request's constructor / setters are private, so the test uses the standard private->public white-box include technique (standard + libmicrohttpd headers are pulled in first so only libhttpserver's own declarations are affected; member order is unchanged, so object layout matches the normally-compiled library). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rkuh4aSmrD8m2f2vYqakb6
1 parent 4d48e0d commit 37d2f93

2 files changed

Lines changed: 129 additions & 1 deletion

File tree

test/Makefile.am

Lines changed: 9 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 http_response create_webserver new_response_types daemon_info uri_log
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 new_response_types daemon_info uri_log post_iterator_null_key
3030

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

@@ -51,6 +51,14 @@ uri_log_SOURCES = unit/uri_log_test.cpp
5151
# it needs an explicit -lmicrohttpd in its link line on top of the default
5252
# LDADD (modern ld enforces --no-copy-dt-needed-entries).
5353
uri_log_LDADD = $(LDADD) -lmicrohttpd
54+
# post_iterator_null_key: issue #375 regression. Drives the static
55+
# webserver::post_iterator callback with a null key (the continuation-chunk
56+
# case MHD hits when a field is split across callbacks) and asserts it no
57+
# longer throws std::logic_error / terminates. Constructs an http_request
58+
# and modded_request directly, so it needs -lmicrohttpd the same way
59+
# uri_log does.
60+
post_iterator_null_key_SOURCES = unit/post_iterator_null_key_test.cpp
61+
post_iterator_null_key_LDADD = $(LDADD) -lmicrohttpd
5462

5563
noinst_HEADERS = littletest.hpp
5664
AM_CXXFLAGS += -Wall -fPIC -Wno-overloaded-virtual
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2019 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
// White-box unit test for issue #375. webserver::post_iterator (the MHD
22+
// post-processor callback) and http_request's constructor / set_arg are
23+
// private, so this TU drives them directly to exercise the exact code path
24+
// MHD hits. The standard-library and libmicrohttpd headers are pulled in
25+
// first, normally; the `private -> public` redefinition that follows then
26+
// only affects libhttpserver's own class declarations. Member order is
27+
// unchanged, so object layout matches the normally-compiled library
28+
// (only access labels differ).
29+
#include <map>
30+
#include <memory>
31+
#include <string>
32+
#include <vector>
33+
#include <microhttpd.h>
34+
35+
#define private public
36+
#include "./httpserver.hpp"
37+
#include "httpserver/details/modded_request.hpp"
38+
#undef private
39+
40+
#include "./littletest.hpp"
41+
42+
namespace {
43+
// Build a modded_request owning a fresh http_request (its arg cache is
44+
// default-initialised, so set_arg / grow_last_arg / get_arg work without a
45+
// live MHD connection). The no-file form-arg branch never reads mr.ws, so
46+
// it is left null.
47+
httpserver::details::modded_request make_request() {
48+
httpserver::details::modded_request mr;
49+
mr.dhr = std::unique_ptr<httpserver::http_request>(
50+
new httpserver::http_request());
51+
return mr;
52+
}
53+
54+
MHD_Result feed(httpserver::details::modded_request* mr, const char* key,
55+
const char* data, uint64_t off, size_t size) {
56+
return httpserver::webserver::post_iterator(
57+
mr, MHD_POSTDATA_KIND, key, /*filename=*/nullptr,
58+
/*content_type=*/nullptr, /*transfer_encoding=*/nullptr,
59+
data, off, size);
60+
}
61+
} // namespace
62+
63+
LT_BEGIN_SUITE(post_iterator_null_key_suite)
64+
void set_up() {
65+
}
66+
67+
void tear_down() {
68+
}
69+
LT_END_SUITE(post_iterator_null_key_suite)
70+
71+
// Regression test for issue #375: MHD may invoke the post iterator with a
72+
// null key on a continuation chunk (off > 0) because the field name was
73+
// only supplied on the first call. The previous implementation passed the
74+
// raw key pointer into std::string, which throws std::logic_error on null
75+
// and aborts the process via std::terminate (the throw escapes a C
76+
// callback). The guard must instead accept and silently skip the chunk.
77+
LT_BEGIN_AUTO_TEST(post_iterator_null_key_suite, null_key_continuation_does_not_throw)
78+
httpserver::details::modded_request mr = make_request();
79+
MHD_Result r = MHD_NO;
80+
LT_CHECK_NOTHROW(r = feed(&mr, /*key=*/nullptr, "value", /*off=*/5, 5));
81+
// MHD_YES keeps the request alive; MHD_NO would abort it.
82+
LT_CHECK_EQ(r, MHD_YES);
83+
// Nothing was stored: there was no field name to key the value under.
84+
LT_CHECK_EQ(mr.dhr->get_args().size(), static_cast<size_t>(0));
85+
LT_END_AUTO_TEST(null_key_continuation_does_not_throw)
86+
87+
// Same guard on the initial-chunk path (off == 0). MHD should not normally
88+
// hand us a null key here, but the guard is unconditional, so pin it.
89+
LT_BEGIN_AUTO_TEST(post_iterator_null_key_suite, null_key_initial_does_not_throw)
90+
httpserver::details::modded_request mr = make_request();
91+
MHD_Result r = MHD_NO;
92+
LT_CHECK_NOTHROW(r = feed(&mr, /*key=*/nullptr, "value", /*off=*/0, 5));
93+
LT_CHECK_EQ(r, MHD_YES);
94+
LT_CHECK_EQ(mr.dhr->get_args().size(), static_cast<size_t>(0));
95+
LT_END_AUTO_TEST(null_key_initial_does_not_throw)
96+
97+
// Happy path: a non-null key on the initial chunk stores the value under
98+
// that field name, proving the guard did not regress normal form handling.
99+
LT_BEGIN_AUTO_TEST(post_iterator_null_key_suite, valid_key_stores_arg)
100+
httpserver::details::modded_request mr = make_request();
101+
MHD_Result r = feed(&mr, "field", "value", /*off=*/0, 5);
102+
LT_CHECK_EQ(r, MHD_YES);
103+
LT_CHECK_EQ(std::string(mr.dhr->get_arg_flat("field")),
104+
std::string("value"));
105+
LT_END_AUTO_TEST(valid_key_stores_arg)
106+
107+
// Continuation chunk with a (repeated) non-null key appends to the value
108+
// MHD started on the first call - the legitimate large-field split that
109+
// commit 1b5fe8f (issue #337) introduced grow_last_arg to handle.
110+
LT_BEGIN_AUTO_TEST(post_iterator_null_key_suite, valid_key_continuation_appends)
111+
httpserver::details::modded_request mr = make_request();
112+
LT_CHECK_EQ(feed(&mr, "field", "hel", /*off=*/0, 3), MHD_YES);
113+
LT_CHECK_EQ(feed(&mr, "field", "lo", /*off=*/3, 2), MHD_YES);
114+
LT_CHECK_EQ(std::string(mr.dhr->get_arg_flat("field")),
115+
std::string("hello"));
116+
LT_END_AUTO_TEST(valid_key_continuation_appends)
117+
118+
LT_BEGIN_AUTO_TEST_ENV()
119+
AUTORUN_TESTS()
120+
LT_END_AUTO_TEST_ENV()

0 commit comments

Comments
 (0)