Use separate dns cache entries for addresses (A+AAAA) and HTTPS-RR
results. That makes also "negative" results independent of each other.
Dns cache entries, once in use, can no longer be modified safely, as
concurrent use would require each access then to be done under lock. By
using separate entries, we can update a HTTPS entry without needing to
duplicate an existing address entry for the same host+port.
Connection filters can now ask for DNS resolves. This works at any time
during connection setup and while connect is going on.
`Curl_conn_dns_add_addr_resolve()` and
`Curl_conn_dns_add_https_resolve()` are used for that. They check if the
resolve is already ongoing, can be added to a resolve not started yet
or, as last resort, create a new filter instance and add it to the
connection (it's easier to add more filters than making the same filter
handle multiple resolves. Since DNS filters are removed once the
connection is established, there is no later penalty).
HTTPS-RR queries are added by the `HTTPS-CONNECT`, `SSL` and `QUIC`
filters. The latter will only do that when ECH is configured and
supported. That means we trigger HTTPS-RR queries only when the results
matter.
Add test_22_06 for ALPN influenced via HTTPS-RR when tunneling through a
proxy. This did not work before.
Adjust test2100 to use https: as the previous http: URL no longer
triggers HTTPS-RR resolves.
Closes#22216
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
- 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
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
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 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
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
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
- 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
Fix a regression in curl 8.21.0-rc3: Check whether the host and the "connect
to host" are equal before initializing the proxy. If they are equal, switching
to tunnel mode is not necessary.
Follow-up to 73daec6Closes#22110
The header path matches these prefixes case sensitively, as 5af0165562
made it for cookie spec reasons, but the Netscape cookie-file path still
used a case-insensitive match. Align the file path so a differently
cased name like __secure-x is treated as an ordinary cookie instead of
being put through the prefix integrity checks.
Extended test 2311 to cover it.
Closes#22085
Add test 2108 covering the rejection over FTP. Drop the now-vestigial
nul bytes from test 1282; they exercised the removed Kerberos FTP
security buffer check and now trip this rejection before the 633
login-denied path is reached.
Closes#21996
Yes, we were counting the "live" transfers before, but were they
*really* alive?
When determining to add the wakeup socket to fdset/waitfds etc, we
should only do that when the multi handle is actually processing
transfers. Other wise, the application could wait on the wakeup socket
forever.
For this, we counted `multi->xfers_alive` (e.g. the "running" number
returned by `curl_multi_perform()`). This was almost correct.
The problem is that added easy handles are counted as "alive" right away
on the addition. But the processing has not started yet. They did not
trigger any DNS resolves or opened any sockets yet.
Add two fields in multi and easy handle:
* `multi->xfers_really_alive`: counts the "alive" transfers that have
passed `MSTATE_INIT` (at least once)
* `data->state.really_alive`: to track if the transfer has been counted
Add test 2412 to check that adding transfers without perform will not
trigger the wakeup socket to be added.
Fixes#22050
Reported-by: Bryan Henderson
Closes#22066
Since we have the quirky of empty credentials (the empty string for
username and password) for Negotiate reactivated, we need to check for
this when considering Basic and Digest auth.
Verify a redirect to blank user+password in test 2208
Closes#22060
`Curl_secure_context()` checked `conn->scheme` to determine if Secure
cookies may be sent. Since 73daec6, `conn->scheme` is set to the proxy's
scheme when using an HTTPS forwarding proxy, causing the function to
return TRUE for HTTP origins. This leaked Secure cookies over the
plaintext connection between proxy and origin.
Use `data->state.origin->scheme` instead, which always reflects the
origin's scheme regardless of proxy configuration.
Not an approved vulnerability because the regression was introduced
after the last release and is not present in any released version.
Verified by test 3401
Follow-up to 73daec6620
Reported-by: daviey on hackerone
URL: https://hackerone.com/reports/3803415Closes#22024
- `N byte` -> `N-byte` or `N bytes`.
- INTERNALS.md: language tweaks.
- schannel: language tweak in comment/error message.
- socks_gssapi, socks_sspi: simplify composing an error message.
(at a cost of 8 extra constant string bytes.)
- m4/curl-compilers.m4: fix typo in link (in comment).
- contrithanks.sh: fix indent, drop stray `;` terminator.
- lib, src, tests: drop/fix a bunch of badwords.
- fix typos in comments.
- fix indent, stray spaces.
Some of these spotted by GitHub Code Quality and Copilot
Closes#22009
... even though they use the curl option "--resolve".
This makes it more convenient to choose or skip the tests.
For example Privoxy's cts test framework relies on the "--resolve"
keyword when executing the "upstream-tests" scenario to skip curl
tests that aren't expected to work when the requests are made through
Privoxy. While some of the modified tests are already skipped for
other reasons through other means when testing Privoxy, it's good to
be consistent.
Closes#21930
Even when two 'chunked' are listed and neither is the last encoding the
transfer is rejected.
Verified by test 1722 and 1723
Reported-by: violet12331 on hackerone
Closes#21966
They were not exact duplicates. Tweaked their names to clarify.
Also renamed 608 accordingly.
Follow-up to 4ead4285a6
Spotted-by: Dan Fandrich
Closes#21938
- test 1030: remove, duplicate of 154
- test 1105: make name unique
- test 161: make name reflect what it tests
- test 2074: correct the name
- test 310: improve name
- test 358: correct the name
- test 409: removed, duplicate of 401
- test 472: clarify the test name (how it differs from 439)
- test 1509: update name
- test 527: duplicate of 526
- test 758: separate the name from 530
- test 611: duplicate of 608, remove
- test 639: adjust the name
- test 688: minor name tweak to clarify
- test 708: enhance name
- test 800/847: clarify the names
- test 1520: dedupe the name
- test 962: enhance name
- test 1196/2203: enhanced names
- test 1211: name tweak
- test 1256/1257: enhance the names
- test 1483: fix name
- test 1541: fix name
- test 1553: fix name
- test 1609: removed, exact duplicate of 1607
- test 2200: fix name
- test 3031: corret the name
- test 3016/3203: fix names and keywords
- test 3201/3220: enhance names
- test 3212: fix name
- add missing FILE keywords
- drop FAILURE as keyword
Closes#21936
