Improvements around HTTP Upgrade: and multiplex hanndling:
* add `Curl_conn_set_multiplex()` to set connection's multiplex
bit and trigger "connchanged" events
* call `Curl_conn_set_multiplex()` in filters' `CF_CTRL_CONN_INFO_UPDATE`
implementation where other connection properties are updated.
This prevents connection updates before the final filter chain
is chosen.
* rename enum `UPGR101_INIT` to `UPGR101_NONE`
* rename connection bit `asks_multiplex` to `upgrade_in_progress`
* trigger "connchanged" when `upgrade_in_progress` clears
* rename `WebSockets` to `WebSocket` as it is the common term
used in documentation
Closes#18227
Split out adding of individual request headers into a switch. Check
the connection http version only on fresh connections, use separate
methods.
Add TE: header directly without allocation. Add bit for indicating
Connection: header has been added and custom headers should not do
that again.
Closes#18444
`getsock()` calls operated on a global limit that could
not be configure beyond 16 sockets. This is no longer adequate
with the new happy eyeballing strategy.
Instead, do the following:
- make `struct easy_pollset` dynamic. Starting with
a minimal room for two sockets, the very common case,
allow it to grow on demand.
- replace all protocol handler getsock() calls with pollsets
and a CURLcode to return failures
- add CURLcode return for all connection filter `adjust_pollset()`
callbacks, since they too can now fail.
- use appropriately in multi.c and multi_ev.c
- fix unit2600 to trigger pollset growth
Closes#18164
Fix compile error when building with `--disable-verbose`.
Adjust pytest to skip when curl is not a debug build but needs
traces.
Follow-up to b453a447ceCloses#18053
- vtls: fix unused variable and symbols.
- ftp: fix unused variable.
- http: fix unused variables.
- smtp: fix unsued variable.
- wolfssl: fix unused variable with !proxy.
- libssh: fix unused argument.
- curl_trc: sync guards between declaration and definition.
- curl_trc: add missing guard for `Curl_trc_ssls` when !verbose.
- curl_trc: fix errors with !http + http3.
- curl_trc: fix missing function with !http + nghttp2.
- cf-h2-proxy: disable when !http + nghttp2, to avoid calling undeclared
functions.
- sha256: fix missing declaration in rare configs.
- md4: fix symbol conflict when building GnuTLS together with AWS-LC or
wolfSSL. By prioritizing the latter two. AWS-LC has no option
to disable the clashing symbol. wolfSSL does, but the most seamless is
to skip including GnuTLS's standalone `md4.h` to avoid the clash.
- build: fix errors with !http + nghttp2.
- build: catch !ssl + ssls-export combination in source. Convert
build-level errors to warnings.
- build: fix errors with !http + http3.
- build: fix building curl tool and unit1302 in rare combinations.
By always compiling base64 curlx functions.
- cmake: add `_CURL_SKIP_BUILD_CERTS` internal option.
To disable automatically building certs with the testdeps target.
To improve performance when testing builds.
(used locally to find the failing builds fixed in this PR.)
Closes#17962
- codespell: break logic out into its own runnable script. Allowing
to run it on local machines.
- codespell: install via `pip`, bump to latest version.
- codespell: show version number in CI log.
- codespell: drop no longer needed word exception: `msdos`.
- codespell: include all curl source tree, except `packages` and
`winbuild`. Drop an obsolete file exclusion.
- add new spellchecker job using the `typos` tool. It includes
the codespell dictionary and a couple more. Use linuxbrew to install
it. This takes 10 seconds, while installing via `cargo` from source
would take over a minute.
- codespell: introduce an inline ignore filter compatible with `cspell`
Make `typos` recognize it, too. Move single exceptions inline.
Fix new typos found. Also rename variables and words to keep
spellchecking exceptions at minumum. This involves touching some tests.
Also switch base64 strings to `%b64[]` to avoid false positives.
Ref: https://github.com/crate-ci/typos/blob/master/docs/reference.md
Ref: https://github.com/codespell-project/codespell?tab=readme-ov-file#inline-ignore
Ref: https://github.com/codespell-project/codespell/issues/1212#issuecomment-1721152455
Ref: https://cspell.org/docs/Configuration/document-settingsCloses#17905
Drop `strcasecompare` and `strncasecompare` in favor of libcurl API
calls `curl_strequal` and `curl_strnequal` respectively.
Also drop unnecessary `strcase.h` includes. Include `curl/curl.h`
instead where it wasn't included before.
Closes#17772
Change the signature of `bufq` functions from
* `ssize_t Curl_bufq_*(..., CURLcode *err)` to
* `CURLcode Curl_bufq_*(..., size_t *pn)`
This allows us to write slightly less code and avoids the ssize_t/size_t
conversions in many cases. Also, it gets the function in line with all
the other send/recv signatures.
Added helper functions in `cfilters.h` for sending from/receving into
a bufq.
Fuzzer now fails to build due to these changes and its testing of
the bufq API.
Closes#17396
Connection filters had a method `get_host()` which had not really been
documented. Since then, the cf had the `query()` method added. Replace
the separate get_host with query.
Add `CF_QUERY_HOST_PORT` as query to connection filters to retrieve
which remote hostname and port the filter (or its sub-filter) is talking
to. The query is implemented by HTTP and SOCKS filters, all others pass
it through.
Add `Curl_conn_get_current_host()` to retrieve the remote host and port
for a connection. During connect, this will return the host the
connection is talking to right now. Before/After connect, this will
return `conn->host.name`.
This is used by SASL authentication.
Closes#17419
Add a bitset `dirty` to the multi handle. The presence of a transfer int
he "dirty" set means: this transfer has something to do ASAP.
"dirty" is set by multiplexing protocols like HTTP/2 and 3 when
encountering response data for another transfer than the current one.
"dirty" is set by protocols that want to be called.
Implementation:
* just an additional `uint_bset` in the multi handle
* `Curl_multi_mark_dirty()` to add a transfer to the dirty set.
* `multi_runsingle()` clears the dirty bit of the transfer at
start. Without new dirty marks, this empties the set after
al dirty transfers have been run.
* `multi_timeout()` immediately gives the current time and
timeout_ms == 0 when dirty transfers are present.
* multi_event: marks all transfers tracked for a socket as dirty.
Then marks all expired transfers as dirty. Then it runs
all dirty transfers.
With this mechanism:
* Most uses of `EXPIRE_RUN_NOW` are replaced by `Curl_multi_mark_dirty()`
* `Curl_multi_mark_dirty()` is cheaper than querying if a transfer is
already dirty or set for timeout. There is no need to check, just do it.
* `data->state.select_bits` is eliminated. We need no longer to
simulate a poll event to make a transfer run.
Closes#17662
When a transfer is done prematurely, a RST is generated to the server,
but was not send right away due to send buffering.
Flush the send buffer in a best effort when transfer is done.
Reported-by: Michael Kaufmann
Fixes#17611Closes#17638
cfilter/conn: change send/recv function signatures. Unify the
calling/return conventions in our send/receive handling.
Curl_conn_recv(), adjust pnread type
Parameter `pnread` was a `ssize_t *`, but `size_t *` is better since the
function returns any error in its `CURLcode` return value.
Closes#17546
Move curlx_ functions into its own subdir.
The idea is to use the curlx_ prefix proper on these functions, and use
these same function names both in tool, lib and test suite source code.
Stop the previous special #define setup for curlx_ names.
The printf defines are now done for the library alone. Tests no longer
use the printf defines. The tool code sets its own defines. The printf
functions are not curlx, they are publicly available.
The strcase defines are not curlx_ functions and should not be used by
tool or server code.
dynbuf, warnless, base64, strparse, timeval, timediff are now proper
curlx functions.
When libcurl is built statically, the functions from the library can be
used as-is. The key is then that the functions must work as-is, without
having to be recompiled for use in tool/tests. This avoids symbol
collisions - when libcurl is built statically, we use those functions
directly when building the tool/tests. When libcurl is shared, we
build/link them separately for the tool/tests.
Assisted-by: Jay Satiro
Closes#17253
With a meta_hash at each connection (similar to easy handle, let
multi_ev.c store its pollsets as meta data, no longer needing its own
hashes.
This eliminates the last use of Curl_hash_offt. Remove it.
Closes#17095
Change multi's book keeping of transfers to no longer use lists, but a
special table and bitsets for unsigned int values.
`multi-xfers` is the `uint_tbl` where `multi_add_handle()` inserts a new
transfer which assigns it a unique identifier `mid`. Use bitsets to keep
track of transfers that are in state "process" or "pending" or
"msgsent".
Use sparse bitsets to replace `conn->easyq` and event handlings tracking
of transfers per socket. Instead of pointers, keep the mids involved.
Provide base data structures and document them in docs/internal:
* `uint_tbl`: a table of transfers with `mid` as lookup key,
handing out a mid for adds between 0 - capacity.
* `uint_bset`: a bitset keeping unsigned ints from 0 - capacity.
* `uint_spbset`: a sparse bitset for keeping a small number of
unsigned int values
* `uint_hash`: for associating `mid`s with a pointer.
This makes the `mid` the recommended way to refer to transfers inside
the same multi without risk of running into a UAF.
Modifying table and bitsets is safe while iterating over them. Overall
memory requirements are lower as with the double linked list apprach.
Closes#16761
When pausing a HTTP/2 transfer, the stream's local window size
is reduced to 0 to prevent the server from sending further data
which curl cannot write out to the application.
When unpausing again, the stream's window size was not correctly
increased again. The attempt to trigger a window update was
ignored by nghttp2, the server never received it and the transfer
stalled.
Add a debug feature to allow use of small window sizes which
reproduces this bug in test_02_21.
Fixes#16955Closes#16960
When a PUSH_PROMISE was received, the h2_stream object was assigned
to the wrong `newhandle->mid` and was thereafter not found. This led
to internal confusion, because the nghttp2 stream user_data was not
cleared and an invalid easy handle was use for trace messages,
resulting in a crash.
Reported-by: Viktor Szakats
Fixes#16881Closes#16905
The issues found fell into these categories, with the applied fixes:
- const was accidentally stripped.
Adjust code to not cast or cast with const.
- const/volatile missing from arguments, local variables.
Constify arguments or variables, adjust/delete casts. Small code
changes in a few places.
- const must be stripped because an API dependency requires it.
Strip `const` with `CURL_UNCONST()` macro to silence the warning out
of our control. These happen at API boundaries. Sometimes they depend
on dependency version, which this patch handles as necessary. Also
enable const support for the zlib API, using `ZLIB_CONST`. Supported
by zlib 1.2.5.2 and newer.
- const must be stripped because a curl API requires it.
Strip `const` with `CURL_UNCONST()` macro to silence the warning out
of our immediate control. For example we promise to send a non-const
argument to a callback, though the data is const internally.
- other cases where we may avoid const stripping by code changes.
Also silenced with `CURL_UNCONST()`.
- there are 3 places where `CURL_UNCONST()` is cast again to const.
To silence this type of warning:
```
lib/vquic/curl_osslq.c:1015:29: error: to be safe all intermediate
pointers in cast from 'unsigned char **' to 'const unsigned char **'
must be 'const' qualified [-Werror=cast-qual]
lib/cf-socket.c:734:32: error: to be safe all intermediate pointers in
cast from 'char **' to 'const char **' must be 'const' qualified
[-Werror=cast-qual]
```
There may be a better solution, but I couldn't find it.
These cases are handled in separate subcommits, but without further
markup.
If you see a `-Wcast-qual` warning in curl, we appreciate your report
about it.
Closes#16142
nghttp2 will on its own send GOAWAY frames, closing the connection, when
internal processing of frames runs into errors. This may not become
visible in a direct error code from a call to nghttp2.
Check for session being closed on ingress processing (on sending, we
already did that) and report an error if so. In addition, monitor
outgoing GOAWAY not initiated by us so that the user will get a fail
message when that happens.
Add some more long response header tests.
Closes#16544
When the server sends HEADER/CONTINUATION frames that exceed nghttp2's
size, this error is being reported via the on_invalid_frame_recv
callback. Without registering there, it will go unnoticed.
RST the stream when such a frame is encountered.
Closes#16544
We send a GOAWAY, but some servers ignore that and happily continue
sending the stream response. RST the stream when response header errors
are encountered.
Fixes#16535
Reported-by: Peng-Yu Chen
Closes#16544
This is a partial fix of #16535. The error message format is borrowed
from the existing code[1].
Sample message before:
curl: (56) process_pending_input: nghttp2_session_mem_recv() returned -902:The user callback function failed
Sample message after:
curl: (56) Error receiving HTTP2 header: 100(A value or data field grew larger than allowed)
[1]: df672695e5/lib/http2.c (L1999-L2000)Closes#16536
Add a standalong hash table for curl_offt_t as key. This allows a
smaller memory footprint and faster lookups as we do not need to deal
with variable key lengths.
Use in all places we had the standard hash for this purpose.
Closes#16442
Adds a "cw-pause" client writer in the PROTOCOL phase that buffers
output when the client paused the transfer. This prevents content
decoding from blowing the buffer in the "cw-out" writer.
Added test_02_35 that downloads 2 100MB gzip bombs in parallel and
pauses after 1MB of decoded 0's.
This is a solution to issue #16280, with some limitations:
- cw-out still needs buffering of its own, since it can be paused
"in the middle" of a write that started with some KB of gzipped
zeros and exploded into several MB of calls to cw-out.
- cw-pause will then start buffering on its own *after* the write
that caused the pause. cw-pause has no buffer limits, but the
data it buffers is still content-encoded.
Protocols like http/1.1 stop receiving, h2/h3 have window sizes,
so the cw-pause buffer should not grow out of control, at least
for these protocols.
- the current limit on cw-out's buffer is ~75MB (for whatever
historical reason). A potential content-encoding that blows 16KB
(the common h2 chunk size) into > 75MB would still blow the buffer,
making the transfer fail. A gzip of 0's makes 16KB into ~16MB, so
that still works.
A better solution would be to allow CURLE_AGAIN handling in the client
writer chain and make all content encoders handle that. This would stop
explosion of encoding on a pause right away. But this is a large change
of the deocoder operations.
Reported-by: lf- on github
Fixes#16280Closes#16296
Add a 'wanted' major HTTP version bitmask next to the 'allowed' bitmask
in HTTP version negotiation. This will try connections as specified in
'wanted', but enabled Alt-Svc and HTTPS-RR to redirect to other major
HTTP versions, if those are 'allowed'.
Changes libcurl internal default to `CURL_HTTP_VERSION_NONE` and removes
the code in curl that sets `CURL_HTTP_VERSION_2TLS` if the command line
does not say anything else.
Closes#16117
Remove `blocking` argument from cfilter's connect method.
Implement blocking behaviour in Curl_conn_connect() instead for all
filter chains.
Update filters implementations. Several of which did never use the
paramter (QUIC for example). Simplifies connect handling in TLS filters
that no longer need to loop
Fixed a blocking connect call in FTP when waiting on a socket accept()
which only worked because the filter did not implement it.
Closes#16397
Translate the `data->set.httpwant` which is one of the consts from the
public API (CURL_HTTP_VERSION_*) into a major version mask plus
additional flags for internal handling.
`Curl_http_neg_init()` does the translation and flags setting in http.c,
using new internal consts CURL_HTTP_V1x, CURL_HTTP_V2x and CURL_HTTP_V3x
for the major versions. The flags are
- only_10: when the application explicity asked fro HTTP/1.0
- h2_upgrade: when the application asks for upgrading 1.1 to 2.
- h2_prior_knowledge: when directly talking h2 without ALPN
- accept_09: when a HTTP/0.9 response is acceptable.
The Alt-Svc and HTTPS RR redirections from one ALPN to another obey the
allowed major versions. If a transfer has only h3 enabled, Alt-Svc
redirection to h2 is ignored.
This is the current implementation. It can be debated if Alt-Svc should
be able to override the allowed major versions. Added test_12_06 to
verify the current restriction.
Closes#16100
The h2 filter mistakenly also checked `sendbuf` when asked
about pending data. The call is only meant to account for
buffered data that still needs to be received.
Also, remove obsolete recvbuf in stream as we write received
headers and data directly.
Fixes#16084Closes#16098
Reported-by: Deniz Sökmen
The variable `conn->httpversion` was used for several purposes and it
was unclear at which time the value represents what.
- rename `conn->httpversion` to `conn->httpversion_seen`
This makes clear that the variable only records the last
HTTP version seen on the connection - if any. And that it
no longer is an indication of what version to use.
- Change Alt-Svc handling to no longer modify `conn->httpversion`
but set `data->state.httpwant` for influencing the HTTP version
to use on a transfer.
- Add `data->req.httpversion_sent` to have a record of what
HTTP version was sent in a request
- Add connection filter type CF_TYPE_HTTP
- Add filter query `CF_QUERY_HTTP_VERSION` to ask what HTTP
filter version is in place
- Lookup filters HTTP version instead of using `conn->httpversion`
Test test_12_05 now switches to HTTP/1.1 correctly and the
expectations have been fixed.
Removed the connection fitler "is_httpN()" checks and using
the version query instead.
Closes#16073
Make transfer attach/detach to/from connections chepaer.
- the "attach" event was no longer implemented by any filter
- the "detach" did the same as the "done" event for the filters
who still implemented it. It should be superfluous as the "done"
must always happen.
Closes#16067
build:
- autotools: fix to build generated sources for the `tidy` target.
- autotools: allow passing custom clang-tidy options via
`CURL_CLANG_TIDYFLAGS` env.
- cmake: add `CURL_CLANG_TIDY` option to configure for `clang-tidy`.
Also add:
- `CLANG_TIDY` variable to customize the `clang-tidy` tool.
- `CURL_CLANG_TIDYFLAGS` to pass custom options to `clang-tidy`.
- apply `--enable-werror` and `-DCURL_WERROR=ON` to `clang-tidy`.
CI/GHA:
- add clang-tidy job for Linux, using autotools and clang-tidy v18.
This one needs to disable `clang-analyzer-valist.Uninitialized`
to avoid false positives:
https://github.com/llvm/llvm-project/issues/40656
Duration: 5.5 minutes
- add clang-tidy job for macOS, using cmake and clang-tidy v19.
This one also covers tests and examples, and doesn't hit the false
positives seen with llvm v18 and earlier.
Duration: 4.5 minutes
- Linux/macOS: skip installing test dependencies when not building or
running tests.
fix fallouts reported by `clang-tidy`:
- lib:
- cf-h2-proxy: unused assignment in non-debug builds.
- cf-socket: silence warning.
FIXME: https://github.com/curl/curl/pull/15825#issuecomment-2561867769
- ftp: NULL passed to `strncmp()`.
- http2: NULL-ptr deref.
- mprintf: silence warning.
- src/tool_writeout: NULL passed to `fputs()`.
- examples:
- invalid file pointers.
- missing `fclose()`.
- tests:
- http/clients/hx-download: memory leaks on error.
- http/clients/hx-download: memory leak on repeat `-r` option.
- server: double `fclose()`.
https://www.man7.org/linux/man-pages/man3/fclose.3.html
- server: invalid file pointer/handle.
- server/getpart: unused assignments.
- server/mqttd: leak on failed `realloc()`.
- server/tftpd: NULL passed to `strcmp()`.
Closes#15825
Provide libcurl's memory functions as callbacks to replace nghttp2's own memory
functions. This allows custom memory callbacks provided by users of libcurl to
be used by nghttp2 as well.
Closes#15527
When a server signals EOS from its side and the curl upload is
unfinished and the server has not given a positive HTTP status response,
auto RST the stream to signal that the upload is incomplete and that the
whole transfer can be stopped.
Fixes the case where the server responds with 413 on an upload but does
not RST the stream from its side, as httpd and others do.
Reported-by: jkamp-aws on github
Fixes#15316Closes#15325
Based on #14135, implement TLSv1.3 earlydata support for the curl
command line, libcurl and its implementation in GnuTLS.
If a known TLS session announces early data support, and the feature is
enabled *and* it is not a "connect-only" transfer, delay the TLS
handshake until the first request is being sent.
- Add --tls-earldata as new boolean command line option for curl.
- Add CURLSSLOPT_EARLYDATA to libcurl to enable use of the feature.
- Add CURLINFO_EARLYDATA_SENT_T to libcurl, reporting the amount of
bytes sent and accepted/rejected by the server.
Implementation details:
- store the ALPN protocol selected at the SSL session.
- When reusing the session and enabling earlydata, use exactly
that ALPN protocol for negoptiation with the server. When the
sessions ALPN does not match the connections ALPN, earlydata
will not be enabled.
- Check that the server selected the correct ALPN protocol for
an earlydata connect. If the server does not confirm or reports
something different, the connect fails.
- HTTP/2: delay sending the initial SETTINGS frames during connect,
if not connect-only.
Verification:
- add test_02_32 to verify earlydata GET with nghttpx.
- add test_07_70 to verify earlydata PUT with nghttpx.
- add support in 'hx-download', 'hx-upload' clients for the feature
Assisted-by: ad-chaos on github
Closes#15211
booleans should use the type 'bool' and set the value to TRUE/FALSE
non-booleans should not be 'bool' and should not set the value to
TRUE/FALSE
Closes#15123
When uploading data from stdin ('-T -'), and the EOS was only detected
on a 0-length read, the EOS was not forwarded to the filters. This led
HTTP/2 to hang on not forwarding this to the server.
Added test_07_14 to reproduce and verify.
Fixes#14870
Reported-by: nekopsykose on github
Closes#14877
