ded synced new reference v0.53.0 to ded/cpp-httplib from mirror
3 days ago
ded synced commits to master at ded/cpp-httplib from mirror
f00e476f1b Release v0.53.0
- 8e702d3837 Gracefully drain socket before close in Server::process_and_close_socket (#2534)
* 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.
- View comparison for these 2 commits »
3 days ago
ded synced commits to fix-graceful-socket-close-2533 at ded/cpp-httplib from mirror
- f978bb5ca1 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.
- 9e855772db 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.
- 19333f80d4 Fix Mbed TLS/wolfSSL hostname verification bugs in set_sni()
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.
- 86d0210391 Add WebSocketClient::enable_server_hostname_verification
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.
- 6018c7feb3 Return ws::Result from WebSocketClient::connect() instead of bool
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.
- View comparison for these 10 commits »
3 days ago
ded synced new reference fix-graceful-socket-close-2533 to ded/cpp-httplib from mirror
3 days ago
ded synced commits to master at ded/cpp-httplib from mirror
- 19333f80d4 Fix Mbed TLS/wolfSSL hostname verification bugs in set_sni()
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.
- 86d0210391 Add WebSocketClient::enable_server_hostname_verification
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.
- View comparison for these 2 commits »
4 days ago
ded synced commits to master at ded/cpp-httplib from mirror
- 6018c7feb3 Return ws::Result from WebSocketClient::connect() instead of bool
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.
- 8d5085df1b Update README
- 8f0ff32056 Add WebSocket TLS and timeout recipes to the Cookbook
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.
- 2dd44d0f52 Document WebSocketClient/SSLClient TLS parity gaps in the READMEs
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.
- 86abc9a0ea Give WebSocketClient the PemMemory client certificate constructor SSLClient has
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.
- View comparison for these 10 commits »
5 days ago
ded synced commits to master at ded/cpp-httplib from mirror
- b2b1d56d6d fix: server example typos (#2532)
5 days ago
ded synced commits to master at ded/cpp-httplib from mirror
- 2b8658fa99 match mount points on a segment boundary in handle_file_request (#2529)
1 week ago
ded synced new reference v0.52.0 to ded/cpp-httplib from mirror
1 week ago
ded synced commits to master at ded/cpp-httplib from mirror
- 095a5c1caf Release v0.52.0
- 6e8a7dcd3f Bind the ordering tests to an ephemeral port (#2528)
test.cc says right above the PORT constant that it is only for the
legacy fixtures and that new standalone tests must use
bind_to_any_port() instead. The six tests added with the insertion-order
work all took the shared PORT anyway, which made them one more thing
contending for it.
Move them to bind_to_any_port() plus listen_after_bind(), the pattern
the note asks for and the rest of the standalone tests already use.
HeadersOrderTest.ReceivedFieldsKeepTheirOrder sends a raw request rather
than going through Client, so send_request() gains an optional port that
defaults to PORT and leaves its other 36 callers alone.
Checked by holding PORT open from another process while running the six:
they pass, where before the change listen(HOST, PORT) would have failed.
- 148d61a6a3 Give the raw listener in wait_writable_INET the socket options its neighbours have (#2527)
SocketStream.wait_writable_INET binds PORT + 1 directly, and it was the
only raw listener in the file that did not set SO_REUSEPORT/SO_REUSEADDR
first. PORT + 1 is shared with three SSL redirect tests and with
VulnerabilityTest.CRLFInjectionInHeaders, so once any of those has run,
the TIME_WAIT entries they leave make bind() fail here.
That failure did not surface where it happened. The bind runs on a
worker thread, where a failed ASSERT_EQ only returns from the lambda, so
the test carried on and failed later at ASSERT_NE(disconnected_svr_sock,
-1) with nothing pointing at the port.
Within one run of the suite the test comes before everything that uses
PORT + 1, so a clean run passes; it failed when a previous process had
left TIME_WAIT behind, which made it look intermittent. Running any of
those four tests first and then this one reproduces it every time: 4 of
4 before this change, 4 of 4 passing after, with up to eight TIME_WAIT
entries on the port.
Call default_socket_options(), which is what the library does for its
own listeners and what the other raw listeners in this file already do.
- c48ed1ed9a Share the query pair splitting between its two callers (#2526)
parse_query_text() and normalize_query_string() both walk a query string
and both open with the same eight lines to cut one "key=value" span at
its first '='. Give that a name and call it from both.
divide() puts everything before the first delimiter in the left half and
the rest in the right, so a span with no '=' lands entirely in key and
leaves val empty. Both callers rely on that: parse_query_text() records
a bare "flag" with an empty value, and normalize_query_string() emits it
back without an '='. The helper's comment says so, since that is the
part of divide()'s behaviour a reader has to know to follow either
caller.
- 8d428361fb Count entries with count() rather than equal_range plus distance (#2525)
Seven accessors each spelled the same two lines:
auto r = x.equal_range(key);
return static_cast<size_t>(std::distance(r.first, r.second));
The containers behind them all became detail::insertion_ordered_multimap
along the way, so count() is available and says what these functions
mean. The work is the same either way: equal_range() scans to the first
match and distance() then walks the restricted iterator to the end,
comparing keys at each step, which comes to one pass over the entries,
and count() is one pass too. This is a readability change, not a faster
one.
Covers get_header_value_count, Request and Response
get_trailer_value_count, Request::get_param_value_count,
MultipartFormData get_field_count and get_file_count, and
Result::get_request_header_value_count. The distance() call in
get_param_values() stays, since it sizes a reserve() and needs the range
anyway.
- View comparison for these 6 commits »
1 week ago
ded
synced and deleted reference skip-redundant-readable-poll at ded/cpp-httplib from mirror
1 week ago
ded
synced and deleted reference formdata-insertion-order at ded/cpp-httplib from mirror
1 week ago
ded
synced and deleted reference copilot/research-headers-type-change at ded/cpp-httplib from mirror
1 week ago
ded synced commits to master at ded/cpp-httplib from mirror
- 7963c382d6 Preserve the order of query parameters (#2523)
Params was a std::multimap, which sorts by parameter name. Parsing a
query string therefore threw away the order it arrived in, and building
one back out of Params handed the caller an alphabetised query rather
than the one they wrote. ClientImpl::send() takes that path whenever a
request carries Params without a query already in its path, so a caller
signing its query string could not reproduce the order it asked for.
normalize_query_string() exists in part to work around exactly this.
Generalize the container #2520 introduced for Headers into
detail::insertion_ordered_multimap<Mapped, KeyEqual> and alias both
types to it. Params passes std::equal_to, so parameter names stay
case-sensitive, while Headers keeps matching field names
case-insensitively. The name says insertion_ordered because in STL
vocabulary std::multimap is the ordered one, which is the reading this
change exists to correct.
This is a correctness change, not a performance one. Measured on the
real paths against the previous implementation, parse_query_text() over
eight parameters goes 931.5ns -> 935.1ns and params_to_query_str()
374.7ns -> 377.2ns; both are inside the run-to-run noise. The container
is a small share of that work, most of which is the string building in
decode_query_component().
Params picks up the same API changes Headers took in #2520: iterators
follow std::vector rules, value_type is std::pair<std::string,
std::string>, and insert(hint, value) is gone. Headers itself becomes an
alias rather than a class, so the names it appears in mangle differently
again; #2520 has not shipped in a release yet, so this costs nothing on
top of the break already there.
1 week ago
ded synced commits to formdata-insertion-order at ded/cpp-httplib from mirror
- 446afad5e9 Preserve the order of multipart form parts
FormFields and FormFiles were std::multimaps, which sort by field name.
RFC 7578 5.2 says a form processor "SHOULD send back results in order"
and that "Intermediaries MUST NOT reorder the results", so a handler
walking req.form.fields saw the parts alphabetised rather than as they
were sent, and a body received for forwarding could not be reproduced.
Point both at the insertion-ordered container #2523 generalized, with
std::equal_to since field names are case-sensitive. Entries sharing a
name already kept their relative order under std::multimap; what is
recovered here is the order across different names.
Server::read_content() keeps a FormFields::iterator alive across the
content callbacks that fill the part it points at, which is the one
thing this container could have broken: it is vector-backed, so a later
emplace can reallocate and leave an older iterator dangling. The code is
safe because the iterator is reassigned by the same emplace that could
reallocate, and is only read while the flag set alongside it says so.
ContentSurvivesContainerGrowth pins that down with 64 parts, enough to
grow the vector through seven reallocations; reverting the reassignment
makes it abort under ASan rather than fail quietly.
Growth also never copies a part's payload: the parser inserts the entry
with an empty content and appends the body bytes afterwards, and both
mapped types are nothrow-move-constructible, so a reallocation steals
the string buffers rather than deep-copying them.
- 7963c382d6 Preserve the order of query parameters (#2523)
Params was a std::multimap, which sorts by parameter name. Parsing a
query string therefore threw away the order it arrived in, and building
one back out of Params handed the caller an alphabetised query rather
than the one they wrote. ClientImpl::send() takes that path whenever a
request carries Params without a query already in its path, so a caller
signing its query string could not reproduce the order it asked for.
normalize_query_string() exists in part to work around exactly this.
Generalize the container #2520 introduced for Headers into
detail::insertion_ordered_multimap<Mapped, KeyEqual> and alias both
types to it. Params passes std::equal_to, so parameter names stay
case-sensitive, while Headers keeps matching field names
case-insensitively. The name says insertion_ordered because in STL
vocabulary std::multimap is the ordered one, which is the reading this
change exists to correct.
This is a correctness change, not a performance one. Measured on the
real paths against the previous implementation, parse_query_text() over
eight parameters goes 931.5ns -> 935.1ns and params_to_query_str()
374.7ns -> 377.2ns; both are inside the run-to-run noise. The container
is a small share of that work, most of which is the string building in
decode_query_component().
Params picks up the same API changes Headers took in #2520: iterators
follow std::vector rules, value_type is std::pair<std::string,
std::string>, and insert(hint, value) is gone. Headers itself becomes an
alias rather than a class, so the names it appears in mangle differently
again; #2520 has not shipped in a release yet, so this costs nothing on
top of the break already there.
- 486c81b275 Determine the final transfer coding across Transfer-Encoding lines (#2522)
RFC 9110 5.3 lets a coding list be split across several Transfer-Encoding
lines, which combine, in the order the lines were received, into one
comma-separated list. RFC 9112 6.1 then frames the message as chunked
only when chunked is the final coding of that combined list.
is_chunked_transfer_encoding() could not apply that rule while Headers
was an unordered_multimap, since the order of the lines was not
recoverable, so it fell back to reporting any message naming chunked on
any line as chunked. Headers now preserves the order the lines were
received in, so read the final coding directly: the last token of the
last line.
The fallback erred toward reporting chunked because mis-reading a
chunked message as unframed leaves its body in the socket, where a
keep-alive connection parses it as a smuggled request. That direction is
no longer needed. A request whose combined list ends in something other
than chunked is now reported as not chunked, and process_request()
answers 400 and closes rather than letting it reach the "no body" path,
which is what it already did for the single-line "chunked, gzip" form.
So `Transfer-Encoding: chunked` followed by `Transfer-Encoding: gzip` is
rejected instead of being read as chunked, and `gzip` followed by
`chunked` is still accepted.
A trailing line carrying no coding at all now leaves the combined list
ending in nothing rather than inheriting the coding from the line
before it.
- be28cf9435 Run CIFuzz only for pull requests that touch the fuzzed code (#2521)
The fuzzers build httplib.h and the targets under test/fuzzing, so a pull
request that touches neither has nothing for CIFuzz to exercise. Fuzzing
is by far the longest job in CI: 600 seconds of fuzzing on top of
building the OSS-Fuzz image, which came to 12m20s on a recent run while
every other job finished within 5m7s.
Filter the trigger by path rather than shortening fuzz-seconds. OSS-Fuzz
recommends 600 seconds as a minimum, and the budget is divided among all
of the project's fuzz targets, so with five targets a shorter run would
leave each one well under two minutes. Skipping the job for
documentation-only changes cuts the wait without giving up any fuzzing
on the pull requests that do reach the parsers.
- d860c842ea Preserve the order of header fields with the same name (#2520)
RFC 9110 5.3 makes the order of header fields sharing a field name
significant, but Headers was a std::unordered_multimap, which gives no
ordering guarantee for equivalent keys. libstdc++ hands duplicates back
in reverse insertion order while libc++ uses insertion order, so
get_header_value() returned a different field depending on the platform,
and code that picks a value out of an accidentally or maliciously
duplicated field name had no way to say which one it wanted.
Replace Headers with a small container that keeps the fields in the
order they were received or set. Storage is a flat vector and lookup is
a linear scan, which beats hashing for the at most
CPPHTTPLIB_HEADER_MAX_COUNT fields a message carries. begin()/end() walk
every field, while find() and equal_range() hand back the same iterator
type restricted to one field name; equality compares only the position,
so a restricted iterator still compares equal to end(). Erasing an
equal_range() therefore removes only the fields with that name and
leaves interleaved fields alone.
std::multimap was the smaller change but sorts by field name, which
would stop control data such as Host from leading the message. Instead
Host is now prepended via emplace_front() so it keeps its place at the
front of a request.
Two side effects worth noting: incrementing past the last field of a
name now saturates at end(), so an out-of-range id passed to
get_header_value() returns the default instead of running off the
container as it did before; and iterators follow std::vector rules, so
they are invalidated by insertion.
Fixes #2509
- View comparison for these 10 commits »
1 week ago
ded synced new reference formdata-insertion-order to ded/cpp-httplib from mirror
1 week ago
ded synced commits to master at ded/cpp-httplib from mirror
- 486c81b275 Determine the final transfer coding across Transfer-Encoding lines (#2522)
RFC 9110 5.3 lets a coding list be split across several Transfer-Encoding
lines, which combine, in the order the lines were received, into one
comma-separated list. RFC 9112 6.1 then frames the message as chunked
only when chunked is the final coding of that combined list.
is_chunked_transfer_encoding() could not apply that rule while Headers
was an unordered_multimap, since the order of the lines was not
recoverable, so it fell back to reporting any message naming chunked on
any line as chunked. Headers now preserves the order the lines were
received in, so read the final coding directly: the last token of the
last line.
The fallback erred toward reporting chunked because mis-reading a
chunked message as unframed leaves its body in the socket, where a
keep-alive connection parses it as a smuggled request. That direction is
no longer needed. A request whose combined list ends in something other
than chunked is now reported as not chunked, and process_request()
answers 400 and closes rather than letting it reach the "no body" path,
which is what it already did for the single-line "chunked, gzip" form.
So `Transfer-Encoding: chunked` followed by `Transfer-Encoding: gzip` is
rejected instead of being read as chunked, and `gzip` followed by
`chunked` is still accepted.
A trailing line carrying no coding at all now leaves the combined list
ending in nothing rather than inheriting the coding from the line
before it.
- be28cf9435 Run CIFuzz only for pull requests that touch the fuzzed code (#2521)
The fuzzers build httplib.h and the targets under test/fuzzing, so a pull
request that touches neither has nothing for CIFuzz to exercise. Fuzzing
is by far the longest job in CI: 600 seconds of fuzzing on top of
building the OSS-Fuzz image, which came to 12m20s on a recent run while
every other job finished within 5m7s.
Filter the trigger by path rather than shortening fuzz-seconds. OSS-Fuzz
recommends 600 seconds as a minimum, and the budget is divided among all
of the project's fuzz targets, so with five targets a shorter run would
leave each one well under two minutes. Skipping the job for
documentation-only changes cuts the wait without giving up any fuzzing
on the pull requests that do reach the parsers.
- d860c842ea Preserve the order of header fields with the same name (#2520)
RFC 9110 5.3 makes the order of header fields sharing a field name
significant, but Headers was a std::unordered_multimap, which gives no
ordering guarantee for equivalent keys. libstdc++ hands duplicates back
in reverse insertion order while libc++ uses insertion order, so
get_header_value() returned a different field depending on the platform,
and code that picks a value out of an accidentally or maliciously
duplicated field name had no way to say which one it wanted.
Replace Headers with a small container that keeps the fields in the
order they were received or set. Storage is a flat vector and lookup is
a linear scan, which beats hashing for the at most
CPPHTTPLIB_HEADER_MAX_COUNT fields a message carries. begin()/end() walk
every field, while find() and equal_range() hand back the same iterator
type restricted to one field name; equality compares only the position,
so a restricted iterator still compares equal to end(). Erasing an
equal_range() therefore removes only the fields with that name and
leaves interleaved fields alone.
std::multimap was the smaller change but sorts by field name, which
would stop control data such as Host from leading the message. Instead
Host is now prepended via emplace_front() so it keeps its place at the
front of a request.
Two side effects worth noting: incrementing past the last field of a
name now saturates at end(), so an out-of-range id passed to
get_header_value() returns the default instead of running off the
container as it did before; and iterators follow std::vector rules, so
they are invalidated by insertion.
Fixes #2509
- 8e08c22783 Cut a syscall and the byte-at-a-time line reader out of the request path (#2513)
* Skip the redundant readability poll on the first read of a request
process_server_socket_core() calls keep_alive(), which polls the socket
and only invokes the callback once it reports readable. The stream is
then constructed and its first read polls the very same socket again
before calling recv, asking the kernel a question that was answered
microseconds earlier.
Profiling a request shows how little else is going on: of the samples
taken while the worker was off the keep-alive wait, 85% sat in syscalls
(recv, send, and the two polls) and only 15% in header parsing, routing
and response serialization. Removing one of five syscalls per request is
worth more than anything reachable inside that 15%.
Let the caller hand the stream what it already knows. The hint is
consumed by the first read, so body reads and every later request keep
polling as before, and set_read_timeout() during a WebSocket upgrade is
unaffected. Nothing is skipped when a read finds the buffer empty on its
own. The accepted socket also carries SO_RCVTIMEO from listen_internal(),
so even a wrong hint could not block forever.
Measured with wrk -t2 -c8 over three interleaved runs, server CPU per
request drops from 33.4/32.5/29.7us to 23.6/22.9/23.9us, and throughput
rises 10-15%. The TLS path gets the same treatment - it shares
process_server_socket_core(), and SSLSocketStream::read() polls after
tls::pending() comes up empty - for a smaller 44.9->40.2us, crypto being
the larger cost there.
Removing the second poll, in write(), looks worth another 15% but is left
alone: it would rely on SO_SNDTIMEO being set on every socket a
SocketStream is built over, and the TLS write path retries on WantWrite
in a way that turns a send timeout into a much longer stall.
* Scan buffered bytes for line ends instead of reading one at a time
stream_line_reader::getline() pulls the request line and every header
through strm_.read(&byte, 1). A 300-byte header block therefore costs
300 virtual calls, each with its own bounds check and one-byte copy.
None of them are syscalls - SocketStream has already pulled up to 4KB
off the socket - so this is pure CPU spent one character at a time.
On a Linux CI runner read_headers() takes 2.44us for seven headers, of
which 1.53us is this loop. That is 63% of header parsing, and header
parsing is a bigger share of the total on Linux than the profiling on
macOS suggested: 3.8us of HTTP processing against 16us of server CPU
per request, versus roughly 3us against 30us on macOS. Cheap syscalls
leave parsing a larger slice of what remains.
Let a stream offer what it has already buffered, and scan that for the
terminator in one pass. Streams that do no buffering of their own report
none and keep the existing byte loop, so Stream subclasses outside the
library are unaffected, as is the TLS path - SSLSocketStream has no
buffer of its own to expose.
A bare LF still does not end a line in the default configuration, so the
scan looks for CRLF and carries the CR across a chunk boundary. Both
append overloads now share one path: the per-character one used to
decide where to write from fixed_buffer_used_size_ alone, which after a
bulk append had spilled into the growable buffer would send the byte to
a fixed buffer that ptr() and size() no longer read. That dropped a byte
from over-long request lines - caught by ServerTest.TooLongRequest and
AlmostTooLongRequest.
- f51df1473b Merge branch 'metsw24-max-mmap-map-failed-guard'
- View comparison for these 14 commits »
1 week ago
ded synced commits to master at ded/cpp-httplib from mirror
- ae8356d86e Clean up addr_map hostname support
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.
- 49b921b52d Allow addr_map_ values to be hostnames, not just IP literals (#2515)
addr_map_ only accepted IP literals as mapped values; a non-IP value was
passed as the `ip` argument and rejected by getaddrinfo's AI_NUMERICHOST
path. The lookup in ClientImpl::create_client_socket and
WebSocketClient::connect now checks the mapped value with
detail::is_ip_address: IP literals keep the existing AI_NUMERICHOST path,
while hostnames are passed as the connect host so they get resolved.
host_ is never touched, so it keeps supplying the Host header and SNI in
both cases.
is_ip_address() moved into the non-SSL detail block so that both client
code paths can use it without CPPHTTPLIB_SSL_ENABLED.
This also fixes the documented Unix domain socket client example, where
the mapped value is a socket path: it previously took the AI_NUMERICHOST
path and never reached the AF_UNIX branch in create_socket.
- 447b9c4a29 Buffer the WebSocket handshake before writing it to the socket
Follow-up to #2514. The rebuilt handshake wrote the request line straight
to the socket, so a header rejected by check_and_write_headers left a
truncated "GET /ws HTTP/1.1" sitting in the peer's buffer before the
connection was torn down, and every header cost its own small write.
Build the request into a BufferStream and flush it in one go, matching
ClientImpl::write_request. The new test drives a raw listener and asserts
the peer sees a clean EOF with zero bytes; without this change it observes
18.
Also fold WebSocketTest.HostHeaderInHandshake into
WebSocketTest.DefaultHeadersInHandshake, which covers the same Host
assertion through the capture helper #2514 introduced.
- f406808497 Merge pull request #2514 from Hyukya/master
websocket: rebuild handshake on Request (support Host override, align with HTTP client behavior)
- 2c28d2fa3e websocket: rebuild handshake on Request/header pipeline
Replace the hand-built upgrade request string in
perform_websocket_handshake with the same Request,
write_request_line, and check_and_write_headers path used by
ClientImpl::open_stream. Add WebSocketClient::prepare_default_headers
to inject Host and User-Agent defaults; protocol-mandatory headers
(Upgrade, Connection, Sec-WebSocket-Key/Version) are always
overwritten.
- View comparison for these 5 commits »
1 week ago