Viktor Szakats [Mon, 9 Jun 2025 16:30:48 +0000 (18:30 +0200)]
GHA/non-native: skip OpenBSD WebSocket tests to mitigate job timeouts
Trying to avoid the occasional ~6-minute long delays seen in the OpenBSD
since last week. The long delay causes the CI job to timeout and fail:
https://github.com/curl/curl/actions/workflows/non-native.yml?page=2&query=branch%3Amaster
The exact reason is or test number is unknown. I base this attempt on
looking at the first occurrences and possible patches that may be
related.
The issue was first seen in CI within PR #17136:
```
[...]
Wed, 07 May 2025 07:10:30 GMT test 3014...[Check if %{num_headers} returns correct number of headers]
Wed, 07 May 2025 07:10:30 GMT s-p----e--- OK (1743 out of 1778, remaining: 00:02, took 0.195s, duration: 01:43)
Wed, 07 May 2025 07:10:30 GMT test 3016...[GET a directory using file://]
[long delay here]
Wed, 07 May 2025 07:16:17 GMT -------
Wed, 07 May 2025 07:16:17 GMT Error: The operation was canceled.
```
Ref: https://github.com/curl/curl/actions/runs/14877264415/job/41776966626#step:3:5566
Ref: https://github.com/curl/curl/actions/runs/14900320627/job/41850699301#step:3:5561 (next in PR)
Then in master, shortly after merging it via d3594be6531df3d5eafcdd09f84ad9dee1777028:
```
[...]
Mon, 02 Jun 2025 09:23:55 GMT test 3201...[HTTP GET when PROXY Protocol enabled and spoofed client IP]
Mon, 02 Jun 2025 09:23:55 GMT --p----e--- OK (1777 out of 1788, remaining: 00:00, took 0.222s, duration: 01:42)
Mon, 02 Jun 2025 09:23:55 GMT RUN: failed to start the HTTP/2 server
Mon, 02 Jun 2025 09:23:55 GMT test 3202...[HTTP-IPv6 GET with PROXY protocol with spoofed client IP]
[long delay here]
Mon, 02 Jun 2025 09:29:48 GMT --p----e--- OK (1778 out of 1788, remaining: 00:00, took 0.1
Mon, 02 Jun 2025 09:29:48 GMT Error: The operation was canceled.
```
Ref: https://github.com/curl/curl/actions/runs/15388587165/job/43292652793#step:3:5097
Ref: https://github.com/curl/curl/actions/runs/15390589464/job/43298911578#step:3:5097 (next in master)
Viktor Szakats [Tue, 27 May 2025 19:20:36 +0000 (21:20 +0200)]
tests: drop mk-bundle exceptions
Using a mixture of techniques to avoid symbols collisions:
- reduce scope.
- add `t*_` / `T*_` prefix.
- move shared functions to `testutil.c`.
(`suburl()`, `rlim2str()`)
- clone re-used lib*.c sources.
(lib587, lib645)
- include shared symbols just once in re-used `lib*.c` sources.
(using `LIB*_C` guards.)
- drop re-used `lib*.c` sources where they were identical or
unused.
- make macros global.
- #undef macros before use.
What remain is the entry functions `test`, and `unit_setup`,
`unit_stop` in unit tests.
Also:
- fix formatting and other minor things along the way.
- add `const` where possible.
- sync some symbol names between tests.
- drop `mk-bundle-hints.sh` that's no longer necessary.
Viktor Szakats [Wed, 21 May 2025 17:37:55 +0000 (19:37 +0200)]
windows: reduce/stop loading DLLs at runtime
- replace dynamic `InitSecurityInterface()` call with early binding and
link `secur32` system DLL.
The library and function are available in all supported curl Windows
targets, meaning WinXP or newer. Add small hack for mingw32ce to
make it build.
- detect and use `if_nametoindex()` on Windows when available. Link
`iphlpapi` system DLL. Requires targeting Vista or newer.
Replacing the dynamic call and the pre-load optimization for lib3026.
Viktor Szakats [Mon, 9 Jun 2025 14:07:05 +0000 (16:07 +0200)]
GHA/windows: make MSVC jobs use MSYS2 libraries: psl, OpenSSL, H2, libssh2
Extend MSVC jobs with the option to use MSYS2 binary package as DLL
dependencies. Allow to use them alone (without vcpkg) or combined with
vcpkg packages. This saves the trouble of building these packages from
source and cache them manually.
This solution requires two tricks:
- workaround for zlib which installs a target-specific `zconf.h` that's
not portable between platforms and C compilers.
- manual dependency configuration in CMake to ensure linking against
the MSYS2 DLLs (and not it static libs). Static libs aren't portable
to MSVC due to missing symbols `__chkstk_ms`, `_stack_chk_fail`,
`_memcpy_chk`, `_stack_chk_guard`, and potentially other issues. CMake
in MSVC mode, `linker.exe` and `pkg-config` pick the static libs by
default. To pick `.dll.a` in favour of `.a`, these tools would have
to be taught about this convention. An alternative is deleting static
libs and see if `.dll.a` are picked-up automatically.
Using MSYS2 packages adds an install step taking 15-45 seconds per job.
It allowed to:
- re-enable libpsl for all MSVC jobs.
- convert the Intel 64-bit job to use MSYS2 without vcpkg, enabling
brotli, zstd, OpenSSL 3.5.0, libssh2 (with OpenSSL cryprography) and
nghttp2.
Using the same technique it's possible to re-enable more features
in MSVC builds, e.g. GnuTLS (also with H3), LibreSSL, mbedTLS, nghttp3,
ngtcp2, libssh, c-ares, gsasl, and replace vcpkg zlib, for faster runs.
What's missing compared to vcpkg is BoringSSL and wolfSSL
(the MSYS2-supplied build doesn't fit curl's requirements IIRC). These
could be built and cached manually.
Also:
- add workaround for zlib (classic) which uses a generated `zconf.h`,
rendering the MSYS2 zlib header incompatible with MSVC.
- set the correct `msystem` for arm64.
- allow using MSVC without vcpkg.
Yedaya Katsman [Wed, 4 Jun 2025 18:22:35 +0000 (21:22 +0300)]
tests: torture: don't duplicate valgrind command
When running torture tests with valgrind enabled, the torture function
got a command line that already had the valgrind invocation in it. It
added another, at the end resulting in an empty valgrind log file.
Remove the duplicate logic adding valgrind, which already had a bit of
different logic that wasn't updated when the other one was.
Stefan Eissing [Sun, 8 Jun 2025 10:00:00 +0000 (12:00 +0200)]
pingpong: on disconnect, check for unflushed pingpong state
When a pingpong based protocol tries to perform a connection disconnect,
it sends a sort of "logout" command to the server, unless the connection
is deemed dead.
But the disconnect might happen before pingpong data has been completely
sent, in which case sending the "logout" will not work. Check the
pingpong state and do not "logout" when data is pending.
This was detected as a condition in fuzzing that triggered a debug
assert in the pingpong sending.
Viktor Szakats [Fri, 6 Jun 2025 21:29:22 +0000 (23:29 +0200)]
GHA: enclose `if` expressions in `${{ }}` where missing
Enclosing expressions in `${{ }}` is optional, but sometimes required.
It seems more straighforward to use it always rather than decide on
a case-by-case basis. Before this patch 71 `if`s were enclosed and 49
were not. Enclosing these makes it easy to grep and recognize these
whereever used.
Dan Fandrich [Fri, 6 Jun 2025 17:21:09 +0000 (10:21 -0700)]
tool_getparam: fix --ftp-pasv
This boolean option was moved to the wrong handling function. Make it
an ARG_NONE and move it to the correct handler and add a test to
verify that the option works.
Daniel Stenberg [Thu, 5 Jun 2025 21:37:59 +0000 (23:37 +0200)]
tests/servers.pm: add more ways to figure out current user
Some CI tests fail due to "Can't start ssh server due to lack of USER name" -
add more ways to try to figure it out if no environment variable works: the
whoami and id commands.
zizmor 1.9.0 effectively bans using GHA macros within shell script
snippets. Rework them to pass GHA macro values via envs and use those
envs in shell snippets. `${{ env.* }}` macros could be converted
to native env references after making their names shell-compatible.
Envs and shell commands can no longer be used in GHA macro values. Most
cases could be fixed by using literals. Passing quoted values with
spaces combined with other args also doesn't work anymore. This was
replaced by passing them separately.
Despite the initial complications, avoiding GHA macros in scripts does
seems to make the CI code reasonable cleaner. It also makes it possible
to analyze the scripts with shellcheck, finding subtle issues that went
unnoticed till now.
Also:
- un-ignore and fix three existing zizmor `template-injection` issues.
- add script to extract and shellcheck all shell code stored within GHA
and Circle CI YAML files.
- add CI job to run this script.
- fix shellcheck issues discovered.
- fix minor differences between cmake and autotools FreeBSD jobs.
- merge cmake/autotools FreeBSD jobs to avoid developing unwanted
differences again.
- fix/sync quote use across shell code.
- replace `$HOME` with `~` or literal where it made sense.
- replace most `brew --prefix` with literals.
- move all curl install tests to the `curl-install*` prefix.
- add missing curl install tests to cygwin/msys/mingw/*bsd.
- pipe to `tar` instead of storing downloads on disk.
- drop unnecessary `PKG_CONFIG_PATH` when building nghttp3.
Stefan Eissing [Wed, 4 Jun 2025 13:26:55 +0000 (15:26 +0200)]
SCP/SFTP: avoid busy loop after EAGAIN
The ssh libraries do not reveal if they still have data buffered from
the peer. Only when their buffers are read empty can curl be sure that
it is safe to rely on socket polling.
This change adds detection of EGAIN on receive in the transfer loop and
allows SFTP/SCP transfers to avoid a busy loop in such a case (which
should happen often when CPU exceeds network bandwidth).
Stefan Eissing [Wed, 4 Jun 2025 07:40:32 +0000 (09:40 +0200)]
test1117: reduce write delays
Test1117 seems to verify that a response, incoming slowly, is read
completely before sending another request on the same connection.
The previsou write delay of 1000ms made the test last 23+ seconds.
A delay of 100ms seems to achieve the same test on modern machines, but
the overall run time is less than 3 seconds.
Viktor Szakats [Mon, 2 Jun 2025 19:15:45 +0000 (21:15 +0200)]
cmake: enable `-std=gnu99` for Windows CE CeGCC
To sync with autotools, which auto-detects this option and enables it by
default.
It also makes it possible to compile unsuffixed long long integer
literals correctly, allowing to drop some legacy macros without bumping
into build errors like:
```
lib/vtls/schannel.c: In function 'schannel_send':
lib/vtls/schannel.c:1815: error: integer constant is too large for 'long' type
```
Ref: https://github.com/curl/curl/actions/runs/15374705821/job/43286736583?pr=17498#step:9:20
Bug: https://github.com/curl/curl/pull/17498#issuecomment-2925507481 Reported-by: Daniel Stenberg
Closes #17523
Jeremy Drake [Tue, 3 Jun 2025 17:35:46 +0000 (10:35 -0700)]
dllmain: exclude from Cygwin builds
On Cygwin, it is unsafe to call POSIX functions from DllMain, which
OPENSSL_thread_stop does. Additionally, it should be unnecessary as
OpenSSL uses pthread_key_create to register a thread destructor to do
thread cleanup in a POSIX way.
Reported-by: Yuyi Wang
Ref: https://cygwin.com/pipermail/cygwin/2025-June/258235.html
Viktor Szakats [Mon, 2 Jun 2025 22:38:20 +0000 (00:38 +0200)]
autotools: recognize more Linux targets when setting `-D_GNU_SOURCE`
To set `-D_GNU_SOURCE`.
Required to declare `accept4` on uclibc targets. `host_os` can have
the value `uclinux-uclibcgnueabi` in this case.
Fixing:
```
cf-socket.c: In function 'cf_tcp_accept_connect':
cf-socket.c:2141:18: error: implicit declaration of function 'accept4'; did you mean 'accept'? [-Werror=implicit-function-declaration]
2141 | s_accepted = accept4(ctx->sock, (struct sockaddr *) &add, &size,
```
Stefan Eissing [Mon, 2 Jun 2025 10:12:13 +0000 (12:12 +0200)]
curl: upload from '.' fix
CURLOPT_NOPROGRESS is being set twice, if a file is uploaded from '.'.
Fix order of options so that '.' can override the global setting. Without
this, the `tool_readbusy_cb()` is never inoked and cannot unpause a
transfer waiting for more input.
Fixes #17513 Reported-by: denandz on github
Closes #17517
This started out as regression tests for the `curl_ws_recv()` and
`curl_ws_send()` implementation and ended up with a bugfix, additional
protocol validation and minor logging improvements.
- Fix reset of fragmented message decoder state when a PING/PONG is
received in between message fragments.
- Fix undefined behavior (applying zero offset to null pointer) in
curl_ws_send() when the given buffer is NULL.
- a) A continuation frame (0x80...) is received without any ongoing
fragmented message.
- b) A new fragmented message is started (0x81/0x01/0x82/0x02...)
before the ongoing fragmented message has terminated.
- Made logs for invalid opcodes easier to understand.
- Moved noisy logs to the `CURL_TRC_WS` log level.
- Unified the prefixes for WebSocket log messages: `[WS] ...`
- Add env var `CURL_WS_FORCE_ZERO_MASK` in debug builds.
- If set, it forces the bit mask applied to outgoing payloads to
0x00000000, which effectively means the payload is not masked at
all. This drastically simplifies defining the expected `<protocol>`
data in test cases.
- 2305: curl_ws_recv() loop reading three larger frames
- This test involuntarily sent an invalid sequence of opcodes (0x01...,0x01...,0x81...) , but neither libcurl nor the test caught this! The correct sequence was tested in 2311 (0x01...,0x00...,0x80...). See below for 2311.
- Validation of the opcode sequence was added to libcurl and is now tested in 2723.
- Superseded by 2719 (fragmented message) and 2707 (large frames).
- 2307: overlong PING payload
- The tested PING payload length check was actually missing, but the test didn't catch this since it involuntarily sent an invalid opcode (0x19... instead of 0x89...) so that the expected error occurred, but for the wrong reason.
- Superseded by 2713.
- 2310: unknown reserved bit set in frame header
- Superseded by 2703 and extended by 2704 and 2705.
- 2311: curl_ws_recv() read fragmented message
- Superseded by 2719 (fragmented message) and 2707 (large frames).
- 2312: WebSockets no auto ping
- Superseded by 2709.
- No tests for `CURLOPT_WRITEFUNCTION`.
- No tests for sending of invalid frames/fragments.
Stefan Eissing [Mon, 2 Jun 2025 08:41:20 +0000 (10:41 +0200)]
tests: improve server start reliability
Fix all lookups of the port a server is using by
- unlinking the portfile before the start
- waiting `timeout` seconds for the port file to exist
and contain a positive number
- check results and fail server start when port could
not be determined
Stefan Eissing [Mon, 2 Jun 2025 07:55:33 +0000 (09:55 +0200)]
test1510: fix expectation
The test had `Closing connection 0` in its expectations, but a stripfile
expression that removes such lines. No recent changes, but started failing
this morning.
Viktor Szakats [Sun, 1 Jun 2025 06:01:23 +0000 (08:01 +0200)]
GHA/non-native: un-ignore tests on OpenBSD, bump to `-j8` for NetBSD/FreeBSD
- un-ignore tests (3017, FTP, TFTP) on OpenBSD Patch-by: Jacob Mealey
Ref: https://github.com/curl/curl/issues/13623#issuecomment-2925129037 Fixes: #13623
- KNOWN_BUGS: drop TFTP failures issue on OpenBSD. Fixes: #13623
- bump test parallelism to `-j8` (from `-j4`) for NetBSD, FreeBSD,
to match OpenBSD.
Before: https://github.com/curl/curl/actions/runs/15371760479
After: https://github.com/curl/curl/actions/runs/15371937205
Yedaya Katsman [Fri, 30 May 2025 15:37:47 +0000 (18:37 +0300)]
tests: fix checks for https-mtls proto
If there were two tests using the "https-mtls" server there was a perl
unbound variable error, since certfile wan't set. Additionally, once the
responsiveserver function was actually called, it failed finding a
responsiveness function. For now I made it use the `verifypid` function,
since the curl execution in `verifyhttp` doesn't know about client
certificates.
Stefan Eissing [Fri, 30 May 2025 10:05:14 +0000 (12:05 +0200)]
ftp: fix teardown of DATA connection in done
When ftp_done() is called to terminate the transfer, it needs to tear
down any open SECONDARY filter chain. The condition on when to do that
was relying on there to be a valid socket. This is not sufficient as the
socket is only set *after* happy eyeballing has decided on one.
Instead of checking for a valid conn->sock, check if any connection
filter is installed.
Stefan Eissing [Sat, 31 May 2025 11:31:03 +0000 (13:31 +0200)]
tests: await portfile to be complete
When running under load, a started server may not produce a valid portfile
before the runner starts reading it. If the read is not successful, wait
for 100ms and try again, timing out after 15 seconds.
Stefan Eissing [Fri, 30 May 2025 09:08:42 +0000 (11:08 +0200)]
wolfssl: fix sending of early data
Early data was reported as being sent, but was not. While everything was
aligned with the Gods and early data was reported as accepted, the
actual sending required another call to wolfSSL.
Daniel Stenberg [Thu, 29 May 2025 08:45:52 +0000 (10:45 +0200)]
tool_getparam: remove two nextarg NULL checks
Because by checking for NULL when the argument can never be NULL, they
mislead the static analyzers into thinking it *can* be NULL and then the
analyzers tend to warn about other places in the code *without* the
check!
Samuel Henrique [Wed, 28 May 2025 22:12:47 +0000 (23:12 +0100)]
docs: fix typos
After Gmail called out the typo I fixed on 532d89a866b5641d380a88144270b1c3ca6ba84e, I've decided to paste the
whole man page into Google docs and check what other issues it would
spot.
I know, it sounds silly, but I've just spent the last hour and a half
going over each one of them and fixing everything which was a true
finding and non-controversial.
Stefan Eissing [Wed, 28 May 2025 11:27:19 +0000 (13:27 +0200)]
http: fail early when rewind of input failed when following redirects
When inspecting a possible follow HTTP request, the result of a rewind
of the upload data was ignored as it was not clear at that point in time
if the request would become a GET.
This initiated the followup, rewound again, which failed again and
terminated the follow up.
This was confusing to users as it was not clear of the follow up was
done or not.
Fix: fail the early rewind when the request is not converted to GET.
Daniel Stenberg [Sun, 25 May 2025 10:24:49 +0000 (12:24 +0200)]
tool_getparam: refactored, simplified
- add dedicated option functions for bools/none/strings
- simplify clearing (some) arguments, use '*' instead of ' '
- scripts/top-complexity: remove getparameter from whitelist
- handle --help separately
- getstr and getstrn do not allow a NULL input
- improve the ;auto check, it needs to be trailing
- add toggle bit helper function
- unify an error message for bad --no- use
- introduce generic handling of deprecated options: ARG_DEPR
- handle --no- prefix on more booleans:
Samuel Henrique [Mon, 26 May 2025 21:46:24 +0000 (22:46 +0100)]
wcurl: update to v2025.05.26
This release fixes a small issue with the retry strategy:
* Increase number of retries to 5 (32 sec total time), fixing the
problem with misleading output. Previously, it was showing a higher
number of retries than what would be done and it always did only 3.
Kai Pastor [Sun, 25 May 2025 19:07:46 +0000 (21:07 +0200)]
asyn-ares: remove obsolete reference to local_ip6
7bf576064c moved local_ip6 from the parameter list to the actual
implementation of Curl_async_ares_set_dns_local_ip6. The no-op code for
!( defined(HAVE_CARES_SET_LOCAL) && defined(USE_IPV6) ) still had an
reference which is removed by this change.
Stefan Eissing [Fri, 23 May 2025 14:06:57 +0000 (16:06 +0200)]
ftp: fix race in upload handling
When TYPE was skipped for an immediate STORE command and the server
replied fast and the EPRT data connection was not ready, the transfer
was not initated, leading to no upload.
Fixes #17394
Closes #17428 Reported-by: JoelAtWisetech on github