* Drop the claim that small bodies skip compression
There is no size threshold anywhere in the compression path.
encoding_type() gates on the content type and Accept-Encoding only, and
apply_ranges() compresses whatever body it is given, so a two-byte
text/plain response comes back gzipped at 22 bytes.
Say what actually happens and leave the decision to the handler.
* Compress static file responses behind an opt-in (Fix#2545)
apply_ranges() runs the compressor inside the branch it takes when
res.body is non-empty. A response served from a file leaves res.body
empty and sets content_length_, so it took the other branch, which
writes Content-Length and returns; encoding_type() was computed before
the split and never consulted on that side. The same bytes handed to
set_content() came back gzipped, which left set_mount_point() and
Response::set_file_content() as the one path that missed out.
Add Server::set_static_file_compression(), off by default so nothing
about an existing server changes. When it is on, the file-backed
provider is run through the compressor into res.body ahead of the rest
of apply_ranges(), so the response is framed the way set_content()
already frames one: it keeps its Content-Length, and HEAD still reports
the size a GET would return.
Ranges are answered from the identity representation, since RFC 9110
applies Range after content coding and slicing a compressed body would
mean compressing the whole file first. The ETag carries the coding it
belongs to, so a client that cached the compressed form revalidates
against its own validator rather than the identity one. Both the ETag
and the body take their coding from static_file_encoding(), so the two
cannot disagree.
Providers registered with set_content_provider() are left alone. zlib
buffers until its window fills, so running one through a compressor
would hold back writes that a caller expects to reach the peer as they
are produced.
The compressed bytes stay in memory until the response has been
written, so the peak cost scales with requests in flight.
set_static_file_compression_max_length() bounds it, defaulting to 4MB.
* Add a minimum size for static file compression
Compressing a file that already fits in a single 1500-byte MTU does not
get it to the client any sooner, and a file of a few bytes comes back
larger than it went in once gzip's header and trailer are added. Every
other server draws this line: nginx's gzip_min_length, Caddy's
minimum_length, IIS's minFileSizeForComp, CloudFront's 1000-byte floor.
The note this replaces told callers to decide in the handler. A response
served through set_mount_point() has no handler to decide in, so the
floor has to live in the server. It defaults to 1400 bytes, the size
that fits inside one MTU with room for headers.
set_static_file_compression_min_length() moves it, and
CPPHTTPLIB_STATIC_FILE_COMPRESSION_MIN_LENGTH sets the default at
compile time. The empty-file case keeps its own early-out so that a zero
floor still cannot turn an empty body into a 20-byte gzip stream.
The two bounds now read as a pair, so the documentation says what each
one is for: the lower bound is about what is worth compressing, the
upper bound about what one request is allowed to cost.
Every file under test/www except 1MB.txt is below the default floor, so
the tests that need a small file compressed lower it explicitly.
parse_disposition_params() and extract_media_type() both split on every
';' and then on every '=', with no idea that a parameter value can be a
quoted-string. RFC 9110 5.6.6 allows ';' and '=' inside one, so
filename="report=v2.pdf" came out as v2.pdf", and filename="a;b.txt" was
truncated at the semicolon and left a bogus parameter behind.
The same defect reached the boundary. RFC 2046 5.1.1 allows '=' in a
boundary, which forces a sender to quote it, so the common MIME form
boundary="----=_NextPart_000_0000_01D9" parsed as
_NextPart_000_0000_01D9".
Add split_unquoted(), which is split() with the one extra rule that a
delimiter inside a quoted-string is not a delimiter, and route both
parameter parsers through it. The key/value split, duplicated verbatim
in the two of them, moves into divide_param_pair(). That one divides at
the first '=' without tracking quotes: 5.6.6 makes the key a token, so
no quote can precede the separator, and reusing divide() keeps this off
the per-byte scan.
A backslash stays an ordinary character here. Both browsers and
httplib's own sender percent-encode '"' rather than escaping it, and
recognizing a quoted-pair without also unescaping it would just trade
one wrong value for another.
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.
make_file_body() measures the file once and that length is already the
response's Content-Length. The provider re-opens the file by path on each
call, so if the file has been truncated since, the read comes up empty and
the provider returned true without writing. write_content_with_progress()
advances its offset only by what was written, so it called the provider
again, got nothing again, and kept spinning until the peer gave up.
Return false instead, as every other failure in this provider does.
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.
Server::process_request() wraps only routing() in a try/catch.
Everything else the user supplies runs outside it:
- the content provider, from write_response_core()
- post_routing_handler_, error_handler_, logger_
- expect_100_continue_handler_
- a WebSocket handler, and pre_routing_handler_ on the upgrade path
An exception from any of those unwinds out of process_and_close_socket()
into the task queue, which calls the job without a catch, so it reaches
the top of a pool thread and terminates the process. One handler that
throws takes down every other connection the server is holding.
Add Server::serve_guarded() and run the serving loop through it in both
process_and_close_socket() overloads. The exception is not turned into a
500: by the time a content provider runs, the status line and headers
are already on the wire, so there is nothing left to replace. Report it
through the error logger as Error::UserCallbackException and drop the
connection, which is what the peer observes regardless. Requests on
other connections are unaffected, and the socket is still drained and
closed - which unwinding used to skip on the non-SSL path, since
drain_and_close_socket() sits after the call rather than in a scope
guard.
The error logger is a user callback too, so the report inside the guard
is itself wrapped: a throwing logger must not be able to open the guard
back up.
Adds ServerExceptionTest: a throwing content provider, post-routing
handler, WebSocket handler and error logger, plus the content provider
case against SSLServer, each checking that a later request on a new
connection still succeeds. Every test runs the server on a single worker
thread, so a guard that catches the exception but still loses the thread
shows up as the follow-up request never being served. Note that all of
them abort the test binary without this change - which is the bug, but
it means a regression here fails the run rather than one test.
write_content_chunked()'s sink treated "the provider wrote nothing" as
"the provider has finished":
data_available = l > 0;
so sink.write(p, 0) ended the loop. Only done()/done_with_trailer()
emit the terminating zero-length chunk, so the body was left
unterminated - and the function still returned Success, because the
post-loop check only reports the is_shutting_down() case. The peer waits
for a last chunk that never arrives, and on a keep-alive connection
anything written next is parsed as a chunk-size line.
A provider reaching a pass with nothing to hand over is ordinary:
popping an empty buffer off a queue, or a compressor that has consumed
its input without producing output yet. It is not the end of the
message.
Ignore zero-length writes instead. A zero-length chunk is the terminator
in chunked coding, so it must never be emitted mid-body either way, and
data_available is now controlled only by done()/done_with_trailer().
This matches write_content_without_length(), where the sink's write
never ends the body.
The old behaviour cannot have been relied on: it produced an
unterminated response, so a provider using it never worked in the first
place.
DataSink has four callbacks, but only write is assigned by every writer
that hands a sink to a content provider:
write_content_with_progress() write, is_writable
write_content_without_length() write, is_writable, done
write_content_chunked() all four
send_with_content_provider...() write
get_multipart_content_provider() write, done (cur_sink)
A provider that calls one of the unassigned ones invokes an empty
std::function and throws std::bad_function_call. Nothing on that path
catches it, so it unwinds out of the thread running the provider and
terminates the process. The README's own idiom is enough to hit it:
sink.done() is documented for the without-length overload, but a
provider registered through set_content_provider() with a length gets a
sink where done is empty.
Default the three optional callbacks instead. A sink is writable unless
a writer says otherwise, and a sink that cannot carry trailers still has
to finish, so done_with_trailer() falls back to done(). Capturing this
for that is safe because DataSink is neither copyable nor movable.
A no-op done() alone would only trade the crash for a hang on the two
length-framed paths: both loop until offset reaches the promised length,
so a provider that reports itself done without writing would be called
again immediately, forever. Both now record that the provider finished
and stop, and the short body is reported as a write error. The client
path gains that check for the compressor-failure exit as well, which
used to send a truncated request body without reporting anything.
cur_sink in get_multipart_content_provider() now forwards is_writable
from the outer sink, so a provider item asking whether it may keep going
gets the stream's answer rather than the default.
The accept loop in Server::listen_internal() classified accept() failures
by reading errno, but Winsock reports them through WSAGetLastError() and
never touches the CRT errno. Both retry branches were therefore dead code
on Windows, and every accept() failure fell through to the fatal path,
which closes the listening socket and ends listen().
That is reachable in normal operation: a peer resetting a pending
connection before it is accepted is enough, and descriptor or buffer
exhaustion shows up under load. One such event stopped the server from
accepting anything again.
Add is_accept_resource_error() and is_accept_transient_error() next to
is_connection_error(), which already abstracts the same errno vs
WSAGetLastError() difference, and use them in the accept loop.
The POSIX sets are widened to match the Windows ones rather than being
left as they were: ECONNABORTED is the POSIX spelling of the aborted
pending connection that motivates this, and ENFILE, ENOBUFS and ENOMEM
are resource exhaustion in the same sense as EMFILE.
When accept() failed for a reason the retry branches do not cover, the
loop closed svr_sock_ but left the descriptor in the atomic. Two things
go wrong from there:
- A later stop() reads the stale value and calls shutdown()/close() on
it. By then the OS may have reused the descriptor for an unrelated
socket (a worker's keep-alive connection, or one the application
opened), and that connection is torn down instead.
- keep_alive() in the worker threads watches svr_sock_ to notice that
the server is going away, so the workers keep waiting on a listening
socket that no longer exists.
Take the descriptor with exchange(INVALID_SOCKET) before closing it,
which is what stop() already does. That also settles the race with a
concurrent stop(): whichever side takes the descriptor closes it exactly
once, and the other sees INVALID_SOCKET and does nothing.
parse_multipart_boundary only rejected an empty boundary, so a request could
declare one as long as a header line is allowed to be. A stock server accepts
up to 8146 bytes there, which is what CPPHTTPLIB_HEADER_MAX_LENGTH leaves after
"Content-Type: multipart/form-data; boundary=".
FormDataParser searches the body for "--" + boundary + CRLF with a plain
substring scan. buf_find scans for that delimiter's first byte, always '-', and
at every position that matches calls start_with, which compares until the first
mismatch. A body of '-' makes every position a candidate, and a boundary of '-'
makes each candidate compare the whole delimiter before failing at the CRLF. The
worst case is the product of the body length and the boundary length, and only
the first factor was bounded.
Measured by driving the parser directly in 16 KB reads, Apple clang 17 at
-O2 -DNDEBUG, best of three runs on an otherwise idle machine. 100 MB of '-',
the default payload limit, costs 2.59 s of CPU with a 70 byte boundary and
281.83 s with an 8147 byte one, a factor of 109. The same shape shows at 8 MB:
0.211 s, 3.081 s, 11.359 s and 22.091 s for boundaries of 70, 1024, 4096 and
8147 bytes.
RFC 2046 5.1.1 caps a boundary at 70 characters, so honoring that limit bounds
the multiplier too. The limit applies to the value after unquoting, so a quoted
70 character boundary stays valid. Only the server receive path parses a
boundary out of a Content-Type, so what clients may send is unaffected, and the
boundaries the library generates itself are 45 characters.
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.
* Bound the multipart parser's buffer while it waits for a boundary
FormDataParser accumulated the entire request body whenever the declared
boundary never appeared in it. State 0 returned without erasing anything, so
the buffer grew to the full payload (100 MB by default) and buf_find rescanned
all of it on every 16 KB read. The cost grew with the square of the body size:
50 MB of '-' took 198 s of CPU on one core, and the buffer pinned the body in
memory for the whole request. One unauthenticated request was enough, and the
parser runs for any multipart request even when the handler never looks at the
parsed result.
State 0 now keeps only the last dash_boundary_crlf_.size() - 1 bytes while it
waits, which bounds both the memory and the rescan without capping how long a
preamble may be. The same 50 MB body now takes 0.14 s and the buffer stays at
one read plus the boundary. A boundary split across reads still parses, which
is what de5a255 (#2159) gave up this erase for.
State 4 buffered without bound in the same way when a boundary was followed by
neither CRLF nor "--". No further data can make such a body valid, so it now
fails right away. That is only safe because the close-delimiter branch moves to
a new state 5 that discards the epilogue: it used to stay in state 4, so an
epilogue arriving in a later read fell into this same branch. An epilogue
beginning with CRLF was then parsed as a new part and the request was rejected
with 400, which state 5 fixes as well.
Affected since v0.23.0, where de5a255 replaced the erase that had kept the
buffer in check.
* Skip buffering the multipart epilogue
Once the close delimiter has been parsed the parser is in state 5 and discards
whatever follows, but it still copied each epilogue read into the buffer before
erasing it. Return before buffering so a large epilogue spread across several
reads is dropped without being copied in at all.
* Clean up the multipart parser tests and the state 4 branch
Review follow-ups on top of the previous two commits, no behavior change.
- Move the four new tests next to the rest of MultipartFormDataTest. They
had landed in the middle of the RedirectTest block.
- Use bind_to_any_port instead of the fixed PORT, as AGENTS.md requires for
newly added servers. NoInitialBoundaryParsingIsNotQuadratic holds its port
for a couple of seconds, which matters when the suite is run sharded.
- Send "Connection: close" from expect_split_multipart_ok. The server kept
the connection alive after answering, so the response drain idled until the
client read timeout; both tests drop from about 3s to about 0.11s.
- Drop the dead `dash_.size() > buf_size()` guard in state 4 and flatten the
nested else. The check above it already guarantees two buffered bytes, and
both CRLF and "--" are two bytes, so it can never fire. Removing it is what
makes the new comment's claim readable straight off the code.
* Rename the timing test's locals to avoid a Windows macro
MSVC's <rpcndr.h>, pulled in by <windows.h>, defines `small` as `char`, so
`auto small = ...` failed to compile on the Windows jobs. Same class of
problem as the std::min / std::max collision.
* Add Server::CustomRoute() for HTTP methods outside the built-in set
parse_request_line validates the request method against a fixed whitelist and
rejects anything else with 400 before routing runs. That blocks WebDAV, where
PROPFIND, PROPPATCH, MKCOL, COPY, MOVE, LOCK and UNLOCK are ordinary methods
defined by RFC 4918, and it blocks extension methods such as UPnP's SUBSCRIBE.
The need has been open since #847.
Registering a handler is now what makes the server accept a method:
svr.CustomRoute("PROPFIND", "/dav/:id", handler);
Because custom methods go through the normal dispatch path, patterns work the
way they do for Get() and friends, and the request body is available in
req.body. Serving these methods through set_pre_routing_handler was never
enough: the body has not been read at that point, so PROPPATCH and LOCK, which
require one, could not be implemented at all.
A HandlerWithContentReader overload is available too. The content reader gate
in routing() also fires when a custom method carries no body, matching what
expect_content() does unconditionally for POST/PUT/PATCH/DELETE, so a body-less
PROPFIND (RFC 4918 treats one as allprop) reaches its handler instead of
falling through to 404.
Method names are validated as RFC 9110 tokens, and the ten built-in methods are
refused. Seven of them are dispatched by the if/else chain in routing() before
the custom tables are consulted, so a route registered for one could never
fire; CONNECT, TRACE and PRI carry protocol-level meaning this library does not
route. A refused registration makes is_valid() return false, so listen() fails
rather than starting a server holding a handler that would never run. This is
also why SSLServer::is_valid() now chains to Server::is_valid() instead of only
checking ctx_.
Servers that never call CustomRoute() keep the previous per-request cost: the
built-in method set is checked first and short-circuits, and the custom lookup
returns early on an empty map.
* Add cookbook recipe for custom HTTP methods
The CustomRoute() docs were a section inside S01, which pushed that page to 90
lines, the longest in the cookbook, and mixed a separate feature into a page
about registering GET/POST/PUT/DELETE handlers. Move the section into its own
recipe and give it room for the part that was missing: the OPTIONS handler
returning DAV: and Allow, which WebDAV clients probe for before anything else.
S01 goes back to 68 lines and keeps a pointer to the new page.
The recipe is titled after the API rather than after WebDAV, and says outright
that generating the 207 Multi-Status XML, interpreting Depth and managing locks
are the reader's job. Routing the method is all the library does.
S23 takes order 42, so the TLS, SSE and WebSocket recipes shift to 43-57. That
only moves the sort key. Filenames, the T01/E01/W01 labels, the published URLs
and every cross-reference are untouched.
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.
A wss:// WebSocket enters a single TLS session from several threads: the
read path, the application's send()/close(), and the heartbeat ping thread.
The existing write_mutex_ only serializes writers, so a reader's SSL_read and
a writer's SSL_write (plus the SSL_peek in is_peer_closed() on the write path)
run concurrently on the same session. OpenSSL and the other backends forbid
concurrent access to one session, so this corrupts the record layer: messages
are silently dropped, and under ASan it shows up as a heap-buffer-overflow.
It affects wss:// only; plain ws:// is unaffected because the kernel allows
concurrent recv()/send() on a socket.
Route wss:// through a new WebSocketSSLStream that serializes every TLS call
with one per-stream mutex. The socket is kept non-blocking for the stream's
lifetime and each read()/write() performs a single non-blocking TLS call under
the lock, then waits for readiness with select() outside the lock. The lock is
therefore held only for CPU-bound work, so a reader blocked waiting for data
never stalls a concurrent sender.
Because the socket is non-blocking, a TLS call can stop needing either
direction, so read() also waits for writability on WantWrite and write() waits
for readability on WantRead. A read that shares its session with the send path
has to flush pending output before it can decrypt more input, and Mbed TLS
surfaces this on every mbedtls_ssl_read(). The read timeouts are atomic since
WebSocket::close() shortens them from the closing thread while the receive
thread is inside wait_readable().
SSLSocketStream is left untouched, so ordinary HTTP/HTTPS keeps its exact code
path and performance. The heartbeat ping thread also stays, so timer-driven
pings keep working as before.
Add test_websocket_thread_safety.cc, which drives send/close/heartbeat against
a concurrent reader over wss://. Built with ASan in CI, a regression surfaces
as a heap-buffer-overflow.
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.
* 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.
* Gracefully drain socket before close in Server::process_and_close_socket
Closing a connection while the receive queue still has unread data,
or while bytes are still in flight, can make the OS send an abortive
RST instead of a graceful FIN. On Windows this surfaces as
WSAECONNABORTED/WSAECONNRESET on the peer's read, which can make an
otherwise fully-written response look like a failed request -- a
likely contributor to the ServerTest.HTTP2Magic flakiness tracked in
#2533.
Add detail::close_socket_gracefully(), which half-closes the write
side, drains any queued/in-flight bytes (bounded to 100ms / 1MB),
then performs the final shutdown+close. Use it in
Server::process_and_close_socket.
Root cause and fix mechanism identified by @Hyukya in #2533.
* Rename close_socket_gracefully to drain_and_close_socket
'gracefully' already means something specific in this codebase: whether
to send a TLS close_notify before closing (shutdown_ssl's
shutdown_gracefully param, ClientImpl::disconnect(gracefully),
tls::shutdown(session, graceful)). Reusing the word for an unrelated
TCP-level drain-before-close made the new function read as part of that
TLS machinery when it isn't. Rename it to describe what it does instead,
matching the existing close_socket/shutdown_socket and
WebSocketClient::shutdown_and_close naming.
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.
