- keep pause state at request
- keep presense of content decoding writers at request
- only add cw-pause writer when content decoding happens
- define macro for passing data down the writer chain
scorecard: add option to test with h2o server
Closes#22335
For threaded resolving, added an additional socket/eventfd pair to the
multi handle for notifications from threads. The original "double use"
of the standard wakeup pair did lead to regressions for apps.
The API definition of curl_multi_poll/wait/wakeup is pretty tight
regarding what effects what and adding notifications on top of that
broke what apps perceived to be the contract.
Fixes#22272
Reported-by: Sergei Zimmerman
Closes#22274
As reported in the bug:
> On a 32-bit architecture (i386):
> 1. `chunks * chunk_size` evaluates to `unsigned int` (32-bit
> unsigned).
> 2. Due to Usual Arithmetic Conversions in C, the signed `-1` (int)
> operand of the ternary operator is promoted to `unsigned int`,
> yielding `4294967295` (0xFFFFFFFF).
> 3. The ternary operator returns `4294967295` as an `unsigned int`.
> 4. This unsigned value is then assigned to `r->clength` (apr_off_t,
> 64-bit signed). Since the source is unsigned, it is zero-extended,
> resulting in `r->clength` becoming `+4294967295`.
> 5. The subsequently executed check `if(r->clength >= 0)` evaluates to
> true.
> 6. Inside the block, `apr_ltoa(r->pool, (long)r->clength)` casts it to
> a 32-bit signed `long` (on 32-bit platforms), which truncates it
> back to `-1`, formatting it as "-1" and sending the
> "Content-Length: -1" header.
>
> On a 64-bit architecture (amd64), `size_t` is 64-bit, and `-1` (32-bit
> int) is promoted to `unsigned long` (64-bit), yielding
> `18446744073709551615`. When assigned to `r->clength` (64-bit signed),
> it wraps back to `-1`, which correctly skips the Content-Length
> generation.
>
> This type promotion mismatch can be safely fixed by avoiding the
> signed/unsigned mixture in the ternary operator.
Origin: debian, https://bugs.debian.org/1140793
Bug-Debian: https://bugs.debian.org/1140793
These are standard Debian patches' trailers, I can get rid of then if you prefer.
I've tested the fix using [debusine on i386 runner](https://debusine.debian.net/debian/developers/artifact/4083252/raw/log):
> 782s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[http/1.0] PASSED [ 17%]
> 782s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[http/1.1] PASSED [ 18%]
> 782s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[h2] PASSED [ 18%]
Versus the [i386 run in Debian's infrastructure](https://debusine.debian.net/debian/developers/artifact/4084220/raw/log):
> 2690s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[http/1.0] FAILED [ 17%]
> 2690s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[http/1.1] PASSED [ 18%]
> 2690s tests/http/test_05_errors.py::TestErrors::test_05_04_unclean_tls_shutdown[h2] PASSED [ 18%]
Closes#22210
- Adjust pollset during connection shutdown.
- Separate the FIRSTSOCKET and SECONDSOCKET check so that one being in
connect or shutdown no longer adds poll events for the other.
- Fix shutdown state evaluation (as detailed below).
- Add a unit test for Curl_conn_adjust_pollset.
- Add a client for event testing.
- Test that shutdown sockets stay with the socket callback until done.
The pollset predicate now reads the connection's own shutdown state
rather than going through data->conn, and the EXPIRE_SHUTDOWN arming in
cshutdn_perform() works again: next_expire_ms started at 0 and could
never be set, so a peer that never sends a close_notify would still park
its connection forever, timeout or not.
Reported-by: pszemus@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/22282
Closes https://github.com/curl/curl/pull/22304
`test798` is the only test fetching `http://localhost:%HTTPPORT` without
`-4`, since it needs the hostname for its folded `domain=localhost`
cookie. curl tries `::1` first, but the test HTTP server is IPv4-only.
On the BSDs, IPv4 and IPv6 have separate ephemeral port namespaces, so
with every test server binding port 0, another parallel runner's
IPv6-bound server can hold the same numeric port. curl then connects to
the wrong runner's server, which cannot open its own `log/N/test798` and
closes without a response, giving exit 52 and an empty `server.input`.
This PR fixes this flake by adding `-4` matches what tests 389 and 392
already do. Linux is immune because wildcard IPv6 binds occupy the IPv4
port too. Seen in
https://github.com/curl/curl/actions/runs/29229170329/job/86749470571.
Closes#22318
E.g. on CHERI pointers are 128 bits [1]. This causes the unit3214 test
to fail, which was written with more traditional platforms in mind. Here
is the output of log/stderr3214:
```
URL: -
BAD: struct Curl_easy is 7984 bytes, allowed to be 5370: 2614 bytes too big
BAD: struct connectdata is 1408 bytes, allowed to be 1300: 108 bytes too big
BAD: struct Curl_multi is 1248 bytes, allowed to be 850: 398 bytes too big
BAD: struct curl_httppost is 224 bytes, allowed to be 112: 112 bytes too big
BAD: struct curl_slist is 32 bytes, allowed to be 16: 16 bytes too big
BAD: struct curl_khkey is 32 bytes, allowed to be 24: 8 bytes too big
BAD: struct curl_hstsentry is 48 bytes, allowed to be 40: 8 bytes too big
BAD: struct curl_mime is 144 bytes, allowed to be 96: 48 bytes too big
BAD: struct curl_mimepart is 592 bytes, allowed to be 440: 152 bytes too big
BAD: struct curl_certinfo is 32 bytes, allowed to be 16: 16 bytes too big
BAD: struct curl_tlssessioninfo is 32 bytes, allowed to be 16: 16 bytes too big
BAD: struct curl_blob is 32 bytes, allowed to be 24: 8 bytes too big
BAD: struct CURLMsg is 48 bytes, allowed to be 24: 24 bytes too big
BAD: struct curl_header is 80 bytes, allowed to be 48: 32 bytes too big
Test ended with result 14
```
Multiply the allowed size on systems with larger than 64-bit pointers.
[1] https://www.cl.cam.ac.uk/research/security/ctsrd/pdfs/20171017a-cheri-poster.pdfCloses#22299
Remove the timestamp passed for ratelimit checks. Let the limit
calculation obtain a timestamp when it needs it. Most transfers run
without active ratelimits and getting a fresh timestamp is unnecessary.
Closes#22292
A CR or LF in the CURLOPT_FTP_ACCOUNT or CURLOPT_FTP_ALTERNATIVE_TO_USER
string split the control-channel command line and smuggled a second FTP
command. Reject a byte below 0x20 in both values before the command is
built.
Closes#22301
- Determine max host and total connection limits from the transfer's own
multi handle and test that it works for shared connections.
Prior to this change, since df67269 (precedes 8.13.0),
Curl_cpool_check_limits() took the limits from the multi handle that
owns the pool. A share-owned pool is not owned by any multi, so the
limit check always passed.
Prior to df67269 the limits came from the transfer's own multi handle.
Fixes https://github.com/curl/curl/issues/22265
Closes https://github.com/curl/curl/pull/22266
Websocket frames need to be written individualy, so that applications
can access the frame meta data correctly. This worked until the write
function triggered a pause. Remaining frames accumulated in the "out"
writer's buffer and on unpaused were written in one chunk.
ws decode writer will now stop writing frames when the client writer
is paused. To handle the writing of buffered raw data after an unpause,
client writers have gotten a new "flush" method.
Add pytest test_20_12 with a new client to check handling of pauses
and websocket frames.
Reported-by: Hendrik Hübner
Fixes#22273Closes#22283
Test 1701 did a HTTP/2 Upgrade in a HTTP/1.1 POST request. This will no
longer be supported in the upcoming nghttp2 release for security reasons
(Upgrade requests with body a re messy, error prone and can easily lead
to connection confusion).
As no other server we have supports this, remove the test.
Reported-by: firexinghe on github
Fixes#22280Closes#22291
The curl tool was creating a new transfer every time it checked if
it needed to add one to reach max parallelism. This led to eventually
all configured transfers to have easy handles created.
Limit the creation again to the ones needed for max parallelism.
scorecard.py: set --out-null only for curl versions that support it
Closes#22277
The test provides an invalid username, but no ':' with password, so that
the curl tool tries to read the password from stdin. That makes the test
hang, also in CI.
The check for invalid characters is happening when the CURLOPT_USERPWD
is applied to the URL, so when the transfer is started. Maybe the check
should happen also in setopt already.
The test hangs because tool_paramhlp.c:548 `checkpasswd()` does not see
a ':' and tries to read the password from stdin.
Fix: add a ':' in the tests invalid user setting, so that the read does
not happen.
Follow-up to ecaa344a7aCloses#22264
FTP servers using SSL can be configured to check TLS session reuse on
the DATA connection. They hand out a new session on every CONTROL
connect and require to see the client using exactly that one when
up-/downloading on DATA.
This means:
1. We have to configure the SSL filter on the DATA connection with
exactly the same peers.
2. We have to remember the SSL session on the CONTROL connection -
separately from the session cache. The SSL filter on the DATA
connection then looks for a session at the CONTROL filter and, if
present, uses that.
Tests:
Enable `require_ssl_reuse` in our pytest setup for vsftpd. This
reproduces the problem reported in #22225 and verifies the fix.
Skip ftp+SSL pytests for rustls, as we have no possibility to reuse
sessions in that backend.
Schannel: we do not run these tests with the backend. I expect it has
similar problems but am not able to verify.
Reported-by: Laurent Sabourin
Fixes#22225Closes#22246
Add classic tests 3223-3226 and extend tests/http/test_12_reuse.py so that
connection reuse decisions are explicit:
- partial/aborted HTTP/1.1 responses must not reuse the connection
(premature multi_done closes non-multiplexed conns)
- different target hostnames must not share a connection even when they
resolve to the same address (url_match_destination)
- HTTP Basic credentials are per-request (PROTOPT_CREDSPERREQUEST), so
different -u values still reuse; assert that with num_connects
- same host positive control reuses (num_connects 1 then 0)
Closes#22224
A sasl zero-length normal response is an empty string, unlike an initial
response. Since the encoding of a zero-length initial response is
protocol dependent, move its handling to the protocol-specific sendauth
procedure. Similarly, do not check for an '=' server message, as this
should normally never occur.
Update the erroneous tests accordingly.
Closes#22218
A gzip Content-Encoding response holding more than one gzip member made
curl decode the first member and then fail the transfer with a bare
CURLE_WRITE_ERROR and no hint about the cause. Detect the trailing
member and fail with a message that explains what happened.
Fixes#22156Closes#22172
Also repurpose existing build-time feature checks into unit test 1961,
to verify.
Prior to this patch these functions were auto-detected with both
autotools and cmake. In case of autotools there was an extra
verification phase ensuring the functions work as expected. This step
required running the function, thus was limited to non-cross-builds. For
cross-builds and CMake it always used the system implementation if
present. On Windows it always used the local implementation, because
availability/use is complicated there.
After this patch all platforms, always use the local implementation,
which is known to be accurate. This makes curl behave more consistently,
and simplifies the build process, a fixes cross-builds and CMake
auto-detection differences.
Also:
- test1960: enable unconditionally.
- checksrc: disallow globally, allowlist in `block_ip.c` example.
- dnsd: verify ntop result for NULL before passing to printf.
Ref: https://github.com/curl/curl/pull/22137#issuecomment-4797440983
Ref: #22137
Ref: 8537a5b0bc#16577Closes#22170
By exposing and reusing existing custom type via `vtls/openss.h`.
Also:
- GHA/curl-for-win: test in CI by enabling building tests.
Cost is 45s per job, so limit it to the gcc job.
Seen with Windows x64 gcc (not tested in CI prior to this patch):
```
tests/libtest/lib1587.c:50:7: error: conversion from 'opt1587' {aka 'long long unsigned int'} to 'long int' may change value [-Werror=conversion]
50 | SSL_CTX_set_options(info->internals, opts);
| ^~~~~~~~~~~~~~~~~~~
tests/libtest/lib1587.c:59:7: error: conversion from 'opt1587' {aka 'long long unsigned int'} to 'long int' may change value [-Werror=conversion]
59 | SSL_set_options(info->internals, opts);
| ^~~~~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/28258372229/job/83727170184?pr=22195#step:3:4884
Ref: da2f05e6f6#22198
Ref: 6163566461#22197
Follow-up to 3e40ccb875#21290
Follow-up to 2db8ae480f#17809#17801Closes#22195
It fails as expected, with a list of errors:
```
In file included from _x64-win-ucrt-for-trurl-bld/tests/tunit/tunits.c:5:
tests/tunit/tool1394.c:76:11: error: call to undeclared function 'parse_cert_parameter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
76 | err = parse_cert_parameter(p->param, &certname, &passphrase);
| ^
[...]
tests/tunit/tool1622.c:68:5: error: call to undeclared function 'timebuf'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
68 | timebuf(buffer, sizeof(buffer), secs);
| ^
[...]
tests/tunit/tool1623.c:104:26: error: call to undeclared function 'GetSizeParameter'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
104 | ParameterError err = GetSizeParameter(check[i].input, &output);
| ^
[...]
```
Ref: https://github.com/curl/curl/actions/runs/28256499057/job/83720797064?pr=22195
Ref: #22195
Ref: 640fed88b6Closes#22198
- cmp-pkg-config.sh: replace `-r -f` with `-rf` to match rest of repo.
- configure.ac: add double quotes for robustness (not a bug).
- curl-openssl.m4: merge nested `if`s.
- CurlTests.c: drop `!= 0`, also to sync with m4.
- CurlTests.c: replace `example.com` with `localhost` in
`gethostbyname()` feature test code. (compile-only, not a bug)
- GHA/http3-linux: drop literal `true` from bool expression.
- lib650: drop redundant `&`.
- move variable/call to left-hand side of equality checks, where
missing.
- perl: detach `<`/`>` from filename in `open()`, where missing.
- schannel: apply two nit fixes lost in rebase.
- scripts/verify-release: drop redundant double quotes.
- scripts/verify-release: exit with error code on error.
- synctime: replace magic numbers with `sizeof()`.
- telnet: add missing parentheses to macro value.
- tests/Makefile.am: use single quotes.
- tool_operate: drop redundant `break` after `return` in VMS code.
- unit2413: drop unused NULL pointer + free call.
- unit2413: fix duplicate test case name.
- urlapi: drop redundant parentheses.
- urlapi: drop `CURL_UNCONST()` that became redundant.
Closes#22186
A custom header name padded with blanks (`Authorization :`) missed the
exact-length compare and slipped past the cross-host Authorization and
Cookie drop, forwarding the header to the redirect target. Trim the
parsed name in both the request and proxy CONNECT header builders.
Closes#22178
- there is a risk for a nasty recursive situation
- we avoid certain risks that the pause call changes things so that when
returning from the callback, the state of some internals is undefined
and we need to reload which is easy to miss
- we can't think of legitiate use cases for doing this. This is
basically just the new favorite point for AI and security researchers
to find hypothetical problems
Closes#22179
- GHA/windows: mention `IgnoreStandardErrorWarningFormat=true`
in comment.
- cmake: fix MIT/GNU GSS order in messages.
- drop some exclamation marks from messages.
- drop redundant ending newlines from messages.
- fold/unfold where possible.
- fix indent, whitespace, typos and other nits.
Closes#22185
- Makefile.am: add root `test-quiet` target that maps to
`tests/quiet-test`, for completeness.
Ref: #22098
- tests/Makefile.am: drop unused remains of `MANFILES`.
Follow-up to fa3f889752#17463
Also replace the `distclean` target to delete `CLEANFILES`, otherwise
the CI dist job fails. (I'm not grokking why)
- tests/Makefile.am: sync up `tests` target with CMake, to pass `TFLAGS`
`-a`.
Follow-up to 904e7ecb66#19347
- tests/certs/Makefile.am: fix to clean generated certs via `CLEANFILES`
to comply with autotools documentation, which says the previously used
variable `DISTCLEANFILES` is meant for files created by `./configure`.
Ref: https://www.gnu.org/software/automake/manual/html_node/Clean.html
Follow-up to 44341e736a#16824
- tests/http/Makefile.am: add `./` prefix to glob passed to `rm -rf` on
clean, to avoid deleting unintented files, or pass unintended options.
Follow-up to 671158242d#10349
Cherry-picked from #22102
Cherry-picked from #22150Closes#22154
Where missing. To avoid passing an option by accident.
End-of-option marker (`--`) is not POSIX, but most of these scripts are
internal and/or CI-focused, where this is fine. `maketgz` and
`verify-release` are meant for public use, and I asses this is fine too,
but let us know if this causes issues in real world envs.
Also:
- maketgz: pass args with `:?` to `rm -rf`, where missing.
Cannot cause an actual issue with current code.
- verified `cp`, `rm` instances too, but none were affected.
- tests/cmake/test.sh: replace `$PWD` with `$(pwd)` for sturdiness.
- appveyor.sh: replace `$PWD` with `$(pwd)` for sturdiness.
Assisted-by: Dan Fandrich
Follow-up to 6aab1dc263#19450Closes#22150
