tls::shutdown() on OpenSSL called SSL_shutdown() a second time to wait
for the peer's close_notify. An idle keep-alive client never sends one,
so closing its connection held the worker thread until the read timeout,
and Server::stop() waited for it. Send close_notify and return, as the
Mbed TLS and wolfSSL backends already do.
The server wrote 100 Continue as soon as it saw the expectation, before
pre_routing_handler, pre_request_handler, or routing ran. A request
those handlers rejected, or one that matched no route, still invited
the client to send a body the server would never read.
Defer the interim response until the body is about to be read. If the
request is answered without reading the body, 100 Continue is never
sent and the connection is closed, since whether and when the client
sends the body is unknown.
Also treat a 417 returned by expect_100_continue_handler as the final
response. It used to be written as a bare status line, after which the
request was processed and a second response was written.
The WebSocket upgrade path matched the route and switched protocols
without setting req.matched_route or calling pre_request_handler, so a
check placed there (e.g. authentication) never ran for WebSocket
routes. Set matched_route and run the handler before the upgrade; if it
handles the request, reply with a regular HTTP response instead of 101.
Also write rejected upgrade responses (from pre_routing_handler too)
with write_response_with_content, so they carry Content-Length. Without
it, a client reading the body waited until the keep-alive timeout.
`sed -i ''` is BSD-only: GNU sed takes the '' as the script and the expression as a file name, so `just release --run` failed on Linux before touching anything.
199d7ee made read() return the new ReadResult::Timeout for every read
timeout and leave the connection open. The compile-time server default
(CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND, 300s) is always in
effect, so a handler written as `while (ws.read(msg))`, the form the
README's Quick Start uses, no longer ended when a peer went quiet:
Timeout is non-zero, so the loop ran its body again with the previous
message still in `msg`, and the worker the backstop is meant to reclaim
was never released. Nothing caught it because every test of the new
result set a timeout explicitly and checked the result by value, and the
heartbeat tests keep the connection alive with pings.
The two timeouts mean different things. One the caller sets through
set_read_timeout() is a request for control back, and is reported as
Timeout on a still-open connection. The compile-time default is a
backstop against a peer that has gone quiet, and elapsing it is now a
failure again: read() returns Fail and closes the connection, as it did
before 199d7ee. WebSocket tracks whether set_read_timeout() was called,
and WebSocketClient carries the same flag over to the WebSocket it
creates on connect().
Tests use the heartbeat binary, which compiles both defaults down to 3s:
a `while (ws.read(msg))` server handler runs its body once and exits
when the client falls silent, and a client that never set a timeout gets
Fail with the connection closed. The README and cookbook now say which
timeout produces Timeout.
Claude-Session: https://claude.ai/code/session_01EF5uZ1X2kaHhqJ8VgfjVaQ
ProxyTest, RedirectTest.HTTPBin*, KeepAliveTest and ProxyTest.SSLOpenStream
still sent their requests through the squid proxies to the external
httpbingo.org, so an upstream hiccup there failed CI with no code change
involved (KeepAliveTest.SSLWithDigest got a 502 on its first /get).
Switch them to the "httpbin" container (nginx + go-httpbin) that
BaseAuthTest/DigestAuthTest already use. go-httpbin serves /get,
/redirect/n and /digest-auth the same way, so the test logic is
unchanged; the SSL variants disable certificate verification for the
self-signed test cert, as BaseAuthTest.SSL does.
RedirectTest.YouTube* is left pointing at youtube.com since it exercises
a real cross-host, http -> https redirect chain.
Claude-Session: https://claude.ai/code/session_0148ZAzsuYRYXkwcA7UFeh95
The intermittent failures it was tracking stopped after the graceful
drain before close in 8e702d3: no windows-without-SSL test failure on
master since 2026-08-09. Drop the reporting step, the issues: write
permission it needed, and the run_tests step id that only it used.
Claude-Session: https://claude.ai/code/session_0148ZAzsuYRYXkwcA7UFeh95
These tests exercise the squid proxies by hitting /basic-auth and
/digest-auth on an external httpbin-style site. That site's identity has
already moved twice (httpbin.org -> httpcan.org, per #2300) chasing
uptime, and httpcan.org itself is now down (Cloudflare 502 from its
origin), failing CI with no code change involved.
Adds two containers to the existing squid docker-compose stack instead:
go-httpbin (mccutchen/go-httpbin) as the backend, and an nginx sidecar in
front of it under the single "httpbin" hostname so both the NoSSL tests
(port 80) and the SSL tests, which CONNECT-tunnel through the proxy to
port 443, resolve the same name -- go-httpbin only listens on one port at
a time, so it can't serve both protocols itself. nginx uses the repo's
existing self-signed test cert; the SSL client tests already disable
verification for it like other self-signed-cert tests in this suite.
go-httpbin was picked over the more feature-complete kennethreitz/httpbin
after finding the latter accepts a wrong digest-auth username as long as
the password matches -- confirmed with a direct curl against the
container, unrelated to anything in this repo. go-httpbin correctly
rejects both. The trade-off is losing SHA-512 digest-auth coverage here,
since go-httpbin only implements MD5 and SHA-256; nothing else in the
suite exercises SHA-512 digest auth against a live server. Response body
assertions are adjusted to go-httpbin's actual JSON shape (an added
"authorized" field, no "algorithm" field), and the domain changes from
httpcan.org to the self-hosted "httpbin".
This only affects 'make proxy'/'make proxy_mbedtls'/'make proxy_wolfssl'
and the Proxy Test CI workflow -- the default 'make' target is untouched.
The previous commit pinned CI and the pre-commit hook to a fixed
clang-format 23.1.0, but the maintainer develops on macOS against
whatever version `brew` currently installs, which changes over time
as Homebrew updates the formula.
style-check now runs on macos-latest and installs clang-format via
`brew install`, so it tracks the same moving target the maintainer's
Mac does. The pre-commit hook switches from pre-commit's own pinned
mirror to a local hook that shells out to the system clang-format,
so a local commit and CI both go through the same Homebrew-installed
binary rather than two independently versioned copies.
Trade-off: this reintroduces the non-determinism a fixed pin avoids
-- a commit's style-check result can now change over time as Homebrew
updates the formula -- but that mirrors how the maintainer already
develops, which is the point.
Also install coreutils in CI: the style_check Makefile target needs
grealpath's --relative-to, which the macOS-native realpath lacks.
CI relied on ubuntu-latest's default apt clang-format (18.1.3), while the
pre-commit hook was pinned to a different 18.x build. Neither tracked a
specific, deliberately-chosen version, and the two could drift from each
other and from whatever a contributor has installed locally.
Both now pin the same clang-format 23.1.0 (latest stable), installed via
pipx in CI. Also scope the pre-commit hook's file matcher to the same set
of files test/Makefile's style_check target checks, since the previous
\.(cpp|cc|h)$ pattern reached into vendored code (test/gtest,
benchmark/crow) that must stay untouched.
Reformat httplib.h's brace-init spacing to match 23.1.0's output.
Defined next to its non-template overload, it landed in the part of the header
that test_split compiles into httplib.cc, leaving a user TU's instantiation
with nothing to link against. The WebSocketClient overload of the same name has
always sat up with the class definitions; this one belongs there too.
Only test_split sees it -- a header-only build instantiates the template
wherever it is written, which is why the regular test target stayed green.
read() collapsed every failure into Fail and marked the connection closed with
it, so a read timeout could not be used to get control back and send on the
same connection -- it killed the connection instead. The information was
already there and thrown away: SocketStream::read records Error::Timeout, and
read_websocket_frame flattened it into a bool.
ReadResult gains Timeout, reported only when the timeout elapsed on a frame
boundary with nothing consumed, which is the only case where the stream can be
read again. Every multi-byte field now loops until it has its bytes, which also
fixes a frame header straddling the read buffer's boundary failing the frame:
Stream::read is allowed to return less than asked for, and only the payload
was reading in a loop.
ws::WebSocket::set_read_timeout() lets a server handler bound its own reads,
and WebSocketClient::set_read_timeout() now reaches an already-open connection
instead of only seeding the next connect().
The read timeout macro splits in two. A client waits forever by default -- a
read timeout is the caller's tool for taking back control, not a liveness
check, which is ping/pong's job -- while a server keeps the 300s that reclaims
a worker from a peer gone quiet. Defining the old name still sets both.
Also record a reason on the two WebSocketSSLStream::read failure paths that
returned -1 without one, so get_error() cannot report a previous call's
timeout, and make SocketStream's read timeout atomic now that it can be
changed while a read is in flight.
parse_www_authenticate() accepted any WWW-Authenticate: Digest
challenge that carried at least one auth-param, so a server sending
e.g. Digest qop="auth" with no realm/nonce would make it through.
make_digest_authentication_header() then dereferences auth.at("realm")
and auth.at("nonce") unconditionally, throwing std::out_of_range with
no try/catch on the retry path, which terminates the client process.
Now require both realm and nonce before treating a Digest challenge as
usable, same as if no Digest challenge were present at all.
The reporting step for issue #2533 was gated on failure(), which is true
when any step in the job failed. Two build failures on feature branches
were posted to the issue as flaky test recurrences, with the body falling
back to "Could not extract failed test name" because no test had run.
Gate it on the test step's own conclusion instead, and skip posting when
no [ FAILED ] line turns up in the shard logs. The explicit failure()
stays because an if expression with no status check function gets an
implicit success().
CPPHTTPLIB_MULTIPART_FORM_DATA_FILE_MAX_COUNT is enforced only in
Server::read_content(), where parts are accumulated into req.form. The
streaming ContentReader path keeps nothing and was never in scope, but
this was undocumented (GHSA-923p-8q8g-xcqj). Note the split in the README
and show how to bound the part count from inside a ContentReader handler.
write_content_with_progress() advances its offset only by what the provider
writes, so a provider that reported success without writing anything and
without calling done() was handed the same offset and length again on the next
pass. With the peer still connected it spun there, re-entering the provider as
fast as the loop could run.
make_file_body()'s provider was one way to reach this and was fixed in #2566,
but any user-supplied provider can do the same. Treat a pass that makes no
progress as a short body, which is how done() called early is already handled.
parse_accept_header() rejected any Accept value with a leading, trailing
or doubled comma, and Server::process_request() validates Accept before
routing, so "Accept: text/html," was answered 400 Bad Request on every
route.
RFC 9110 Section 5.6.1.2 requires a recipient to parse and ignore empty
list elements in a #rule list, so those values are legal. split() already
trims each element and skips the empty ones, which made the guard inside
the callback unreachable as well; drop both and let the empty elements
fall away. The header length limit bounds how many a sender can send, so
ignoring all of them cannot be used as a denial-of-service vector.
get_combined_header_value() keeps skipping empty field lines, but that
skip is no longer observable through a request now that a stray comma
parses cleanly, so it gets its own test.
expect_split_multipart_ok() carries a comment saying the request sends
"Connection: close" so the response drain ends as soon as the server has
answered, but the header itself never made it into the request, so both
callers kept idling until the read timeout instead.
Add the header. EpilogueSplitAcrossReadsIsIgnored and
InitialBoundarySplitAfterLongPreamble each drop from about 3.1s to about
0.11s.
Start-Process joins ArgumentList entries with spaces, so /DIR=C:\Program
Files\OpenSSL reached Inno Setup as /DIR=C:\Program and the install landed
there. Linking still succeeded, because the import libraries were present
under that path, and the failure surfaced only when gtest_discover_tests ran
the test binary: exit code 0xc0000135, DLL not found, since PATH pointed at
C:\Program Files\OpenSSL\bin.
Quote the value, and assert that the import libraries and runtime DLLs are
where we expect before exporting PATH, so a misplaced install fails loudly at
the install step instead of quietly at load time.
The Chocolatey openssl package hardcodes a versioned slproweb URL in its
install script, and slproweb keeps only the newest build of each OpenSSL
branch. Every OpenSSL release therefore deletes the file the current package
points at, and "windows with SSL" fails at the install step with a 404 until
someone respins the package. That is what broke the job today: the package is
still at 4.0.1 while slproweb has moved to 4.0.2.
slproweb publishes a JSON manifest of its current downloads, linked from the
download page and updated at the same time as the files themselves. Read that
and take the newest 64-bit 4.x installer from it, so the URL is always live.
The SHA512 in the manifest is verified before the installer runs.
The silent flags are the ones the Chocolatey package used. /DIR pins the
install location that the CMake step already finds, instead of relying on a
registry lookup. PATH and OPENSSL_CONF are exported the same way the package
set them.
Staying on 4.x is deliberate: it keeps this job on the OpenSSL 4.0 series
rather than dropping to the 3.6 that vcpkg would provide.
close() drained the peer's Close reply with its own frame read. If an
application reader thread was inside read() at that moment, two threads
parsed frames off one stream: read_websocket_frame()'s payload loop keeps
reading until it has the declared length, so bytes stolen by the drain
were silently replaced with bytes from further along the stream. The
in-flight message kept its correct length but got the wrong content.
Add a read_mutex_ that marks which thread owns the stream's read side.
read() holds it for the whole call. close() sends the Close frame, then
drains the peer's reply (RFC 6455 7.1.1) only if it can try_lock the
mutex; otherwise it returns immediately, leaving the stream entirely to
the thread already reading it. This also fixes close() blocking for the
full close timeout when a reader thread was parked waiting on a peer
that never replies.
Add WebSocketTest.CloseDoesNotStealBytesFromConcurrentRead, which drives
a raw TCP peer that stalls mid-payload to force the race; it fails
reliably against the old code and passes against the fix.
Update README-websocket.md: close() during a concurrent read() is now
supported.
The harness had a single endpoint returning a 12-byte set_content() body.
That is the one case where the response line, the headers and the body
already share a single write(), so any change to the write path measured
as noise. Comparing a gather-write branch against its merge base reported
0.993x at p = 1.000 while the same branch moved static-file throughput by
a quarter and TLS throughput by nearly half in both directions.
The server now also serves a large set_content() body and small and large
files from a mount point, over HTTPS when a certificate is given, with
--path, --large-mib and --tls selecting the combination.
ab.sh now compiles the harness from the invoking worktree instead of each
ref's own copy, so both refs run an identical workload and a ref that
predates a harness change stays measurable. Only httplib.h varies, through
-I. --timeout is exposed because bombardier's 2s default aborts large TLS
responses, which then fails the non-2xx check.
Drop the now-redundant has_header guard (get_header_value already
returns "" for a missing header, which the length check rejects),
name the "Bearer " prefix once, and cite RFC 9110 to match the
file's convention. Convert the regression test to the table-driven
form used elsewhere in test.cc and move it out of the middle of
GetHeaderValueTest so that suite stays contiguous.
detail::parse_www_authenticate() assumed a single challenge starting at
the first space in the field value and read only its first occurrence,
so a Basic challenge listed before Digest (or split across two field
lines, as some servers do) hid the Digest challenge entirely, and a
second Digest challenge with different parameters (RFC 7616 offering
both SHA-256 and MD5) could mix params from both. Combine repeated
field lines the same way the other list-valued headers do, then split
on commas that aren't inside a quoted-string so a quoted realm can
contain a comma, and track which challenge each auth-param belongs to
by the auth-scheme token that starts it. Also require at least one
auth-param before reporting a Digest challenge as found, since an
empty challenge can't produce a usable Authorization header.
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.
It was defined as a static inline above the border line so that the split
build would not turn it into an exported symbol of the shared library, which
kept abidiff from reporting an added function. That put an internal helper's
location at the mercy of a CI check rather than of where it belongs: it reads
a header field the same way get_header_value() and get_combined_header_value()
do, and it is the closest sibling of the latter, both being about a list-valued
field spread over several field lines.
Define it as a plain inline beside them and forward-declare it with the
split() family it calls. Adding a symbol is a source and binary compatible
change, so let abidiff report it.
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.
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.
T04 (mTLS) had grown a "WebSocketClient" subsection describing
wss:// client certificates, and c12/t02 were getting similar
WebSocketClient asides for timeouts and CA paths. The Cookbook's
own index already separates WebSocket into its own category
(W01-W04) from TLS/Security (T01-T05) and Client (C01-C19), so
burying WebSocketClient specifics inside those pages fought the
site's structure.
Move that content into two new recipes under the WebSocket
category instead:
- W05: wss:// TLS setup (set_ca_cert_path CA directory parity,
PemMemory client certificate)
- W06: WebSocketClient's three timeouts, including the recently
added chrono overloads
T04, T02, C12, and W01 now carry a single reference link to the
new pages instead of duplicated explanations, matching the site's
existing cross-link convention.
While rewriting T04's client-side section, noticed it documented
SSLClient's file-path constructor but not its PemMemory one, even
though the server-side section covered both forms for SSLServer.
Added the missing PemMemory example so both sides are symmetric.
WebSocketClient::set_connection_timeout (both the time_t and
chrono overloads) was missing from README-websocket.md's API
reference and the timeout example, even though set_read_timeout
and set_write_timeout were both listed.
README.md never documented the PemMemory in-memory constructor
that SSLServer and SSLClient both have, so mTLS setup only showed
the file-path form. Add a "Mutual TLS (mTLS)" section covering
both forms for server and client, and note that
ws::WebSocketClient's wss:// constructor takes the same PemMemory
struct.
Also note, next to Client::set_interface, that WebSocketClient has
the same method, matching the existing cross-reference for
set_hostname_addr_map right below it.
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.
Temporary instrumentation to track the intermittent windows-without-SSL
failures reported in #2533. When the job fails on a push, it posts the
run URL, commit, and per-shard failed-test lines as a comment on the
issue, building up failure-pattern history automatically.
This should be removed once the root cause is found and fixed.
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.
ClientImpl::prepare_default_headers and WebSocketClient::prepare_default_headers
each built the Host value with the same AF_UNIX special case and appended the
same User-Agent. Move both into detail:: so there is one copy.
The Host helper returns only the value, because the two callers disagree on
where it goes: ClientImpl prepends it per RFC 9110 5.3, WebSocketClient appends.
The User-Agent helper takes the Request, since it has to consult and set a
header rather than compute a string, and it stays inside ClientImpl's
content_receiver branch so that path keeps sending no User-Agent.
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.
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.
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.
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.
The first run of this workflow exposed three problems.
Crow's amalgamated header includes <asio.hpp>, which no runner provides,
so the build died immediately. It compiled locally only because CPATH
happened to point at Homebrew's include directory. Install asio
explicitly, and on macOS pass its include path through CROW_CXXFLAGS.
The macos runner image has no Go, so bombardier could not be installed.
Add actions/setup-go, which also pins a known toolchain on Linux.
Worst of all, the ubuntu job reported success. `make ... | tee` returns
tee's status, so the failed build was invisible. Enable pipefail. That
alone is not enough: every recipe in benchmark/Makefile ends in `kill`,
so make still exits 0 when bombardier itself fails to run. Assert that
the expected number of "Reqs/sec" lines came out.
benchmark/Makefile has always been local-only, so the numbers it produces
were never recorded anywhere. Wire it up to a manual workflow so a run can
be kicked off and its output kept in the job summary.
This does not gate anything: it reports absolute throughput for the
current ref, with Crow v1.3.1 alongside for reference. Absolute req/s is
only comparable against other runs on the same runner type, which is why
the ref, runner and load parameters are recorded next to the numbers.
Use benchmark-ab instead when the question is whether a specific change
made things faster; comparing absolute numbers across runs cannot answer
that.
Linux and macOS only. Windows needs benchmark/Makefile rewritten first,
since it relies on nc, & and kill.
The existing test_benchmark asserts a single request completes within
5ms, which catches gross connection-setup regressions but cannot see
throughput changes: the effects we care about are tens of microseconds
per request, a hundredth of that resolution. There was no way to answer
"does this patch make the server faster" other than measuring by hand.
Absolute req/s is not usable for that. Running the same binary five
times on an idle 8-core machine gave 53.9k to 74.6k req/s, and shared CI
runners are noisier still, so a number printed per push says nothing.
Build both refs and measure them alternately in one session, flipping
the order each round to cancel ordering bias, then report only the ratio
of the medians. Whether that ratio means anything is decided by an exact
permutation test rather than by comparing the change against the min/max
spread - a single slow round is enough to make a spread check give up,
while the rank test rides it out.
Validated against a patch that removes a redundant poll() per request:
individual measurements ranged 41.7k-93.9k req/s, yet nine rounds
resolved a 1.244x speedup at p = 0.019. The same data truncated to five
rounds was inconclusive, so the workflow defaults to nine.
Manual dispatch only, Linux and non-SSL for now, and it never fails the
build - this is a measurement, not a test.
The accept queue was limited to 5 pending connections, which overflows
easily under connection churn or a burst of simultaneous connects (LB
health checks, thundering herd on restart). On overflow the kernel
silently drops the ACK rather than failing fast, so clients stall on
SYN/ACK retransmission backoff.
Benchmark (bombardier -c 10 -d 10s, 3 trials, interleaved with the old
binary) shows max latency dropping from 48-89ms to 5.8-11.2ms, while
p99 is unchanged - the fix affects only the extreme tail, as expected.
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.
The is_field_name(name) && is_field_value(value) predicate was repeated
across five output paths (set_header, write_headers, write_content_chunked
trailer, perform_websocket_handshake, check_and_write_headers). Introduce
fields::is_field_valid(name, value) and route all five through it so the
CR/LF-injection guard has a single definition. 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_.
The from_chars template is instantiated with unsigned types (e.g.
uint64_t for Content-Length), where unary minus on `result` triggers
MSVC warning C4146, failing the 32-bit build under warnings-as-errors.
Use `T(0) - result`, which yields the identical two's-complement value
without the warning.
Replace the strtoull + errno + cast-back dance in get_header_value_u64
with the existing hand-written detail::from_chars, which reports
result_out_of_range at size_t width. This detects the 32-bit truncation
case directly (instead of via a separate cast-back comparison), drops
the reliance on the global errno, and keeps the parsing locale-
independent and consistent with the rest of the codebase.
Cookie and Cookie2 headers were forwarded to the new host when following
a cross-origin redirect, even though Host, Proxy-Authorization, and
Authorization were already stripped. Add them to the removal list so
session cookies are not leaked to a different origin.
shutdown_and_close() freed the TLS session before ws_->close() sent the
WebSocket close frame. The WebSocket's SSLSocketStream keeps a raw pointer
to that session, so sending the close frame then read/wrote a freed SSL
object. Reorder so ws_->close()/ws_.reset() run while the session is still
alive, then free the session (GHSA-w7p7-f35j-mw7q).
Server::apply_ranges computed the correct Content-Length header for
body-based responses but never updated content_length_, so the
Logger callback always saw 0. Set content_length_ to the final body
size (post-range/post-compression) alongside the header.
SSLClient::~SSLClient() freed the TLS context before shutting down
the SSL session. mbedTLS sessions hold a raw pointer into the
context's mbedtls_ssl_config, so a live keep-alive session's
close_notify would read freed memory. Shut down the session first,
then free the context.
Add a regression test that destructs an SSLClient while a keep-alive
mbedTLS session is still open.
The quick preview used a nonexistent httplib::ws::Message type with
.is_text()/.data. The actual API, as shown in README-websocket.md,
uses a plain std::string with ws.read(msg).
Trailer field names and values written by write_content_chunked()'s
done_with_trailer lambda were never validated, unlike every other
header output path (set_header, WebSocket handshake, client request
headers). An application reflecting untrusted input into a trailer
via DataSink::done_with_trailer() could inject CR/LF sequences and
achieve HTTP response splitting.
Skip trailer fields with invalid names or values, matching the
silent-skip behavior of set_header().
Cookbook body links referenced sibling pages with a bare slug
(e.g. `c14-keep-alive`). Under the pretty-URL layout each page lives
in its own directory, so these resolve against the page's own
directory and 404. Prefix them with `../` to match the convention
already used in the tour and llm-app sections.
Verified clean with `docs-gen check`.
std::isalnum and std::isdigit consult the global C locale, so a byte
like 0xC5 can classify as alphanumeric once an embedder calls
setlocale() (observed on macOS). HTTP grammars are defined over ASCII,
so raw bytes must be classified without regard to the locale.
Add detail::is_ascii_digit/is_ascii_alpha/is_ascii_alnum and use them
at every classification site: multipart boundary validation, token
checks, URI encoding, range header parsing, is_numeric, is_hex, and
IPv4 host detection. Also unify the hand-written digit range checks in
from_chars, URL parsing, and parse_ipv4 onto the same helpers.
With these in place nothing uses <cctype> anymore, so the include is
dropped.
The WebSocket upgrade request always appended ":port" to the Host
header, violating RFC 6455 Section 4.1 which says the port should be
included only when it is not the default (80 for ws, 443 for wss).
Some CDNs alter routing when the Host header carries an explicit
default port.
Build the Host header with detail::make_host_and_port_string, which
also brackets IPv6 literal hosts correctly.
Same header-injection vector as the name/filename fix: item.content_type
was concatenated into the part's Content-Type header unescaped, so
embedded CR/LF could inject arbitrary part headers.
Escape CR -> %0D and LF -> %0A via escape_multipart_field with a new
escape_quote = false mode. '"' is left intact since it is legal in
Content-Type values (e.g. quoted charset parameters) and appears outside
a quoted-string context here.
yhirose