RFC 9110 7.8 defines Upgrade as a comma-separated list of protocols and asks
recipients to match each protocol-name case-insensitively; RFC 6455 4.2.1 asks
for a header field containing the value "websocket". Both handshake checks
instead read occurrence zero and required the whole field value to be exactly
"websocket", so a client offering "websocket, HTTP/3.0" -- or naming websocket
on a second Upgrade field line -- was answered 404 rather than 101.
This is the defect ffe2a1c fixed for Connection two lines below, and
has_header_token() is already called in both of these functions.
The client-side check loosens what we accept back from a server, which is the
same reading: a server answering 101 may name websocket alongside another
protocol, and rejecting that handshake was ours to get wrong.
The four ExpectTokenTest cases landed inside the #ifndef _WIN32 that guards the
10 GiB content-provider test below them, so Windows never compiled them and the
green Windows jobs said nothing about the fix. Nothing in them is POSIX-only --
they use the same helpers as ConnectionTokenTest, which sits outside any guard
-- so move them above the guard.
probe_expect() re-implemented send_request(), down to the create_client_socket
argument list. Call send_request() instead, with Connection: close so its read
loop ends at the response rather than idling to the read timeout; the Connection
check runs before the Expect block, so it does not disturb what is under test.
Move the new Content-Encoding case below its siblings. Appending it to the tail
of the comment block left the "whole token" paragraph reading as documentation
for a test about repeated field lines. The paragraph above it had been detached
from KnownEncodingWithoutSupportIsReported the same way one commit earlier; put
that one back too.
RFC 9110 Section 10.1.1 defines Expect as a comma-separated list, states that
its value is case-insensitive, and requires a server that receives a
100-continue expectation in an HTTP/1.0 request to ignore it. Comparing the
whole field value against "100-continue" met none of those.
An HTTP/1.0 request asking for 100-continue was answered with a 100 (Continue)
interim response, which that section forbids. "100-Continue" and
"100-continue, foo" were both read as no expectation at all, so a client that
waits for the interim response before sending its content waited for a response
that was never coming.
Route the check through has_header_token(), which walks every field line and
compares complete tokens case-insensitively, and skip it for HTTP/1.0. An
expectation cpp-httplib does not recognize is still ignored rather than
refused; the 417 the section offers for one is a MAY, not a requirement.
RFC 9110 Section 5.3 makes a Content-Encoding spread over several field lines
the same message as the comma-joined one, so the two have to be read the same
way. Reading occurrence zero did not: a response carrying "gzip" on two field
lines was decoded as a single gzip coding, so a body the sender says was
encoded twice came back after one pass -- still compressed, but presented to
the caller as decoded. The same value written as "gzip, gzip" on one line took
the pass-through path instead.
Read the combined value at both sites. A value naming several codings matches
none of the ones cpp-httplib implements, so both representations now take the
pass-through path that prepare_content_receiver() already documents for an
unrecognized coding.
This does mean a sender that repeats "Content-Encoding: gzip" on two lines for
a body it gzipped once no longer has that body decoded. There is no way to tell
that sender apart from one that really did encode twice, and the conservative
reading is the one the field value states.
is_brotli_encoding() and is_zstd_encoding() searched the Content-Encoding
value for "br" and "zstd" as substrings, while is_zlib_encoding() beside them
compared the whole value. So "fibre" and "librarian" were read as Brotli and
"x-zstd-ish" as Zstandard, and "gzip, br" -- a value naming two codings, which
cpp-httplib does not support -- was labeled Brotli and run through a Brotli
decompressor over gzip data.
RFC 9110 8.4.1 defines a content coding as a token, so compare the whole value
case-insensitively as the zlib check already does. A value naming several
codings no longer matches any of them and takes the pass-through path
prepare_content_receiver() already documents for an unrecognized coding.
contains_case_ignore() has no callers left.
RFC 9110 Section 7.6.1 defines Connection as a comma-separated list of
case-insensitive connection options, and Section 5.3 lets that list be split
across several field lines. Comparing the whole field value against a single
option gets both wrong.
A client sending "Connection: keep-alive, close" was answered without a
Connection header and its socket was kept open, so the close it asked for was
never performed and never announced. An HTTP/1.0 client asking for keep-alive
only got it by spelling the option exactly "Keep-Alive"; the lowercase form
everyone actually sends closed the connection instead.
Route the five Connection checks through has_header_token(), which already
walks every field line and compares complete tokens. Expect is left alone:
matching "100-continue" as a token would make an unrecognized expectation
alongside it look acceptable, where Section 10.1.1 asks for 417.
RFC 9110 Section 5.2 and 5.3 define the combined value of repeated field
lines as their values joined by commas in the order they were received.
Several call sites read only the first occurrence and then split that on
commas, so whatever the later field lines carried was silently dropped: an
acceptable media type or content coding, an ETag, a WebSocket subprotocol, a
declared trailer name, or an address a proxy appended as its own line rather
than by extending the one it received.
Add detail::get_combined_header_value() and use it for Accept,
Accept-Encoding, If-None-Match, Sec-WebSocket-Protocol, Trailer and
X-Forwarded-For. Empty field lines are skipped so the combined value never
starts with a bare comma, which parse_accept_header() rejects outright.
Also drop the now-dead manual trimming in parse_trailers() and replace the
istringstream-based subprotocol tokenizer with detail::split(); split()
already trims each token and skips empty ones.
* Match the Connection "Upgrade" token exactly in WebSocket handshakes
The server and the client both looked for "upgrade" as a substring of the
Connection field value, so "notupgrade", "upgrade-not" and "xupgrade" all
passed as the standalone token the handshake requires. RFC 6455 4.2.1 asks
for an ASCII case-insensitive token match, and a value split across several
Connection lines was missed entirely because only the first line was read.
Parse the field as the comma-separated token list it is, across every line,
and reuse the same helper for the server request check and the client
response check.
Reported by gb1dev.
* Tidy up the Connection token helper
Move has_header_token() out of the WebSocket-only detail block and next to
the other header field helpers, forward-declaring it beside split(). Use the
existing split_find(), which drops the manual found flag and stops at the
first matching token.
Drive the client-side test from an ordinary Server route answering 101 with
a bad Connection value, rather than the hand-rolled listening socket copied
from the test above it.
* Keep the Connection token helper out of the split build's ABI
The split build strips inline from everything below the border line, so a
helper defined there becomes an exported symbol of the shared library and
abidiff reports it as an added function. The tests also could not see
is_websocket_upgrade() or websocket_accept_key(), since neither is declared
in the part of the header that survives the split.
Define has_header_token() as a static inline above the border, next to the
split() declarations its two call sites already sit below, and declare the
two WebSocket helpers the way ws::impl::read_websocket_frame() already is.
The shared library's exported symbols are now identical to master's.
decode_uri was a byte-for-byte copy of decode_uri_component: it decoded every
%XX, including escapes of the reserved characters that encode_uri leaves
literal. So decode_uri was not the inverse of encode_uri and promoted an
escaped delimiter into a real one -- decode_uri("http://h/a%2Fb") returned
"http://h/a/b". Keep escapes of the reserved set encode_uri preserves, matching
JS decodeURI; non-reserved escapes still decode.
Server::make_matcher() built a std::regex for every pattern that did not
contain "/:", even though most route patterns are plain literals with no
regular expression syntax in them. Matching those went through
std::regex_match on every request, for every registered route the
dispatcher scanned before reaching the one that matches.
PathParamsMatcher already performs an exact literal comparison when it
captures no parameter, so no new matcher class is needed: a pattern with
no regex metacharacter can simply use it. Add an early return for the
zero parameter case in PathParamsMatcher::match(), and select the matcher
by also looking for the 14 ECMAScript metacharacters instead of only for
"/:". Path params keep taking precedence, so a pattern that mixes both,
such as "/users/:id/(.*)", is unaffected.
Measured with clang -O2 on macOS, scanning routes that all miss until the
last one: at 100 routes a scan drops from 10.3us to 0.46us, and end to end
throughput rises by about 24%. At 1000 routes throughput is roughly 3
times higher. Registering 5000 routes drops from about 3.0ms to about
0.9ms, since no std::regex is built for literal patterns.
This also keeps CPPHTTPLIB_REGEX_ROUTE_PATH_MAX_LENGTH confined to the
routes it is meant for. That limit rejects overlong paths before calling
std::regex_match, but until now every literal route was a RegexMatcher
too, so a literal route longer than the limit stopped matching even though
no regular expression was involved. Literal routes no longer go through
RegexMatcher, so only real regex routes are capped.
Patterns containing a metacharacter keep their current behavior, so
"/index.html" still matches "/indexXhtml" the way it always has. One
visible change: a literal route no longer populates Request::matches,
which is now a default constructed std::smatch. Path parameter routes
have always behaved that way, and Request::matches only carries useful
information for regex routes.
RegexMatcher::match() called std::regex_match() directly on the
attacker-controlled request path. For quantified patterns such as "(.*)",
std::regex_match's recursive backtracking implementation (most acute on
libstdc++) recurses roughly once per matched character, so a long enough
path can exhaust the calling thread's stack and crash the process. Verified
against real GNU libstdc++: under the default thread stack size, a path of
a couple thousand characters against a simple quantified route pattern
reliably crashed the process, well within the existing 8192-byte request
URI limit.
Add CPPHTTPLIB_REGEX_ROUTE_PATH_MAX_LENGTH (default 256) and reject paths
longer than it before ever calling std::regex_match, treating them as a
non-match instead. Confirmed the fix eliminates the crash under the same
libstdc++ build and default stack size that reproduced it.
For a non-SSL request with neither Content-Length nor Transfer-Encoding,
Server::read_content_core() fell back to reading raw wire bytes with
detail::read_content_without_length() directly, bypassing the decompressor
wrapper that the length-framed and chunked paths already use. As a result,
payload_max_length only bounded the compressed bytes read off the socket,
not the decompressed size a handler could produce from them.
Route this fallback through detail::read_content(..., decompress=true)
instead, the same helper already used below for the length-framed and
chunked cases, so the decompressed-size guard applies uniformly.
The stricter ws::Result error checks added in 6018c7f and 86d0210
exposed two backend-parity bugs in setup_client_tls_session(), shared
by SSLClient and WebSocketClient since their TLS setup was merged:
- enable_server_hostname_verification(false) had no effect on Mbed TLS
or wolfSSL for DNS hosts: mbedtls_ssl_set_hostname() and
wolfSSL_check_domain_name() bind SNI and handshake-time identity
checking together, so the identity check ran regardless of the
option, failing the handshake before the post-handshake
server_hostname_verification check was ever reached.
- On a genuine wrong-hostname failure, Mbed TLS reported the generic
Error::SSLServerVerification instead of
Error::SSLServerHostnameVerification, because
MBEDTLS_ERR_X509_CERT_VERIFY_FAILED was mapped without looking at
which verify flag actually caused it.
Fixes:
- set_sni() now takes a verify_hostname flag. wolfSSL skips
wolfSSL_check_domain_name() when it's false. Mbed TLS can't request
SNI without also arming the CN/SAN check, so it installs a verify
callback that masks the mismatch flag instead - a self-contained one
when the session has no user verify callback of its own, so it never
reads the process-wide set_verify_callback() slot another client may
have populated (this was caught by ASAN as a stack-use-after-scope:
VerifyCallbackTest.VerifyContextFields leaves a dangling lambda
there because MbedTlsSession never had a reason to consult it
before).
- map_mbedtls_error() now takes the handshake's verify flags and
reports HostnameMismatch when CN/SAN mismatch is the only one set,
matching the wolfSSL mapping and the post-handshake identity check.
- The duplicated verify-flags/error-mapping/backend_code logic in
connect() and connect_nonblocking() is factored into
fill_mbedtls_tls_error(); the duplicated flag-clearing in the two
verify callbacks is factored into mbedtls_clear_cn_mismatch(); both
use the existing hostname_mismatch_code() accessor instead of the
raw Mbed TLS macro.
Also tightens SSLClientTest.ServerHostnameVerificationError_Online to
assert the specific error code now that all three backends agree,
rather than accepting Mbed TLS's old fallback value.
Verified full non-online suite green on OpenSSL (791), Mbed TLS (737),
and wolfSSL (735), plus the split build, plus the Online
hostname-mismatch test against badssl.com on all three backends.
WebSocketClient's TLS setup already threaded
ClientTlsSessionOptions::server_hostname_verification through
setup_client_tls_session(), the same path SSLClient uses, but never
exposed a way to set it: create_stream() called setup_client_tls_session()
without an options argument, so the default (verification on) was the
only reachable value.
Add the public setter, mirroring ClientImpl/SSLClient/Client, and wire
it into create_stream()'s ClientTlsSessionOptions. Last open item from
issue #2531's WebSocketClient/SSLClient API alignment.
Issue #2531 asked for connect() to expose error detail the way
ClientImpl/SSLClient do via Result, instead of collapsing every failure
into a bare bool. The groundwork (detail::ClientTlsSessionError) was
already laid during the WebSocketClient/SSLClient dedup but left
unwired.
- Add httplib::ws::Result: explicit operator bool(), error(), and
flattened upgrade-response accessors (status(), headers(),
get_header_value(), has_header()); ssl_error()/ssl_backend_error() on
SSL builds.
- Add Error::WebSocketHandshake for upgrade-validation failures
(non-101 status, bad Sec-WebSocket-Accept, bad Upgrade/Connection
headers).
- Extract detail::parse_status_line from ClientImpl::read_response_line
and reuse it in read_websocket_upgrade_response, replacing the
previous "HTTP/1.1 101" substring match with a proper parse. Non-101
responses now surface their status and headers instead of being read
and discarded.
- Wire WebSocketClient::create_stream() to capture ClientTlsSessionError
so TLS failures (SSLServerVerification, SSLServerHostnameVerification,
...) reach the caller with backend error codes.
- Update tests and README-websocket.md accordingly.
This is a source-breaking change for callers that assign the result to
bool (e.g. bool ok = cli.connect();); if (cli.connect()) and gtest's
ASSERT_TRUE/EXPECT_FALSE(...) macros are unaffected since operator bool
still participates in contextual conversion.
Adds ws::WebSocketClient::PemMemory and a constructor overload that
installs an in-memory client certificate on the TLS context, enabling
mutual TLS for wss:// connections. The certificate is silently ignored
for ws:// URLs, consistent with the existing TLS-only setters such as
set_ca_cert_path().
Part of the interface alignment discussed in #2531.
SSLClient::initialize_ssl kept its own copy of the session setup that
detail::setup_client_tls_session already implemented for WebSocketClient.
Extend the shared function with the pieces only SSLClient needed - a session
verifier, an independent hostname verification flag, the context mutex,
Windows Schannel verification and error details - and let initialize_ssl
build a ClientTlsSessionOptions and call it. All of them default, so
WebSocketClient's call site is unchanged.
This settles one difference between the two: WebSocketClient used to call
tls::set_hostname for named hosts, which on OpenSSL turns on verification
during the handshake, while SSLClient always set SNI only and verified
post-handshake. The shared function now does the latter for both, so
tls::set_hostname loses its last caller and goes away, as does the
write-only SSLClient::verify_result_.
Certificate verification with a host name rather than an IP literal was the
one combination the WebSocket tests never covered, and it is exactly the
path this normalizes. WebSocketSSLDnsHostTest fills that in; cert2 gains a
DNS:localhost SAN so a name can be verified against it.
WebSocketClient::set_ca_cert_path took a single path and create_stream()
hardcoded an empty directory when calling detail::load_client_ca_config, while
ClientImpl has always accepted (ca_cert_file_path, ca_cert_dir_path = ""). Give
WebSocketClient the same signature and store the directory, so both clients
configure CA loading identically. The one-argument form is unchanged for
callers.
Also note at both call sites why the "load the CA config once" guard differs:
SSLClient needs call_once because one client serves concurrent requests, and
WebSocketClient does not because connect() is not safe to call concurrently
anyway.
WebSocketClient only accepted timeouts as (time_t sec, time_t usec), while
ClientImpl has taken std::chrono::duration overloads for its read, write and
connection timeouts for a long time. Add the same three overloads, forwarding
through the existing detail::duration_to_sec_and_usec helper so the split
matches ClientImpl exactly.
The template bodies go above the first split.py BORDER, next to the class, so
that the .h/.cc split keeps them in the header where instantiation needs them.
test.cc says right above the PORT constant that it is only for the
legacy fixtures and that new standalone tests must use
bind_to_any_port() instead. The six tests added with the insertion-order
work all took the shared PORT anyway, which made them one more thing
contending for it.
Move them to bind_to_any_port() plus listen_after_bind(), the pattern
the note asks for and the rest of the standalone tests already use.
HeadersOrderTest.ReceivedFieldsKeepTheirOrder sends a raw request rather
than going through Client, so send_request() gains an optional port that
defaults to PORT and leaves its other 36 callers alone.
Checked by holding PORT open from another process while running the six:
they pass, where before the change listen(HOST, PORT) would have failed.
SocketStream.wait_writable_INET binds PORT + 1 directly, and it was the
only raw listener in the file that did not set SO_REUSEPORT/SO_REUSEADDR
first. PORT + 1 is shared with three SSL redirect tests and with
VulnerabilityTest.CRLFInjectionInHeaders, so once any of those has run,
the TIME_WAIT entries they leave make bind() fail here.
That failure did not surface where it happened. The bind runs on a
worker thread, where a failed ASSERT_EQ only returns from the lambda, so
the test carried on and failed later at ASSERT_NE(disconnected_svr_sock,
-1) with nothing pointing at the port.
Within one run of the suite the test comes before everything that uses
PORT + 1, so a clean run passes; it failed when a previous process had
left TIME_WAIT behind, which made it look intermittent. Running any of
those four tests first and then this one reproduces it every time: 4 of
4 before this change, 4 of 4 passing after, with up to eight TIME_WAIT
entries on the port.
Call default_socket_options(), which is what the library does for its
own listeners and what the other raw listeners in this file already do.
FormFields and FormFiles were std::multimaps, which sort by field name.
RFC 7578 5.2 says a form processor "SHOULD send back results in order"
and that "Intermediaries MUST NOT reorder the results", so a handler
walking req.form.fields saw the parts alphabetised rather than as they
were sent, and a body received for forwarding could not be reproduced.
Point both at the insertion-ordered container #2523 generalized, with
std::equal_to since field names are case-sensitive. Entries sharing a
name already kept their relative order under std::multimap; what is
recovered here is the order across different names.
Server::read_content() keeps a FormFields::iterator alive across the
content callbacks that fill the part it points at, which is the one
thing this container could have broken: it is vector-backed, so a later
emplace can reallocate and leave an older iterator dangling. The code is
safe because the iterator is reassigned by the same emplace that could
reallocate, and is only read while the flag set alongside it says so.
ContentSurvivesContainerGrowth pins that down with 64 parts, enough to
grow the vector through seven reallocations; reverting the reassignment
makes it abort under ASan rather than fail quietly.
Growth also never copies a part's payload: the parser inserts the entry
with an empty content and appends the body bytes afterwards, and both
mapped types are nothrow-move-constructible, so a reallocation steals
the string buffers rather than deep-copying them.
Params was a std::multimap, which sorts by parameter name. Parsing a
query string therefore threw away the order it arrived in, and building
one back out of Params handed the caller an alphabetised query rather
than the one they wrote. ClientImpl::send() takes that path whenever a
request carries Params without a query already in its path, so a caller
signing its query string could not reproduce the order it asked for.
normalize_query_string() exists in part to work around exactly this.
Generalize the container #2520 introduced for Headers into
detail::insertion_ordered_multimap<Mapped, KeyEqual> and alias both
types to it. Params passes std::equal_to, so parameter names stay
case-sensitive, while Headers keeps matching field names
case-insensitively. The name says insertion_ordered because in STL
vocabulary std::multimap is the ordered one, which is the reading this
change exists to correct.
This is a correctness change, not a performance one. Measured on the
real paths against the previous implementation, parse_query_text() over
eight parameters goes 931.5ns -> 935.1ns and params_to_query_str()
374.7ns -> 377.2ns; both are inside the run-to-run noise. The container
is a small share of that work, most of which is the string building in
decode_query_component().
Params picks up the same API changes Headers took in #2520: iterators
follow std::vector rules, value_type is std::pair<std::string,
std::string>, and insert(hint, value) is gone. Headers itself becomes an
alias rather than a class, so the names it appears in mangle differently
again; #2520 has not shipped in a release yet, so this costs nothing on
top of the break already there.
RFC 9110 5.3 lets a coding list be split across several Transfer-Encoding
lines, which combine, in the order the lines were received, into one
comma-separated list. RFC 9112 6.1 then frames the message as chunked
only when chunked is the final coding of that combined list.
is_chunked_transfer_encoding() could not apply that rule while Headers
was an unordered_multimap, since the order of the lines was not
recoverable, so it fell back to reporting any message naming chunked on
any line as chunked. Headers now preserves the order the lines were
received in, so read the final coding directly: the last token of the
last line.
The fallback erred toward reporting chunked because mis-reading a
chunked message as unframed leaves its body in the socket, where a
keep-alive connection parses it as a smuggled request. That direction is
no longer needed. A request whose combined list ends in something other
than chunked is now reported as not chunked, and process_request()
answers 400 and closes rather than letting it reach the "no body" path,
which is what it already did for the single-line "chunked, gzip" form.
So `Transfer-Encoding: chunked` followed by `Transfer-Encoding: gzip` is
rejected instead of being read as chunked, and `gzip` followed by
`chunked` is still accepted.
A trailing line carrying no coding at all now leaves the combined list
ending in nothing rather than inheriting the coding from the line
before it.
RFC 9110 5.3 makes the order of header fields sharing a field name
significant, but Headers was a std::unordered_multimap, which gives no
ordering guarantee for equivalent keys. libstdc++ hands duplicates back
in reverse insertion order while libc++ uses insertion order, so
get_header_value() returned a different field depending on the platform,
and code that picks a value out of an accidentally or maliciously
duplicated field name had no way to say which one it wanted.
Replace Headers with a small container that keeps the fields in the
order they were received or set. Storage is a flat vector and lookup is
a linear scan, which beats hashing for the at most
CPPHTTPLIB_HEADER_MAX_COUNT fields a message carries. begin()/end() walk
every field, while find() and equal_range() hand back the same iterator
type restricted to one field name; equality compares only the position,
so a restricted iterator still compares equal to end(). Erasing an
equal_range() therefore removes only the fields with that name and
leaves interleaved fields alone.
std::multimap was the smaller change but sorts by field name, which
would stop control data such as Host from leading the message. Instead
Host is now prepended via emplace_front() so it keeps its place at the
front of a request.
Two side effects worth noting: incrementing past the last field of a
name now saturates at end(), so an out-of-range id passed to
get_header_value() returns the default instead of running off the
container as it did before; and iterators follow std::vector rules, so
they are invalidated by insertion.
Fixes#2509
Explain why `addr_` is reset before `close()`: `munmap()` must not be
called with the sentinel. The test also checks `size()`, since the hazard
is a stale size paired with a sentinel `data()`.
The added test only exercised a 403 with a single range, leaving three
paths that were equally broken untested: a suffix range, whose first_pos
is still -1 when the bounds asserts are compiled out under NDEBUG; two
ranges, where apply_ranges() makes no boundary for a non-206 response and
the multipart branch wrote an empty one; and a 2xx that is not a 206,
which was validated but still announced the full content length.
Fold the copied route back into /streamed-with-range with a status query
parameter, following the ?error idiom already used there.
Since 8bba34e, prepare_content_receiver() rejected every non-empty
Content-Encoding that create_decompressor() could not handle, conflating an
unrecognized content coding with a known one whose support was not compiled
in. A response labeled `Content-Encoding: UTF-8` -- a header some servers
misuse to advertise a character set -- therefore failed with 415, which the
client surfaced as the generic Error::Read. Before that commit the raw body
came back untouched.
Restore that behavior: reject only a coding cpp-httplib recognizes but was
not built with, and leave an unrecognized one (including "identity") alone.
Match codings case-insensitively while here, as RFC 9110 8.4.1 requires;
otherwise `Content-Encoding: GZIP` would look unrecognized and hand back a
still-compressed body.
open_stream() had the mirror-image problem. It silently passed the payload
through whenever create_decompressor() returned null, so a gzip response on
a build without zlib reached the caller still compressed, and it never
checked is_valid() -- gzip_decompressor::decompress() only asserts, so a
failed inflateInit2() was undefined behavior in release builds. It now
applies the same policy as the buffered path.
Add Error::UnsupportedContentEncoding so callers can tell this apart from a
read failure, and report an unusable decompressor as Error::Compression. The
status read_content() writes was previously discarded into an uninitialized
dummy_status.
* Close the listening socket in stop() even when not serving
Server::stop() released svr_sock_ only under `if (is_running_)`, and
is_running_ is set inside listen_internal(). A server that bound with
bind_to_port() / bind_to_any_port() and never reached listen_after_bind()
therefore kept its listening descriptor for the life of the process:
~Server() is defaulted and svr_sock_ is a bare atomic<socket_t>, so nothing
else closes it. The port stayed held too.
Close whenever the socket is open instead. The exchange already made this
safe against a concurrent accept loop, so dropping the is_running_ gate also
removes a TOCTOU between the check and the exchange.
* Fail listen_after_bind() when stop() already closed the socket
With stop() now releasing a bound-but-not-serving socket, a stop() that
lands between bind and listen used to slip through listen_internal():
the accept loop saw INVALID_SOCKET, never iterated, and returned success
without ever serving, pulsing is_running_ just long enough that a
wait_until_ready() caller could miss it and spin forever. Return false
instead and mark the server decommissioned the way any failed listen
does, so waiters wake up.
Also drop the assert() in stop(). It read is_running_ and svr_sock_
separately, which is exactly the race the exchange removes: a second
stop() while the accept loop is still unwinding sees is_running_ true
and svr_sock_ already INVALID_SOCKET, aborting debug builds.
The regression test checks the released port with a raw connect()
instead of a Client request: against the old code the connection is
accepted into the backlog and never answered, which would hang the test
rather than fail it.
Follow-up to 49b921b.
Move the duplicated addr_map lookup into detail::apply_addr_map, shared by
ClientImpl::create_client_socket and WebSocketClient::connect.
Add a WebSocketClient test for a hostname mapped value, so that path has
the same coverage as the Client one. Guard its teardown with scope_exit:
a failing ASSERT_TRUE returns from the test body, and destroying a still
joinable std::thread calls std::terminate, taking the whole binary down.
Document set_hostname_addr_map in README. It had no entry at all.
addr_map_ only accepted IP literals as mapped values; a non-IP value was
passed as the `ip` argument and rejected by getaddrinfo's AI_NUMERICHOST
path. The lookup in ClientImpl::create_client_socket and
WebSocketClient::connect now checks the mapped value with
detail::is_ip_address: IP literals keep the existing AI_NUMERICHOST path,
while hostnames are passed as the connect host so they get resolved.
host_ is never touched, so it keeps supplying the Host header and SNI in
both cases.
is_ip_address() moved into the non-SSL detail block so that both client
code paths can use it without CPPHTTPLIB_SSL_ENABLED.
This also fixes the documented Unix domain socket client example, where
the mapped value is a socket path: it previously took the AI_NUMERICHOST
path and never reached the AF_UNIX branch in create_socket.
Follow-up to #2514. The rebuilt handshake wrote the request line straight
to the socket, so a header rejected by check_and_write_headers left a
truncated "GET /ws HTTP/1.1" sitting in the peer's buffer before the
connection was torn down, and every header cost its own small write.
Build the request into a BufferStream and flush it in one go, matching
ClientImpl::write_request. The new test drives a raw listener and asserts
the peer sees a clean EOF with zero bytes; without this change it observes
18.
Also fold WebSocketTest.HostHeaderInHandshake into
WebSocketTest.DefaultHeadersInHandshake, which covers the same Host
assertion through the capture helper #2514 introduced.
Replace the hand-built upgrade request string in
perform_websocket_handshake with the same Request,
write_request_line, and check_and_write_headers path used by
ClientImpl::open_stream. Add WebSocketClient::prepare_default_headers
to inject Host and User-Agent defaults; protocol-mandatory headers
(Upgrade, Connection, Sec-WebSocket-Key/Version) are always
overwritten.
ClientImpl::open_stream() passed the caller-supplied path straight to the
request line, so it ignored path_encode_ entirely and always behaved as if
set_path_encode(false) had been called. The same path therefore produced
different bytes on the wire depending on which API was used:
Get() "/a b" -> GET /a%20b HTTP/1.1
open_stream() "/a b" -> GET /a b HTTP/1.1
A space is the request-target delimiter, so the streaming form is not merely
inconsistent: an RFC 9112 conformant server reads the target as "/a" and the
version as "b". Non-ASCII bytes and '+' diverged the same way, the latter
changing the value a server that decodes '+' as space sees.
Extract the path/query splitting and encoding out of ClientImpl::write_request
into detail::encode_request_target() and call it from both paths, so the
encoding rule lives in one place. open_stream() appends Params before
encoding, matching ClientImpl::Get(path, params), which builds its target the
same way.
Note a behavior change: with path encoding enabled, CR/LF in the target is now
percent-encoded and sent rather than rejected with Error::Write, matching
Get(). This is not a weakening of the CR/LF guard in write_request_line() --
that check is independent of path_encode_ and still backstops
set_path_encode(false), where encode_path() does nothing.
Mbed TLS has no SSL_peek() equivalent, so is_peer_closed() (called after
every SSL request write) probed liveness with a real 1-byte
mbedtls_ssl_read() and discarded whatever it read. If the response had
already arrived by the time the probe ran — plausible under CI load or
plain OS scheduling — the probe silently ate the first byte of the
status line, corrupting the response and surfacing as a fast
"Failed to read connection" failure.
This was the root cause of the long-standing MbedTLS-only CI flakiness
(ServerTest cases failing intermittently on Ubuntu and macOS), previously
worked around by reducing gtest shard parallelism. Fix: push the probed
byte back into MbedTlsSession and have tls::read()/pending() account for
it, so no data is lost.
Also fix a second, unrelated flake: ProxyTunnelTest.
OriginReturning407InsideTunnelDoesNotLeakProxyDigest used "localhost" for
its client while the test's proxy harness only listens on 127.0.0.1;
under dual-stack resolution this could race with another test's server
on ::1 using the same ephemeral port. Pin the test to 127.0.0.1.
With the root cause fixed, restore the mbedTLS CI jobs (ubuntu,
ubuntu-26.04, macOS) to the default shard count instead of the
previously reduced SHARDS=1/2 mitigation.
Only 8 of 423 call sites logged the actual error on failure. The
MbedTLS-backend flaky CI failure (connection-level ASSERT_TRUE(res),
~20-30ms) keeps landing on assertions without this diagnostic, so the
real error code has never been captured. Broadens the existing
GetWithRange-only logging (a4d7066) to every plain ASSERT_TRUE(res);
site, no behavior change.
Server::process_request only checked that trusted_proxies_ was
non-empty before deriving req.remote_addr from the X-Forwarded-For
header. It never verified that the actual TCP peer (remote_addr) was
itself one of the trusted proxies, so any client connecting directly
to the server could spoof remote_addr simply by sending an arbitrary
X-Forwarded-For header.
Now X-Forwarded-For is only honored when the connecting peer address
matches an entry in trusted_proxies_.
* Add Mbed TLS 4.x support (PSA Crypto) for macOS
Auto-detect Mbed TLS 4.x via MBEDTLS_VERSION_MAJOR and adapt the backend:
- Include psa/crypto.h and drop the headers removed in 4.x (ctr_drbg,
entropy, md5, sha*), gated behind the version macro.
- Compute MD5/SHA-256/SHA-512 via PSA (psa_hash_compute) and initialize
PSA Crypto once with std::call_once.
- Drop the explicit entropy/CTR-DRBG RNG (PSA provides the TLS RNG) and
skip the RNG-callback overloads of pk_parse_key/pk_check_pair on 4.x.
- Retry on a TLS 1.3 NewSessionTicket (the 4.x default) in connect, read,
write and is_peer_closed via a single mbedtls_is_session_ticket() helper,
so online HTTPS works, including large redirected downloads where the
ticket arrives mid-write.
Note V4 implies V3, so 3.x-only paths now check V3 && !V4.
Build systems (macOS): the CMake config and pkg-config shipped by Homebrew
resolve 4.x transitively, so CMakeLists.txt and meson.build need no change
for linking; the Makefile links libtfpsacrypto when present, else
libmbedcrypto.
Tests: generate the encrypted client key as both PBES2-AES (3.6+/4.x,
OpenSSL, wolfSSL) and PBES1-3DES (Mbed TLS 2.28) and pick by version, since
4.x dropped DES and 2.28 lacks PBES2. Also generate the IP-host certs in
test/meson.build to match gen-certs.sh and CMakeLists.txt.
* CI: test Mbed TLS 4.x on macOS, 3.x on Ubuntu 26.04
Homebrew's default mbedtls is now 4.x, so switch the macOS build and CI job
to it (drop the mbedtls@3 pin). That leaves 3.x (Ubuntu 24.04 apt ships 2.28,
macOS now 4.x) uncovered, so add an ubuntu-26.04 job whose apt provides Mbed
TLS 3.6. Net coverage: 2.28 (ubuntu-latest), 3.6 (ubuntu-26.04), 4.2 (macOS).
ubuntu-26.04 is a public-preview runner image; fold it into the main ubuntu
matrix once ubuntu-latest moves to 26.04.
* Document Mbed TLS 4.x support and libtfpsacrypto rename
Update README.md and the tour's TLS setup pages (en/ja) to note that
Mbed TLS 4.x is now auto-detected and that it renames libmbedcrypto
to libtfpsacrypto.
