Skip to content

Commit 51b5a37

Browse files
committed
TASK-058 step 2: pre-normalize auth_skip_paths + empty-list early-out
Pre-normalize each auth_skip_paths entry once at webserver construction time and short-circuit should_skip_auth when the list is empty. Two wins: 1. Empty-list early-out -- servers with no skip paths configured (the production-typical case for any auth_handler that gates every route, or any server with no auth_handler at all) pay zero normalize cost per request. 2. Compare canonical-vs-canonical -- the request path is normalised on the per-request side; pre-TASK-058 the skip list was matched verbatim against it, so non-canonical inputs like "/public/" or "/a/../b" silently never matched. This is a latent bug-fix in addition to the perf win. Note on the task brief's "store on route_entry" framing. The plan calls for storing the normalized form on route_entry, but route_entry is a per-route attribute and auth_skip_paths is a webserver-level request filter that doesn't look at route_entry at all (see webserver_request.cpp should_skip_auth -- it compares the request path against parent->auth_skip_paths, not any route attribute). The optimization is moved to where the registration-time data actually lives: auth_skip_paths_normalized_ as a sibling on webserver. The brief's *spirit* (do the normalize once at registration, not per request) is preserved. New helper detail::normalize_auth_skip_paths handles the wildcard- suffix carve-out: entries ending in "/*" keep their trailing wildcard verbatim; only the prefix before "/*" is normalized. The "/*" case maps to itself (matches the v1 dispatch behaviour). TDD red->green via new test/unit/auth_skip_normalize_test.cpp: - non-canonical skip entries ({/public/, /a/../b, /x/./y}) now match canonical request paths ({/public, /b, /x/y}) - canonical skip entries continue to work (regression net) - wildcard-suffix skip entries normalize the prefix correctly - empty skip list returns false for every path without touching the per-request normalize machinery Files touched: - src/httpserver/detail/path_normalize.hpp (new, declares the helper) - src/detail/webserver_request.cpp (defines normalize_auth_skip_paths alongside normalize_path; should_skip_auth uses the normalized list and the empty-list early-out) - src/httpserver/webserver.hpp (auth_skip_paths_normalized_ member) - src/webserver.cpp (populate at construction) - test/Makefile.am (wire auth_skip_normalize) - test/unit/auth_skip_normalize_test.cpp (new)
1 parent e2f730d commit 51b5a37

6 files changed

Lines changed: 273 additions & 2 deletions

File tree

src/detail/webserver_request.cpp

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
#include "httpserver/detail/http_endpoint.hpp"
6868
#include "httpserver/detail/lambda_resource.hpp"
6969
#include "httpserver/detail/modded_request.hpp"
70+
#include "httpserver/detail/path_normalize.hpp"
7071
#include "httpserver/detail/resource_hook_table.hpp"
7172
#include "httpserver/http_request.hpp"
7273
#include "httpserver/http_resource.hpp"
@@ -136,10 +137,61 @@ std::string normalize_path(const std::string& path) {
136137

137138
} // namespace
138139

140+
// TASK-058 step 2: pre-normalize each auth_skip_paths entry once at
141+
// webserver construction time. Entries ending in "/*" keep their
142+
// wildcard suffix; the prefix before the wildcard is normalized.
143+
// Callers (webserver::webserver) pass the raw config-bag list and
144+
// store the result on the webserver instance as a sibling to the
145+
// original `auth_skip_paths` list. Pre-TASK-058 the skip list was
146+
// matched verbatim against a normalized request path, so non-
147+
// canonical inputs (e.g. "/public/", "/a/../b") silently never
148+
// matched -- the on-the-wire bug this normalization closes.
149+
std::vector<std::string> normalize_auth_skip_paths(
150+
const std::vector<std::string>& raw) {
151+
std::vector<std::string> out;
152+
out.reserve(raw.size());
153+
for (const auto& entry : raw) {
154+
// Wildcard suffix: strip the trailing "/*", normalize the
155+
// prefix, then re-append "/*". Empty prefix (just "/*")
156+
// canonicalises to "/" + "/*" -> "//*", which would never
157+
// have matched anything sensible pre-fix either, so we keep
158+
// the literal "/*" unchanged (matches the v1 wildcard
159+
// behaviour at the dispatch site).
160+
if (entry.size() > 2 && entry.back() == '*' &&
161+
entry[entry.size() - 2] == '/') {
162+
std::string prefix = entry.substr(0, entry.size() - 2);
163+
std::string normalized_prefix = normalize_path(prefix);
164+
if (normalized_prefix == "/") {
165+
// "/" + "/*" -> "/*" (keep literal).
166+
out.push_back("/*");
167+
} else {
168+
out.push_back(normalized_prefix + "/*");
169+
}
170+
continue;
171+
}
172+
out.push_back(normalize_path(entry));
173+
}
174+
return out;
175+
}
176+
139177
bool webserver_impl::should_skip_auth(const std::string& path) const {
178+
// TASK-058 step 2: empty-list early-out. Servers with no
179+
// auth_skip_paths configured pay zero normalization cost. This
180+
// is the production-typical case for any server whose auth
181+
// surface either covers every route or has no auth_handler at
182+
// all.
183+
if (parent->auth_skip_paths_normalized.empty()) {
184+
return false;
185+
}
186+
187+
// TASK-058 step 2: compare against the pre-normalized list (built
188+
// once at construction time) instead of re-normalizing skip-list
189+
// entries on every request. The per-request normalize_path call
190+
// on @p path remains -- the inbound URL is per-request data and
191+
// cannot be pre-normalized.
140192
std::string normalized = normalize_path(path);
141193

142-
for (const auto& skip_path : parent->auth_skip_paths) {
194+
for (const auto& skip_path : parent->auth_skip_paths_normalized) {
143195
if (skip_path == normalized) return true;
144196
// Support wildcard suffix (e.g., "/public/*")
145197
if (skip_path.size() > 2 && skip_path.back() == '*' &&
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 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+
// Internal detail header. Strict gate: reachable only from libhttpserver
22+
// translation units.
23+
#if !defined(HTTPSERVER_COMPILATION)
24+
#error "path_normalize.hpp is internal; only reachable when compiling libhttpserver."
25+
#endif
26+
27+
#ifndef SRC_HTTPSERVER_DETAIL_PATH_NORMALIZE_HPP_
28+
#define SRC_HTTPSERVER_DETAIL_PATH_NORMALIZE_HPP_
29+
30+
#include <string>
31+
#include <vector>
32+
33+
namespace httpserver {
34+
namespace detail {
35+
36+
// TASK-058 step 2: pre-normalize an auth_skip_paths list so the per-
37+
// request comparison in webserver_impl::should_skip_auth runs against
38+
// already-canonical entries. Each entry is fed through normalize_path
39+
// (the same helper that normalizes the *request* path inside
40+
// should_skip_auth). Entries ending in "/*" keep their trailing "/*"
41+
// wildcard suffix; the prefix before the wildcard is normalized.
42+
//
43+
// Pure function: no shared state, callable from the webserver
44+
// constructor body. The definition lives in detail/webserver_request.cpp
45+
// alongside normalize_path so the helper and its callers share a single
46+
// canonicalisation rule.
47+
std::vector<std::string> normalize_auth_skip_paths(
48+
const std::vector<std::string>& raw);
49+
50+
} // namespace detail
51+
} // namespace httpserver
52+
53+
#endif // SRC_HTTPSERVER_DETAIL_PATH_NORMALIZE_HPP_

src/httpserver/webserver.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,13 @@ class webserver {
450450
const file_cleanup_callback_ptr file_cleanup_callback;
451451
const auth_handler_ptr auth_handler;
452452
const std::vector<std::string> auth_skip_paths;
453+
// TASK-058 step 2: pre-normalized form of @ref auth_skip_paths,
454+
// populated once at construction. webserver_impl::should_skip_auth
455+
// compares request paths against this list (not @ref auth_skip_paths)
456+
// so non-canonical entries like "/public/" or "/a/../b" match the
457+
// canonical request path the dispatch surface produces. Built by
458+
// detail::normalize_auth_skip_paths in webserver_request.cpp.
459+
const std::vector<std::string> auth_skip_paths_normalized;
453460
const sni_callback_t sni_callback;
454461
const bool no_listen_socket;
455462
const bool no_thread_safety;

src/webserver.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
#include "httpserver/detail/http_endpoint.hpp"
7171
#include "httpserver/detail/lambda_resource.hpp"
7272
#include "httpserver/detail/modded_request.hpp"
73+
#include "httpserver/detail/path_normalize.hpp"
7374
#include "httpserver/http_request.hpp"
7475
#include "httpserver/http_resource.hpp"
7576
#include "httpserver/http_response.hpp"
@@ -235,6 +236,8 @@ webserver::webserver(const create_webserver& params):
235236
file_cleanup_callback(params._file_cleanup_callback),
236237
auth_handler(params._auth_handler),
237238
auth_skip_paths(params._auth_skip_paths),
239+
auth_skip_paths_normalized(
240+
detail::normalize_auth_skip_paths(params._auth_skip_paths)),
238241
sni_callback(params._sni_callback),
239242
no_listen_socket(params._no_listen_socket),
240243
no_thread_safety(params._no_thread_safety),

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/ -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 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_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_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 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 auth_handler_optional_signature auth_handler_legacy_shim
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 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_move_sanitizer webserver_pimpl http_request_pimpl create_test_request http_request_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 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 auth_handler_optional_signature auth_handler_legacy_shim
3030

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

@@ -291,6 +291,14 @@ routing_regression_LDADD = $(LDADD) -lmicrohttpd
291291
route_lookup_canonicalize_SOURCES = unit/route_lookup_canonicalize_test.cpp
292292
route_lookup_canonicalize_LDADD = $(LDADD) -lmicrohttpd
293293

294+
# auth_skip_normalize (TASK-058 step 2): pins that auth_skip_paths
295+
# entries are normalized at webserver construction time, so non-
296+
# canonical inputs match canonical request paths. Same -lmicrohttpd
297+
# dependency as routing_regression: webserver pulls libhttpserver in,
298+
# which transitively touches libmicrohttpd headers.
299+
auth_skip_normalize_SOURCES = unit/auth_skip_normalize_test.cpp
300+
auth_skip_normalize_LDADD = $(LDADD) -lmicrohttpd
301+
294302
# v2_dispatch_contract: TASK-053. End-to-end safety net pinning the
295303
# observable invariants the dispatch path must satisfy AFTER finalize_answer
296304
# is cut over from resolve_resource_for_request (v1 maps) to
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 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+
// TASK-058 step 2: pin that auth_skip_paths entries are normalized at
22+
// webserver construction time, so non-canonical inputs ({"/public/",
23+
// "/a/../b", "/x/./y"}) match canonical request paths ({"/public",
24+
// "/b", "/x/y"}).
25+
//
26+
// Pre-TASK-058: should_skip_auth normalized the *request* path but
27+
// compared it verbatim against the skip list, so a non-canonical skip-
28+
// list entry never matched. This was a latent bug -- callers who
29+
// passed pretty-but-non-canonical skip paths (e.g. trailing slashes
30+
// from copy-paste, "/foo/./bar" from path-builder code) silently saw
31+
// their auth-skip preference ignored.
32+
//
33+
// After step 2 the skip list is pre-normalized at construction; this
34+
// test pins the new behaviour and also covers the empty-list early-
35+
// out path (no skip paths configured -> should_skip_auth returns
36+
// false without touching normalize_path).
37+
38+
#include <memory>
39+
#include <string>
40+
#include <vector>
41+
42+
#include "./httpserver.hpp"
43+
#include "./httpserver/detail/webserver_impl.hpp"
44+
#include "./littletest.hpp"
45+
46+
namespace ht = httpserver;
47+
48+
namespace {
49+
50+
ht::detail::webserver_impl& impl_of(ht::webserver& ws) {
51+
return *ht::webserver_test_access::impl(ws);
52+
}
53+
54+
} // namespace
55+
56+
LT_BEGIN_SUITE(auth_skip_normalize_suite)
57+
void set_up() {}
58+
void tear_down() {}
59+
LT_END_SUITE(auth_skip_normalize_suite)
60+
61+
// ---------------------------------------------------------------------
62+
// Non-canonical skip paths with trailing slashes match canonical
63+
// request paths after construction-time normalization.
64+
// ---------------------------------------------------------------------
65+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
66+
trailing_slash_skip_path_matches_canonical_request)
67+
ht::webserver ws{ht::create_webserver(8080)
68+
.start_method(ht::http::http_utils::INTERNAL_SELECT)
69+
.auth_skip_paths({"/public/"})};
70+
71+
// Canonical request path -- pre-normalize on skip-list side makes
72+
// this match.
73+
LT_CHECK(impl_of(ws).should_skip_auth("/public"));
74+
LT_END_AUTO_TEST(trailing_slash_skip_path_matches_canonical_request)
75+
76+
// ---------------------------------------------------------------------
77+
// ".." segments in skip paths are collapsed at construction time.
78+
// ---------------------------------------------------------------------
79+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
80+
dotdot_skip_path_matches_canonical_request)
81+
ht::webserver ws{ht::create_webserver(8080)
82+
.start_method(ht::http::http_utils::INTERNAL_SELECT)
83+
.auth_skip_paths({"/a/../b"})};
84+
85+
LT_CHECK(impl_of(ws).should_skip_auth("/b"));
86+
LT_END_AUTO_TEST(dotdot_skip_path_matches_canonical_request)
87+
88+
// ---------------------------------------------------------------------
89+
// "." segments in skip paths are stripped at construction time.
90+
// ---------------------------------------------------------------------
91+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
92+
dot_skip_path_matches_canonical_request)
93+
ht::webserver ws{ht::create_webserver(8080)
94+
.start_method(ht::http::http_utils::INTERNAL_SELECT)
95+
.auth_skip_paths({"/x/./y"})};
96+
97+
LT_CHECK(impl_of(ws).should_skip_auth("/x/y"));
98+
LT_END_AUTO_TEST(dot_skip_path_matches_canonical_request)
99+
100+
// ---------------------------------------------------------------------
101+
// Already-canonical skip paths continue to work (regression net for
102+
// the existing happy path).
103+
// ---------------------------------------------------------------------
104+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
105+
canonical_skip_path_still_matches)
106+
ht::webserver ws{ht::create_webserver(8080)
107+
.start_method(ht::http::http_utils::INTERNAL_SELECT)
108+
.auth_skip_paths({"/public", "/health"})};
109+
110+
LT_CHECK(impl_of(ws).should_skip_auth("/public"));
111+
LT_CHECK(impl_of(ws).should_skip_auth("/health"));
112+
LT_CHECK(!impl_of(ws).should_skip_auth("/api"));
113+
LT_END_AUTO_TEST(canonical_skip_path_still_matches)
114+
115+
// ---------------------------------------------------------------------
116+
// Wildcard-suffix skip paths ("/public/*") have their prefix
117+
// normalized too; the wildcard semantics are preserved.
118+
// ---------------------------------------------------------------------
119+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
120+
wildcard_suffix_skip_path_normalizes_prefix)
121+
ht::webserver ws{ht::create_webserver(8080)
122+
.start_method(ht::http::http_utils::INTERNAL_SELECT)
123+
.auth_skip_paths({"/public/*"})};
124+
125+
LT_CHECK(impl_of(ws).should_skip_auth("/public/foo"));
126+
LT_CHECK(impl_of(ws).should_skip_auth("/public/foo/bar"));
127+
LT_CHECK(!impl_of(ws).should_skip_auth("/private/foo"));
128+
LT_END_AUTO_TEST(wildcard_suffix_skip_path_normalizes_prefix)
129+
130+
// ---------------------------------------------------------------------
131+
// Empty skip list early-out: no auth_skip_paths configured, so
132+
// should_skip_auth must return false without touching the per-request
133+
// normalize machinery. Behavior pin only -- the perf win is visible
134+
// in the bench, not here.
135+
// ---------------------------------------------------------------------
136+
LT_BEGIN_AUTO_TEST(auth_skip_normalize_suite,
137+
empty_skip_list_returns_false_for_any_path)
138+
ht::webserver ws{ht::create_webserver(8080)
139+
.start_method(ht::http::http_utils::INTERNAL_SELECT)};
140+
141+
LT_CHECK(!impl_of(ws).should_skip_auth("/anywhere"));
142+
LT_CHECK(!impl_of(ws).should_skip_auth("/"));
143+
LT_CHECK(!impl_of(ws).should_skip_auth(""));
144+
LT_END_AUTO_TEST(empty_skip_list_returns_false_for_any_path)
145+
146+
LT_BEGIN_AUTO_TEST_ENV()
147+
AUTORUN_TESTS()
148+
LT_END_AUTO_TEST_ENV()

0 commit comments

Comments
 (0)