Skip to content

Commit 86f2889

Browse files
etrclaude
andcommitted
TASK-081: Fill empty-on-correct-build unit suites; delete pthread detector
Six action items, six cycles: 1. New webserver_ws_available_test.cpp: HAVE_WEBSOCKET-on companion to webserver_ws_unavailable_test (which is empty on classic / flag-invariance-on lanes). Pins features().websocket==true and the throw-type contrast: null unique_ptr/shared_ptr → invalid_argument (NOT feature_unavailable). Body is empty on HAVE_WEBSOCKET-off lanes where the off-path contracts live in the unavailable TU. 2. New webserver_dauth_available_test.cpp: same paired pattern for HAVE_DAUTH. Pins features().digest_auth==true and that digest_auth(true)/digest_auth(false) construct cleanly. Empty body on HAVE_DAUTH-off (flag-invariance-off + Windows). 3. webserver_register_ws_smartptr_test: added HAVE_WEBSOCKET-off runtime block (was previously runtime-empty on off lanes — only the compile-time signature asserts fired). Pins features().websocket ==false and null unique_ptr → feature_unavailable (not invalid_argument — the throw-type contrast against the HAVE_WEBSOCKET-on contract). 4. http_request_operator_stream_test: split credential-redaction into HAVE_BAUTH-on (existing) and HAVE_BAUTH-off (new) variants. The off variant pins that get_user/get_pass return empty (admin/hunter2 never leak) AND that Authorization-class header and cookie redaction are independent of HAVE_BAUTH (the dumpers are unconditional). 5. body / http_response_factories / iovec_entry Windows pipe/iovec gates: documented the gap in test/PORTABILITY.md under a new "Skipped in test/unit/" section (branch B per the plan). Each #ifndef _WIN32 block gets a // reason: comment plus a PORTABILITY.md entry; the iovec_entry POSIX-iovec gate is structurally unfixable (no Windows equivalent), and pipe_body/file_body Windows ports are flagged as a future follow-up. MHD_IoVec bridge test is unconditional and covers the actual production cast path. 6. header_hygiene_test pthread detector: deleted (DELETE-PATH per the plan's investigation gate). Both libc++ and libstdc++-with-threads unconditionally pull <pthread.h> in from any STL container header. The libhttpserver public surface uses STL containers, so the detector cannot fire on any supported CI lane without rewriting the public surface to drop std::string / std::vector / std::map. The Makefile.am HEADER_HYGIENE_FORBIDDEN comment is updated and a RELEASE_NOTES.md entry under "Test infrastructure" records the deletion rationale. The remaining hygiene sentinels (microhttpd, gnutls, sys/socket, sys/uio) still fire on every lane. Acceptance criteria: - AC-1 (every unit suite exercises non-trivial code on at least one CI lane): satisfied by the paired ws/dauth on-path TUs + the new smartptr off-path block + the new operator_stream off-path block. - AC-2 (pthread detector works or is deleted): deleted with rationale. - AC-3 (Windows lane runs body/response/iovec variants OR PORTABILITY.md records the gap): PORTABILITY.md records all three gaps with Symptom / Root cause / Restoration plan. - Typecheck passes: verified by cross-flag compile of each touched TU under HAVE_*=on and HAVE_*=off. - Tests pass: all touched unit tests exit 0; no regressions in the rest of the unit suite. (The 15 integ FAILs in local make check are the pre-existing thread_safety / port-8080 flake on feature/v2.0, unrelated to TASK-081.) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent cc60be4 commit 86f2889

14 files changed

Lines changed: 522 additions & 82 deletions

Makefile.am

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -255,29 +255,20 @@ check-install-layout:
255255
# Cross-reference: keep HEADER_HYGIENE_FORBIDDEN in sync with the
256256
# #ifdef ladder in test/unit/header_hygiene_test.cpp.
257257
#
258-
# TASK-020 caveat (libc++ AND libstdc++ in thread mode): <pthread.h>
259-
# is intentionally absent from the forbidden list below. Both
260-
# mainstream STLs unconditionally pull <pthread.h> in from any STL
261-
# container header (<vector>, <string>, <map>, etc.) when threading
262-
# is enabled:
263-
# - libc++ (Apple's default STL on macOS) routes through
264-
# <__thread/support/pthread.h>.
265-
# - libstdc++ in thread-enabled mode (which is the default whenever
266-
# -D_REENTRANT is set, as configure.ac does) routes through
267-
# <bits/gthr-default.h>, which #include <pthread.h> directly.
268-
# The resulting `# N "...pthread.h"` line markers therefore appear in
269-
# the preprocessed output even though libhttpserver itself does not
270-
# include <pthread.h>. The runtime sentinel
271-
# test/unit/header_hygiene_test.cpp keeps the pthread guards but skips
272-
# them on both libc++ (_LIBCPP_VERSION) and libstdc++ in thread mode
273-
# (_GLIBCXX_HAS_GTHREADS), so the guards still fire on STLs that don't
274-
# route std::thread through pthread (e.g. MSVC's Microsoft STL).
275-
# pthread.h is intentionally absent from HEADER_HYGIENE_FORBIDDEN below
276-
# for the same reason: the grep-based check would produce false positives
277-
# on libc++ and libstdc++ in thread mode. The runtime sentinel in
278-
# test/unit/header_hygiene_test.cpp enforces the pthread invariant via
279-
# the #ifndef _LIBCPP_VERSION && !_GLIBCXX_HAS_GTHREADS guard ladder so
280-
# it still fires on STLs that do not route std::thread through pthread.
258+
# TASK-081 pthread-detector removal: <pthread.h> is intentionally absent
259+
# from the forbidden list below, AND the corresponding runtime-sentinel
260+
# guards in test/unit/header_hygiene_test.cpp have been removed
261+
# entirely. Both mainstream STLs (libc++ and libstdc++ in thread mode,
262+
# which is the default whenever -D_REENTRANT is set, as configure.ac
263+
# does) unconditionally pull <pthread.h> in from any STL container
264+
# header. The libhttpserver public surface uses STL containers
265+
# (std::string, std::vector, std::map in http_request.hpp, http_resource.hpp,
266+
# ip_representation.hpp, ...), so the pthread leak is structural to the
267+
# C++ standard library on every CI lane libhttpserver supports. The
268+
# detector could not be satisfied on any supported configuration without
269+
# rewriting the public surface to drop STL containers, which would
270+
# break source compatibility. See RELEASE_NOTES.md (Test infrastructure)
271+
# for the deletion rationale.
281272
# ---------------------------------------------------------------------------
282273

283274
# NOTE: each entry matches a basename; the grep in check-hygiene anchors with a

RELEASE_NOTES.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,23 @@ DR-011 ([specs/architecture/11-decisions/DR-011.md](specs/architecture/11-decisi
377377
CI signal. Two new CI lanes (`tls-no-cli`, plus a baseline canary)
378378
assert the SKIP wiring fires (or does not) as configured.
379379

380+
- **`<pthread.h>` runtime-sentinel guard removed (TASK-081).** The
381+
pthread leak detector in `test/unit/header_hygiene_test.cpp` (and
382+
its companion regex slot in `Makefile.am`'s `HEADER_HYGIENE_FORBIDDEN`)
383+
was unable to fire on any CI lane libhttpserver actually runs on. The
384+
libhttpserver public surface uses STL container headers
385+
(`std::string`, `std::vector`, `std::map`, ...) and both mainstream
386+
C++ standard libraries — libc++ via
387+
`<__thread/support/pthread.h>` and libstdc++ in thread-enabled mode
388+
via `<bits/gthr-default.h>` — unconditionally drag `<pthread.h>` in
389+
from those headers. Because the public surface cannot be rewritten
390+
to drop STL containers without a source-incompatible break, the
391+
pthread guard is structurally unsatisfiable on every supported
392+
configuration. The detector was deleted rather than kept as
393+
dead code gated on STL-implementation-detection macros. The
394+
remaining hygiene sentinels (`<microhttpd.h>`, `<gnutls/gnutls.h>`,
395+
`<sys/socket.h>`, `<sys/uio.h>`) continue to run on every CI lane.
396+
380397
## See also
381398

382399
- [README.md](README.md) — full v2.0 introduction and worked examples.

specs/tasks/M7-v2-cleanup/TASK-081.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,4 @@ Make sure every suite actually exercises code on the build configuration where t
3737
**Related Requirements:** PRD-FLG-REQ-003 (`features()` coverage)
3838
**Related Decisions:** None new
3939

40-
**Status:** Backlog
40+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ TASK-093).
4545
| TASK-078 | Resolve commented-out CONNECT-method test bodies | HIGH | S | Done |
4646
| TASK-079 | Drive nonce/opaque state machine in v2 digest-auth integ tests | MED | M | Done |
4747
| TASK-080 | Tighten threadsafety_stress latency gate back from 100× to 10× | MED | M | Done |
48-
| TASK-081 | Fill empty-on-correct-build unit suites and re-enable pthread leak detector | MED | M | Backlog |
48+
| TASK-081 | Fill empty-on-correct-build unit suites and re-enable pthread leak detector | MED | M | Done |
4949
| TASK-082 | Tighten static-size bounds in `http_resource_test` and `webserver_pimpl_test` | MED | S | Backlog |
5050
| TASK-083 | Wire real CI gates into benchmarks | MED | M | Backlog |
5151
| TASK-084 | Re-measure libstdc++/Linux v1 baseline for `get_headers` ns/call | MED | S | Backlog |

test/Makefile.am

Lines changed: 19 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/ -DHTTPSERVER_COMPILATION
2828
METASOURCES = AUTO
29-
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication digest_challenge_format deferred http_resource http_response create_webserver create_webserver_explicit new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories http_response_digest_factory http_response_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_arena http_request_unescape_arena http_request_const_getters http_request_tls_accessors http_request_operator_stream webserver_register_smartptr webserver_register_path_prefix webserver_on_methods webserver_route route_table lookup_pipeline route_table_concurrency routing_regression route_lookup_canonicalize auth_skip_normalize http_resource_allow_cache v2_dispatch_contract threadsafety_stress webserver_features webserver_ws_unavailable webserver_register_ws_smartptr webserver_dauth_unavailable consumer_fixture header_hygiene_hooks hook_api_shape hooks_no_firing hooks_accept_ctx_shape hooks_connection_lifecycle hooks_accept_decision_banned hooks_accept_decision_throwing hooks_body_chunk_ctx_shape hooks_request_received_short_circuit hooks_body_chunk_observes_progress hooks_body_chunk_short_circuit_no_leak hooks_before_handler_ctx_shape hooks_route_resolved_miss_and_hit hooks_before_handler_short_circuit hooks_alias_count hooks_alias_functional hooks_handler_exception_chain hooks_handler_exception_user_handler_throws_continues_chain hooks_handler_exception_fallback_to_hardcoded_500 hooks_handler_exception_slot hooks_response_sent_ctx_shape hooks_request_completed_ctx_shape hooks_after_handler_replaces_response hooks_after_handler_mutates_response_in_place hooks_response_sent_carries_status_bytes_timing hooks_request_completed_fires_on_early_failure hooks_log_access_alias_slot hooks_per_route_invalid_phase_throws hooks_per_route_order hooks_per_route_early_413_per_endpoint hooks_per_route_resource_destroyed_first hooks_per_route_concurrent_registration hooks_not_found_alias auth_handler_optional_signature no_v1_compat_shim cookie_header_sentinel cookie_render cookie_deprecation_sentinel http_response_cookie_wire http_request_cookies_parsed peer_address_to_string secure_zero_dce connection_state_sentinel connection_state_body_residue debug_dump_request_body_unset debug_dump_request_body_set littletest_skip_semantics digest_client_self_test
29+
check_PROGRAMS = basic file_upload http_utils threaded nodelay string_utilities http_endpoint ban_system ws_start_stop authentication digest_challenge_format deferred http_resource http_response create_webserver create_webserver_explicit new_response_types daemon_info uri_log feature_unavailable header_hygiene_iovec header_hygiene iovec_entry http_method constants body http_response_sbo http_response_factories http_response_digest_factory http_response_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_arena http_request_unescape_arena http_request_const_getters http_request_tls_accessors http_request_operator_stream webserver_register_smartptr webserver_register_path_prefix webserver_on_methods webserver_route route_table lookup_pipeline route_table_concurrency routing_regression route_lookup_canonicalize auth_skip_normalize http_resource_allow_cache v2_dispatch_contract threadsafety_stress webserver_features webserver_ws_unavailable webserver_ws_available webserver_register_ws_smartptr webserver_dauth_unavailable webserver_dauth_available consumer_fixture header_hygiene_hooks hook_api_shape hooks_no_firing hooks_accept_ctx_shape hooks_connection_lifecycle hooks_accept_decision_banned hooks_accept_decision_throwing hooks_body_chunk_ctx_shape hooks_request_received_short_circuit hooks_body_chunk_observes_progress hooks_body_chunk_short_circuit_no_leak hooks_before_handler_ctx_shape hooks_route_resolved_miss_and_hit hooks_before_handler_short_circuit hooks_alias_count hooks_alias_functional hooks_handler_exception_chain hooks_handler_exception_user_handler_throws_continues_chain hooks_handler_exception_fallback_to_hardcoded_500 hooks_handler_exception_slot hooks_response_sent_ctx_shape hooks_request_completed_ctx_shape hooks_after_handler_replaces_response hooks_after_handler_mutates_response_in_place hooks_response_sent_carries_status_bytes_timing hooks_request_completed_fires_on_early_failure hooks_log_access_alias_slot hooks_per_route_invalid_phase_throws hooks_per_route_order hooks_per_route_early_413_per_endpoint hooks_per_route_resource_destroyed_first hooks_per_route_concurrent_registration hooks_not_found_alias auth_handler_optional_signature no_v1_compat_shim cookie_header_sentinel cookie_render cookie_deprecation_sentinel http_response_cookie_wire http_request_cookies_parsed peer_address_to_string secure_zero_dce connection_state_sentinel connection_state_body_residue debug_dump_request_body_unset debug_dump_request_body_set littletest_skip_semantics digest_client_self_test
3030

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

@@ -395,6 +395,16 @@ webserver_features_SOURCES = unit/webserver_features_test.cpp
395395
# webserver_register_ws_smartptr).
396396
webserver_ws_unavailable_SOURCES = unit/webserver_ws_unavailable_test.cpp
397397

398+
# webserver_ws_available: TASK-081 cycle 1. Paired on-path companion to
399+
# webserver_ws_unavailable. On HAVE_WEBSOCKET-on builds pins
400+
# webserver::features().websocket == true and that null unique_ptr /
401+
# shared_ptr arguments throw std::invalid_argument (NOT
402+
# feature_unavailable) — the throw-type contrast that makes the
403+
# unavailable TU's contract meaningful. On HAVE_WEBSOCKET-off builds
404+
# the suite is empty (the off-path contracts live in
405+
# webserver_ws_unavailable). Default LDADD is sufficient.
406+
webserver_ws_available_SOURCES = unit/webserver_ws_available_test.cpp
407+
398408
# webserver_register_ws_smartptr: TASK-035. Compile-time signature
399409
# assertions that the new unique_ptr/shared_ptr register_ws_resource
400410
# overloads exist, the raw-pointer overload is gone, and
@@ -415,6 +425,14 @@ webserver_register_ws_smartptr_SOURCES = unit/webserver_register_ws_smartptr_tes
415425
# on HAVE_DAUTH-off). On HAVE_DAUTH-on builds the suite is empty.
416426
webserver_dauth_unavailable_SOURCES = unit/webserver_dauth_unavailable_test.cpp
417427

428+
# webserver_dauth_available: TASK-081 cycle 2. Paired on-path companion
429+
# to webserver_dauth_unavailable. On HAVE_DAUTH-on builds pins
430+
# webserver::features().digest_auth == true and that constructing a
431+
# webserver with digest_auth(true) does NOT throw feature_unavailable.
432+
# On HAVE_DAUTH-off builds the suite is empty (the off-path contracts
433+
# live in webserver_dauth_unavailable). Default LDADD is sufficient.
434+
webserver_dauth_available_SOURCES = unit/webserver_dauth_available_test.cpp
435+
418436
# consumer_fixture: TASK-034 / AC #2 + TASK-037 CI matrix gate
419437
# (PRD-FLG-REQ-001 / arch §9 testing item 2). Compile-and-link gate
420438
# proving that a single consumer source file referencing every

test/PORTABILITY.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,81 @@ Each section names a specific skip site (`<file>:<line>`) and records:
128128

129129
---
130130

131+
## Skipped in test/unit/
132+
133+
The skip sites in `test/unit/` predate the `check-skip-rationales.sh` lint
134+
(which scopes itself to `test/integ/` only). They are tracked here for
135+
parity, but the lint does NOT enforce a `// reason:` comment on these
136+
blocks.
137+
138+
### `body_test.cpp` — pipe_body POSIX-pipe tests (lines 263, 277)
139+
140+
- **Symptom**: the two pipe_body tests (`pipe_body_kind_and_materialize`,
141+
`pipe_body_destructor_closes_fd_when_not_materialized`) call POSIX
142+
`::pipe()` directly. MSYS2/MinGW64 does not ship POSIX `::pipe()`;
143+
the Windows equivalent is `_pipe()` from `<io.h>` or
144+
`CreatePipe()` from `<windows.h>`, with different fd semantics.
145+
- **Root cause**: the `pipe_body` class itself is portable (it owns and
146+
closes an fd; `MHD_create_response_from_pipe` is wired the same way
147+
on every MHD build), but the tests need to *create* a pipe to feed
148+
the constructor, and pipe creation is platform-specific.
149+
- **Restoration plan**: a future follow-up could swap `::pipe()` for
150+
`_pipe(fds, 4096, _O_BINARY)` from `<io.h>` under an `#ifdef _WIN32`
151+
branch (mingw-w64 ships `<io.h>` with `_pipe`/`_close`). The Linux
152+
and macOS CI lanes currently exercise the pipe_body code path; the
153+
Windows MSYS lanes do not. The class's fd lifecycle and
154+
destructor-closes-fd contract are still verified on Linux/macOS, and
155+
the production `MHD_create_response_from_pipe` call is the same on
156+
every platform, so the Windows gap is a defence-in-depth coverage
157+
loss rather than a contract gap.
158+
159+
### `body_test.cpp` — file_body destructor test (line 198)
160+
161+
- **Symptom**: `file_body_destructor_closes_fd_when_not_materialized`
162+
uses POSIX `::open()` / `::close()` and inspects `errno == EBADF`.
163+
- **Root cause**: the file_body class itself is portable (the
164+
constructor uses `::open` via libmicrohttpd's headers on every
165+
platform), but the anchor-fd technique (open / close / re-open to
166+
obtain a known fd slot) is POSIX-shaped.
167+
- **Restoration plan**: a future follow-up could use `_open` / `_close`
168+
from `<io.h>` on Windows for an equivalent anchor-fd test. The
169+
destructor-closes-fd contract for file_body is still verified on
170+
Linux/macOS lanes; the Windows gap is the same defence-in-depth
171+
loss as for pipe_body above.
172+
173+
### `http_response_factories_test.cpp` — pipe() factory tests (lines 235–267)
174+
175+
- **Symptom**: `pipe_factory_kind` and
176+
`pipe_factory_signature_is_single_arg` call POSIX `::pipe()` to feed
177+
`http_response::pipe(int)`.
178+
- **Root cause**: same as body_test pipe_body — pipe creation is
179+
platform-specific. The factory's signature pin (`static_assert` that
180+
`http_response::pipe` takes exactly one `int` argument) is wrapped
181+
inside the `#ifndef _WIN32` block but is itself portable; a future
182+
Windows port could split the static_assert out so it runs on every
183+
lane while the runtime test stays gated.
184+
- **Restoration plan**: same follow-up as body_test pipe_body. Until
185+
then, the Linux/macOS CI lanes cover the factory and the Windows
186+
lanes do not.
187+
188+
### `iovec_entry_test.cpp` — POSIX struct iovec bridge (line 86)
189+
190+
- **Symptom**: `reinterpret_cast_to_struct_iovec_preserves_data`
191+
reinterpret-casts an `iovec_entry` array to `const struct iovec*`
192+
(POSIX `<sys/uio.h>`). MSYS2/MinGW64 does not ship `<sys/uio.h>`.
193+
- **Root cause**: POSIX `struct iovec` has no Windows equivalent; the
194+
Windows MHD build uses MHD's own `MHD_IoVec` shape, which the
195+
`reinterpret_cast_to_MHD_IoVec_preserves_data` test (unconditional,
196+
lines 105–118) already exercises on every platform.
197+
- **Restoration plan**: **not currently planned**. The
198+
`MHD_IoVec` cast test already covers the actual production cast
199+
path that `iovec_response::get_raw_response()` performs at dispatch
200+
time. The POSIX `struct iovec` cast is a defence-in-depth assertion
201+
only — its purpose is to catch a future libc-shaped consumer that
202+
re-uses the same layout. The Windows gap here is structural (no
203+
POSIX type to cast to), not a coverage loss for any production
204+
call site.
205+
131206
## Adding a new entry
132207

133208
1. Add a `// reason: <one-liner>` comment within the 5 lines immediately

test/unit/body_test.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ LT_BEGIN_AUTO_TEST(body_suite, file_body_returns_null_for_non_regular_file)
195195
LT_CHECK_EQ(b.materialize(), static_cast<MHD_Response*>(nullptr));
196196
LT_END_AUTO_TEST(file_body_returns_null_for_non_regular_file)
197197

198+
// reason: POSIX ::open/::close + errno EBADF inspection has no portable
199+
// Windows equivalent. The file_body class is itself portable; only the
200+
// anchor-fd test technique is POSIX-shaped. Gap tracked in test/PORTABILITY.md.
198201
#ifndef _WIN32
199202
// Parallel of pipe_body_destructor_closes_fd_when_not_materialized: the
200203
// ownership contract states 'if materialize() is never called, ~file_body()
@@ -253,11 +256,12 @@ LT_END_AUTO_TEST(iovec_body_empty_entries_materializes)
253256
// -----------------------------------------------------------------------
254257
// pipe_body
255258
//
256-
// Gated on !_WIN32: MSYS2/mingw does not expose POSIX ::pipe() — Windows
257-
// pipes use _pipe() / CreatePipe(). The pipe_body class itself is portable
258-
// (it just owns and closes a fd) but the tests below need to *create* a
259-
// pipe to exercise it, which is platform-specific. The Linux/macOS CI
260-
// matrix exercises this code path.
259+
// reason: MSYS2/mingw does not expose POSIX ::pipe() — Windows pipes use
260+
// _pipe() / CreatePipe() with different fd semantics. The pipe_body class
261+
// itself is portable (it just owns and closes a fd) but the tests below
262+
// need to *create* a pipe to exercise it, which is platform-specific. The
263+
// Linux/macOS CI matrix exercises this code path; the Windows gap is
264+
// tracked in test/PORTABILITY.md.
261265
// -----------------------------------------------------------------------
262266
#ifndef _WIN32
263267
LT_BEGIN_AUTO_TEST(body_suite, pipe_body_kind_and_materialize)

0 commit comments

Comments
 (0)