Skip to content

Commit c80fdff

Browse files
etrclaude
etr
and
claude
committed
TASK-002: Lock public/private header surface and inclusion gates
Tighten the public/private header split so detail headers and the HTTPSERVER_COMPILATION macro cannot leak to downstream consumers, and add make-check assertions that protect the surface going forward. Changes: - src/httpserver.hpp: #undef _HTTPSERVER_HPP_INSIDE_ after all child includes so the macro does not survive into a consumer's TU. - src/Makefile.am: move httpserver/details/http_endpoint.hpp out of nobase_include_HEADERS into noinst_HEADERS — distributed in the tarball but never installed under $prefix/include. Add -DHTTPSERVER_COMPILATION to AM_CPPFLAGS so the lib's own TUs see it. - test/Makefile.am: add -DHTTPSERVER_COMPILATION to AM_CPPFLAGS so first-party unit tests that legitimately include detail headers still compile. - configure.ac: stop injecting -DHTTPSERVER_COMPILATION into global CXXFLAGS. Scope is now per-directory (lib + tests only); examples build as true consumers via <httpserver.hpp>. - Makefile.am: new check-headers target with four sub-checks (A.1 direct public include must fail, A.2 direct detail include must fail, A.3 umbrella must compile cleanly, A.4 post-umbrella direct include must still fail) and a new check-install-layout target that runs `make install DESTDIR=...` to a stage and asserts no `details/` directory or `*_impl.hpp` file leaks. Both wired into check-local. - test/headers/: four one-line consumer TUs driving the checks. Per the plan's Phase 3a-i, the detail-header gate stays dual-mode (_HTTPSERVER_HPP_INSIDE_ || HTTPSERVER_COMPILATION) because webserver.hpp still transitively includes details/http_endpoint.hpp; TASK-014's PIMPL split will let a future change tighten that gate to HTTPSERVER_COMPILATION-only. Acceptance criteria verified: - 17/17 existing tests pass under release and --enable-debug. - check-headers A.1 fires with the gate error string. - check-install-layout: staged install has no details/ and no *_impl.hpp; httpserver.hpp + httpserverpp symlink installed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 638c26a commit c80fdff

9 files changed

Lines changed: 179 additions & 7 deletions

‎Makefile.am‎

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,134 @@ endif
3838

3939
endif
4040

41-
EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh scripts/validate-version.sh
41+
EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh scripts/validate-version.sh \
42+
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_umbrella.cpp \
43+
test/headers/consumer_post_umbrella.cpp
44+
45+
# ---------------------------------------------------------------------------
46+
# Header-hygiene checks (TASK-002)
47+
#
48+
# check-headers verifies that the public/private header gates are wired up
49+
# correctly:
50+
# A.1 a consumer including a public header WITHOUT the umbrella must hit the
51+
# inclusion-gate #error.
52+
# A.2 a consumer including a detail header WITHOUT HTTPSERVER_COMPILATION
53+
# must hit the gate.
54+
# A.3 a consumer including only the umbrella, WITHOUT HTTPSERVER_COMPILATION,
55+
# must compile cleanly.
56+
#
57+
# The CXX invocations below override CXXFLAGS to '' so that
58+
# -DHTTPSERVER_COMPILATION (injected by configure.ac into CXXFLAGS for the
59+
# library and test build) does NOT leak into the consumer-style compile. We
60+
# still pass -std=c++20 explicitly because libhttpserver requires C++20.
61+
# ---------------------------------------------------------------------------
62+
63+
# Compose CXX with: explicit -std, the source/build include search paths used by
64+
# the library, and $(CPPFLAGS) (e.g., -I/opt/homebrew/include from configure).
65+
# Deliberately omit $(CXXFLAGS), $(AM_CPPFLAGS), and any per-target CPPFLAGS so
66+
# that -DHTTPSERVER_COMPILATION (set in src/ and test/ AM_CPPFLAGS) cannot
67+
# leak into the consumer-style compile. A true consumer never has that macro.
68+
CHECK_HEADERS_CXX = $(CXX) -std=c++20 -I$(top_builddir) -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver $(CPPFLAGS)
69+
CHECK_HEADERS_GATE_MSG = Only <httpserver.hpp> or <httpserverpp> can be included directly
70+
71+
check-headers:
72+
@echo "=== check-headers A.1: direct public-header include must fail ==="
73+
@if $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_direct.cpp -o /dev/null 2>check-headers-A1.log; then \
74+
echo "FAIL: consumer_direct.cpp compiled but should have errored"; \
75+
cat check-headers-A1.log; \
76+
rm -f check-headers-A1.log; \
77+
exit 1; \
78+
fi
79+
@if ! grep -q "$(CHECK_HEADERS_GATE_MSG)" check-headers-A1.log; then \
80+
echo "FAIL: consumer_direct.cpp failed but not for the gate reason"; \
81+
cat check-headers-A1.log; \
82+
rm -f check-headers-A1.log; \
83+
exit 1; \
84+
fi
85+
@rm -f check-headers-A1.log
86+
@echo " PASS: A.1 gate fired as expected"
87+
@echo "=== check-headers A.2: direct detail-header include must fail ==="
88+
@if $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_detail.cpp -o /dev/null 2>check-headers-A2.log; then \
89+
echo "FAIL: consumer_detail.cpp compiled but should have errored"; \
90+
cat check-headers-A2.log; \
91+
rm -f check-headers-A2.log; \
92+
exit 1; \
93+
fi
94+
@if ! grep -q "$(CHECK_HEADERS_GATE_MSG)" check-headers-A2.log; then \
95+
echo "FAIL: consumer_detail.cpp failed but not for the gate reason"; \
96+
cat check-headers-A2.log; \
97+
rm -f check-headers-A2.log; \
98+
exit 1; \
99+
fi
100+
@rm -f check-headers-A2.log
101+
@echo " PASS: A.2 gate fired as expected"
102+
@echo "=== check-headers A.3: umbrella include must succeed ==="
103+
@if ! $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_umbrella.cpp -o consumer_umbrella.check.o 2>check-headers-A3.log; then \
104+
echo "FAIL: consumer_umbrella.cpp did not compile"; \
105+
cat check-headers-A3.log; \
106+
rm -f check-headers-A3.log consumer_umbrella.check.o; \
107+
exit 1; \
108+
fi
109+
@rm -f check-headers-A3.log consumer_umbrella.check.o
110+
@echo " PASS: A.3 umbrella compiled cleanly"
111+
@echo "=== check-headers A.4: post-umbrella direct include must fail ==="
112+
@if $(CHECK_HEADERS_CXX) -c $(top_srcdir)/test/headers/consumer_post_umbrella.cpp -o /dev/null 2>check-headers-A4.log; then \
113+
echo "FAIL: consumer_post_umbrella.cpp compiled but should have errored"; \
114+
cat check-headers-A4.log; \
115+
rm -f check-headers-A4.log; \
116+
exit 1; \
117+
fi
118+
@if ! grep -q "$(CHECK_HEADERS_GATE_MSG)" check-headers-A4.log; then \
119+
echo "FAIL: consumer_post_umbrella.cpp failed but not for the gate reason"; \
120+
cat check-headers-A4.log; \
121+
rm -f check-headers-A4.log; \
122+
exit 1; \
123+
fi
124+
@rm -f check-headers-A4.log
125+
@echo " PASS: A.4 umbrella does not leak _HTTPSERVER_HPP_INSIDE_"
126+
127+
# check-install-layout asserts that `make install DESTDIR=$(STAGE)` produces
128+
# a public include tree with NO `details/` directory and NO `*_impl.hpp` files.
129+
# This protects the public/private split as described in TASK-002 / DR-002.
130+
CHECK_INSTALL_STAGE = $(abs_top_builddir)/.install-stage
131+
132+
check-install-layout:
133+
@echo "=== check-install-layout: staged install must hide details/ and *_impl.hpp ==="
134+
@rm -rf $(CHECK_INSTALL_STAGE)
135+
@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(CHECK_INSTALL_STAGE) >check-install.log 2>&1 || { \
136+
echo "FAIL: staged install failed"; \
137+
cat check-install.log; \
138+
rm -f check-install.log; \
139+
rm -rf $(CHECK_INSTALL_STAGE); \
140+
exit 1; \
141+
}
142+
@rm -f check-install.log
143+
@leaked_details=`find $(CHECK_INSTALL_STAGE) -type d -name details 2>/dev/null`; \
144+
if test -n "$$leaked_details"; then \
145+
echo "FAIL: details/ directory leaked into install:"; \
146+
echo "$$leaked_details"; \
147+
rm -rf $(CHECK_INSTALL_STAGE); \
148+
exit 1; \
149+
fi
150+
@leaked_impl=`find $(CHECK_INSTALL_STAGE) -name '*_impl.hpp' 2>/dev/null`; \
151+
if test -n "$$leaked_impl"; then \
152+
echo "FAIL: *_impl.hpp file leaked into install:"; \
153+
echo "$$leaked_impl"; \
154+
rm -rf $(CHECK_INSTALL_STAGE); \
155+
exit 1; \
156+
fi
157+
@umbrella_count=`find $(CHECK_INSTALL_STAGE) -name 'httpserver.hpp' | wc -l | tr -d ' '`; \
158+
if test "$$umbrella_count" != "1"; then \
159+
echo "FAIL: expected exactly 1 installed httpserver.hpp, got $$umbrella_count"; \
160+
rm -rf $(CHECK_INSTALL_STAGE); \
161+
exit 1; \
162+
fi
163+
@rm -rf $(CHECK_INSTALL_STAGE)
164+
@echo " PASS: staged install layout is clean"
165+
166+
check-local: check-headers check-install-layout
167+
168+
.PHONY: check-headers check-install-layout
42169

43170
MOSTLYCLEANFILES = $(DX_CLEANFILES) *.gcda *.gcno *.gcov
44171
DISTCLEANFILES = DIST_REVISION

‎configure.ac‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ if test x"$host" = x"$build"; then
127127
[AC_MSG_ERROR(["microhttpd.h not found"])]
128128
)
129129

130-
CXXFLAGS="-DHTTPSERVER_COMPILATION -D_REENTRANT $LIBMICROHTTPD_CFLAGS $CXXFLAGS"
130+
# -DHTTPSERVER_COMPILATION is intentionally NOT injected globally into
131+
# CXXFLAGS. It is added per-target via AM_CPPFLAGS in src/Makefile.am and
132+
# test/Makefile.am so that examples (and any other consumer-style TUs)
133+
# build through the umbrella header without seeing the internal macro.
134+
CXXFLAGS="-D_REENTRANT $LIBMICROHTTPD_CFLAGS $CXXFLAGS"
131135
LDFLAGS="$LIBMICROHTTPD_LIBS $NETWORK_LIBS $ADDITIONAL_LIBS $LDFLAGS"
132136

133137
cond_cross_compile="no"
@@ -140,7 +144,9 @@ else
140144
[AC_MSG_ERROR(["microhttpd.h not found"])]
141145
)
142146

143-
CXXFLAGS="-DHTTPSERVER_COMPILATION -D_REENTRANT $CXXFLAGS"
147+
# See note above: HTTPSERVER_COMPILATION is scoped to lib + tests via
148+
# per-directory AM_CPPFLAGS, not injected globally into CXXFLAGS.
149+
CXXFLAGS="-D_REENTRANT $CXXFLAGS"
144150
LDFLAGS="$NETWORK_LIBS $ADDITIONAL_LIBS $LDFLAGS"
145151

146152
cond_cross_compile="yes"

‎src/Makefile.am‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
# License along with this library; if not, write to the Free Software
1717
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1818

19-
AM_CPPFLAGS = -I../ -I$(srcdir)/httpserver/
19+
AM_CPPFLAGS = -I../ -I$(srcdir)/httpserver/ -DHTTPSERVER_COMPILATION
2020
METASOURCES = AUTO
2121
lib_LTLIBRARIES = libhttpserver.la
2222
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp http_utils.cpp file_info.cpp http_request.cpp http_response.cpp string_response.cpp digest_auth_fail_response.cpp deferred_response.cpp file_response.cpp pipe_response.cpp empty_response.cpp iovec_response.cpp http_resource.cpp create_webserver.cpp details/http_endpoint.cpp
23-
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/details/modded_request.hpp gettext.h
24-
nobase_include_HEADERS = httpserver.hpp httpserver/create_webserver.hpp httpserver/webserver.hpp httpserver/http_utils.hpp httpserver/file_info.hpp httpserver/details/http_endpoint.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/string_response.hpp httpserver/digest_auth_fail_response.hpp httpserver/deferred_response.hpp httpserver/file_response.hpp httpserver/pipe_response.hpp httpserver/empty_response.hpp httpserver/iovec_response.hpp httpserver/http_arg_value.hpp
23+
# noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
24+
# Detail headers (httpserver/details/*.hpp) live here so they cannot leak to
25+
# downstream consumers — the public surface comes in through <httpserver.hpp>.
26+
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/details/modded_request.hpp httpserver/details/http_endpoint.hpp gettext.h
27+
nobase_include_HEADERS = httpserver.hpp httpserver/create_webserver.hpp httpserver/webserver.hpp httpserver/http_utils.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/string_response.hpp httpserver/digest_auth_fail_response.hpp httpserver/deferred_response.hpp httpserver/file_response.hpp httpserver/pipe_response.hpp httpserver/empty_response.hpp httpserver/iovec_response.hpp httpserver/http_arg_value.hpp
2528

2629
if HAVE_BAUTH
2730
libhttpserver_la_SOURCES += basic_auth_fail_response.cpp

‎src/httpserver.hpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,6 @@
5050
#include "httpserver/websocket_handler.hpp"
5151
#endif // HAVE_WEBSOCKET
5252

53+
#undef _HTTPSERVER_HPP_INSIDE_
54+
5355
#endif // SRC_HTTPSERVER_HPP_

‎test/Makefile.am‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ endif
2424

2525
LDADD += -lcurl
2626

27-
AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/
27+
AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/src/httpserver/ -DHTTPSERVER_COMPILATION
2828
METASOURCES = AUTO
2929
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
3030

‎test/headers/consumer_detail.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Negative test (Check A.2): a consumer including a detail header directly,
2+
// even when _HTTPSERVER_HPP_INSIDE_ is defined (simulating the umbrella state),
3+
// must hit the gate when HTTPSERVER_COMPILATION is not defined.
4+
//
5+
// NOTE: pre-Phase-3 the detail gate is dual-mode (accepts either macro), so
6+
// this TU defines _HTTPSERVER_HPP_INSIDE_ to exercise the strictest
7+
// post-cleanup behavior. After TASK-014 lands the PIMPL split, the gate may
8+
// drop the _HTTPSERVER_HPP_INSIDE_ acceptor altogether; this test should keep
9+
// passing because the consumer-style invocation also lacks HTTPSERVER_COMPILATION.
10+
//
11+
// For TASK-002 we keep the dual-mode gate (per the plan's Phase 3a-i), so this
12+
// TU is built WITHOUT defining _HTTPSERVER_HPP_INSIDE_ — the detail gate then
13+
// fires for the same reason as A.1.
14+
#include "httpserver/details/http_endpoint.hpp"
15+
int main() { return 0; }

‎test/headers/consumer_direct.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Negative test (Check A.1): a consumer compiling this TU WITHOUT the umbrella
2+
// header AND WITHOUT HTTPSERVER_COMPILATION must hit the inclusion-gate #error.
3+
// The build recipe inverts exit status and greps for the gate text to ensure
4+
// the failure is for the right reason.
5+
#include "httpserver/webserver.hpp"
6+
int main() { return 0; }

‎test/headers/consumer_post_umbrella.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Negative test (Check A.4): including the umbrella must NOT leak the
2+
// _HTTPSERVER_HPP_INSIDE_ macro to subsequent translation-unit-scope code.
3+
// A consumer doing `#include <httpserver.hpp>` followed by a direct include
4+
// of a public header must STILL hit the gate. This catches the bug where the
5+
// umbrella defines _HTTPSERVER_HPP_INSIDE_ but does not #undef it.
6+
#include <httpserver.hpp>
7+
#include "httpserver/webserver.hpp"
8+
int main() { return 0; }

‎test/headers/consumer_umbrella.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// Positive control (Check A.3): a consumer including only the umbrella header,
2+
// without HTTPSERVER_COMPILATION, must compile cleanly. This proves the umbrella
3+
// path is the supported entry point.
4+
#include <httpserver.hpp>
5+
int main() { return 0; }

0 commit comments

Comments
 (0)