Aquila Macedo [Fri, 7 Mar 2025 21:40:34 +0000 (18:40 -0300)]
runtests: add feature-based filtering
This commit introduces support for features in the test selection
process by adding them to the keywords list with the `feat:` prefix. It
allows users to specify features to run only tests with them, or exclude
tests using `!feat:<feature>`, similar to how keywords are handled.
Viktor Szakats [Mon, 17 Mar 2025 13:05:12 +0000 (14:05 +0100)]
GHA/windows: skip flaky test 498
On suspect of strain on the runtime env/pipes, disable this test, which
is flaky due to `runtests` detecting a 2009 result code from curl, while
curl is returning the expected 56:
```
test 0498...[Reject too large HTTP response headers on endless redirects]
curl returned 2009, when expecting 56
498: exit FAILED
== Contents of files in the log/5/ dir after test 498
[...]
0 0 0 0 0 0 0 0 --:--:-- 0:00:10 --:--:-- 0
curl: (56) Too large response headers: 6144086 > 6144000
```
In such cases the number of log lines for this single test is 4800. In
comparison the total number of log lines for a clear test run is 3800.
These attributes were causing unexplained warnings while playing with
PR #16738: In `CURLDEBUG` builds with mingw, gcc (14.2.0), and `-O3`,
while building `libcurlu`. `-O3` is required. May be related to having
the `CURLDEBUG` allocators in the same source file as their callers
(unity mode). PR #16738 moves `memdebug.c` into the main unity unit.
Unclear why it doesn't affect `libcurl`.
E.g. CI job `mingw, CM ucrt-x86_64 schannel R TrackMemory` CI job:
https://github.com/curl/curl/actions/runs/13888662354/job/38856868429
Viktor Szakats [Sun, 8 Sep 2024 22:33:04 +0000 (00:33 +0200)]
tests/server: support bundle binary
Extend existing `--enable-test-bundles` and `-DCURL_TEST_BUNDLES=ON`
options to also bundle test server programs into a single binary. With
autotools, also bundle auxiliary libcurl sources for a "unity"-style
build.
It saves almost 10 minutes per run, across all CI jobs.
On average it makes `build tests` steps 25% faster.
With CMake, it brings down `testdeps` build steps to 32 to 37, from
45 to 64 before this patch, with unity. Without unity it brings it down
from 400-420 to 280-300. For comparison, without unity and bundles,
the number of build steps is around 1850.
With autotools the gain is possibly larger because this patch does unity
and bundle for test servers.
The total reduction of build steps / log lines is 12000. It's 44% of
reduction on average across all CI jobs.
Viktor Szakats [Sat, 15 Mar 2025 11:03:56 +0000 (12:03 +0100)]
GHA/windows: unignore 2310, disable SCP/FTP for vcpkg `libssh2[core]`
Skipping these tests saves time and reduces test logs from 11500 lines
to 3800.
Tests are permanently broken due to `curl: (67) Authentication failure`.
This libssh2 is built with WinCNG. Builds using libcrypto from OpenSSL
work fine.
Viktor Szakats [Sat, 15 Mar 2025 01:39:05 +0000 (02:39 +0100)]
asyn-thread: drop `free()` on non-heap address
seen with mingw-w64 gcc 14.2.0 while playing with other modifications:
```
lib/asyn-thread.c: In function 'init_resolve_thread':
lib/asyn-thread.c:447:5: warning: 'free' called on pointer 'data' with nonzero offset 3264 [-Wfree-nonheap-object]
447 | free(td);
| ^~~~~~~~
```
Where `td` is:
```c
struct thread_data *td = &data->state.async.thdata;
```
Daniel Stenberg [Fri, 14 Mar 2025 16:30:09 +0000 (17:30 +0100)]
http: remove a HTTP method size restriction
By allocating the method string as part of the struct, the previous
fixed size limit (23 bytes) can be avoided. It would previously make
"curl -X [long string]" work against http://localhost but fail against
https://curl.se with no clear error message.
Daniel Stenberg [Fri, 14 Mar 2025 15:57:33 +0000 (16:57 +0100)]
http: remove the space requirement after status-code
It was introduced in 294136b75411893c5, but not shipped in a release. It
has caused problems and after checking, the browsers don't insist on it
even if RFC 9112 says it is mandatory.
> I have some tooling leftover from when I was implementing CRL support
> in the webpki crate that downloaded every CRL I could find referenced
> in ccadb (without doing any special filtering for defunct CAs/CRLs
> mind you) and found CRLs that spanned the range from very small
> (<1mb), to medium sized (11 .. 22mb) to very large (100mb).
Viktor Szakats [Fri, 14 Mar 2025 02:37:37 +0000 (03:37 +0100)]
autotools: use `CURLDEBUG` to exclude TrackMemory code from unity
Before this patch, autotools builds excluded TrackMemory sources
(`lib/memdebug.c` and `lib/curl_multibyte.c`) based on the `DEBUGBUILD`
setting. This works in most cases because its value is the same as
`CURLDEBUG` by default, but the correct condition is `CURLDEBUG`.
It should fix `--disable-debug --enable-curldebug --enable-unity`
builds. (not tested in CI)
Viktor Szakats [Wed, 12 Mar 2025 21:41:57 +0000 (22:41 +0100)]
build: do not apply curl debug macros to `tests/server` by default
It seems unnecessary and possibly unexpected to build test servers with
debug-enabled features and memory tracking whenever the tested curl is
built like that (which is a requirement for some tests, so curl is
mostly built like that when running tests.) It also makes building
servers a little bit faster with cmake for the most common cases.
You can apply debug options to `tests/server` with these new options:
- `./configure`: `--enable-server-debug`.
- cmake: `-DENABLE_SERVER_DEBUG`.
Also sync the way we pass these macros in autotools, with CMake builds.
Before this patch, autotools passed them via `curl_config.h`. After this
patch it passes them on the command-line, like cmake builds do.
This patch also make these option no longer passed to examples and
`http/client` in cmake builds, where they were no-ops anyway.
Viktor Szakats [Wed, 12 Mar 2025 21:35:23 +0000 (22:35 +0100)]
tests/server: give global `path` variable a more descriptive name
Use a more descriptive global variable name in server code, also
to avoid colliding with this name used elsewhere in libcurl.
This isn't causing an issue at this time, but makes the code prone
to `-Wshadow` warnings in unity mode, if the global variable is
compiled first. This specific variable could collide with the `path`
argument of the `curlx_win32_stat()` function.
Viktor Szakats [Thu, 13 Mar 2025 22:53:40 +0000 (23:53 +0100)]
autotools: fix `dllmain.c` in unity builds
Sync it with cmake to:
- exclude it from all builds except Windows and Cygwin.
- exclude it from unity builds for Cygwin to avoid the included
`windows.h` header interfere with the rest of the code.
Also:
- fix to trim ending spaces from `CSOURCES` for the `tidy` target.
The solution requires a non-POSIX `-E` `sed` option. Supported by BSD
and GNU implementations.
Follow-up to 37523c91bc418fc734ee54f329677dc7123eb465 #16480
Daniel Stenberg [Thu, 13 Mar 2025 22:28:42 +0000 (23:28 +0100)]
doh: remove wrong but unreachable exit path from doh_decode_rdata_name
The condition could not happen, as the function is only called from a
single place where the caller already made sure it can't happen. This
change still removes the flawed logic.
Viktor Szakats [Thu, 13 Mar 2025 02:06:09 +0000 (03:06 +0100)]
GHA/windows: replace OpenSSH-Windows-Prelease job with standard openssh
After restricting OpenSSH-Windows to a single job, and bumping it to
the pre-release version, that job started hanging then timing out with
reasonable consistency.
Since we saw similar hangs before with OpenSSH-Windows stable, in all
jobs, drop OpenSSH-Windows from CI, and replace it with MSYS openssh.
After this patch, all Windows jobs use MSYS2 or Cygwin openssh.
Viktor Szakats [Thu, 13 Mar 2025 02:15:46 +0000 (03:15 +0100)]
resolve: fix building without Unix sockets and `CURLDEBUG`
```
In file included from server_bundle.c:7:
../../../tests/server/resolve.c:110:5: error: unknown type name 'curl_socket_t'; did you mean 'curl_socklen_t'?
curl_socket_t s = socket(PF_INET6, SOCK_DGRAM, 0);
^~~~~~~~~~~~~
curl_socklen_t
../../../include/curl/system.h:392:38: note: 'curl_socklen_t' declared here
typedef CURL_TYPEOF_CURL_SOCKLEN_T curl_socklen_t;
^
In file included from server_bundle.c:7:
../../../tests/server/resolve.c:111:13: error: use of undeclared identifier 'CURL_SOCKET_BAD'
if(s == CURL_SOCKET_BAD)
^
```
Ref: https://github.com/curl/curl/actions/runs/13825438937/job/38679418428?pr=15000#step:14:47
Viktor Szakats [Tue, 11 Mar 2025 18:29:14 +0000 (19:29 +0100)]
tests/server: do not redefine standard functions in `sockfilt`
Use a namespaced macro instead. To avoid confusion when other headers
also redefine these functions. And to improve readability by making it
apparent that the code sometimes overrides these functions.
Viktor Szakats [Wed, 12 Mar 2025 21:59:53 +0000 (22:59 +0100)]
memdebug.h: avoid `-Wredundant-decls` with an extra guard
Add an extra guard for the function and variable declarations to avoid
redundant redeclaration warnings when including this header multiple
times. This can happen in unity builds when including it again after
`curl_memory.h`.
Fixes:
```
bld/tests/server/CMakeFiles/servers.dir/Unity/unity_0_c.c
In file included from lib/mprintf.c:32,
from bld/tests/server/CMakeFiles/servers.dir/Unity/unity_0_c.c:7:
lib/memdebug.h:52:14: error: redundant redeclaration of ‘curl_dbg_logfile’ [-Werror=redundant-decls]
52 | extern FILE *curl_dbg_logfile;
| ^~~~~~~~~~~~~~~~
In file included from tests/server/resolve.c:50,
from bld/tests/server/server_bundle.c:7,
from bld/tests/server/CMakeFiles/servers.dir/Unity/unity_0_c.c:4:
lib/memdebug.h:52:14: note: previous declaration of ‘curl_dbg_logfile’ with type ‘FILE *’
52 | extern FILE *curl_dbg_logfile;
| ^~~~~~~~~~~~~~~~
[...]
lib/memdebug.h:110:17: error: redundant redeclaration of ‘curl_dbg_fclose’ [-Werror=redundant-decls]
110 | CURL_EXTERN int curl_dbg_fclose(FILE *file, int line, const char *source);
| ^~~~~~~~~~~~~~~
lib/memdebug.h:110:17: note: previous declaration of ‘curl_dbg_fclose’ with type ‘int(FILE *, int, const char *)’
110 | CURL_EXTERN int curl_dbg_fclose(FILE *file, int line, const char *source);
| ^~~~~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/13822010778/job/38669360980#step:39:55
Viktor Szakats [Thu, 13 Mar 2025 00:20:52 +0000 (01:20 +0100)]
cmake: unity mode optimization for non-`CURLDEBUG` `testdeps` targets
Include more sources in unity mode to optimize libtest and tests/server
builds for non-debug-enabled builds, syncing this pattern with `lib` and
`src`.
It reduces build steps from 62 to 47 (-14, -24%) with test bundles.
Without test bundles, from 680 to 642 (-38, -6%).
Daniel Stenberg [Thu, 13 Mar 2025 07:22:11 +0000 (08:22 +0100)]
cookie: accept "high byte" cookie content
Regression from 597ee915c4c6da132dbd1735345 (not shipped in a release) Reported-by: Carlos Henrique Lima Melara Assisted-by: Scott Talbert
Added such a cookie to test 31.
Viktor Szakats [Sat, 8 Mar 2025 01:18:35 +0000 (02:18 +0100)]
core: stop redefining `E*` macros on Windows, map `EACCES`, related fixes
Before this patch, standard `E*` errno codes were redefined on Windows,
onto matching winsock2 `WSA*` error codes, which have different values.
This broke uses where using the `E*` value in non-socket context, or
other places expecting a POSIX `errno`, e.g. file I/O, threads, IDN or
interfacing with dependencies.
Fix it by introducing a curl-specific `SOCKE*` set of macros that map to
`WSA*` on Windows and standard POSIX codes on other platforms. Then
verify and update the code to use `SOCKE*` or `E*` macro depending on
context.
- Add `SOCKE*` macros that map to either winsock2 or POSIX error codes.
And use them with `SOCKERRNO` or in contexts requiring
platform-dependent socket error codes.
This fixes `E*` uses which were supposed be POSIX values, not `WSA*`
socket errors, on Windows:
- lib/curl_multibyte.c
- lib/curl_threads.c
- lib/idn.c
- lib/vtls/gtls.c
- lib/vtls/rustls.c
- src/tool_cb_wrt.c
- src/tool_dirhie.c
- Ban `E*` codes having a `SOCKE*` mapping, via checksrc. Authored-by: Daniel Stenberg
- Add exceptions for `E*` codes used in file I/O, or other contexts
requiring POSIX error codes.
Also:
- ftp: fix missing `SOCKEACCES` mapping for Windows.
- add `SOCKENOMEM` for `Curl_getaddrinfo()` via `asyn-thread.c`.
- tests/server/sockfilt: fix to set `SOCKERRNO` in local `select()`
override on Windows.
- lib/inet_ntop: fix to return `WSAEINVAL` on Windows, where `ENOSPC` is
used on other platforms. To simulate Windows' built-in `inet_ntop()`,
as tested on a Win10 machine.
Note:
- WINE returns `STATUS_INVALID_PARAMETER` = `0xC000000D`.
- Microsoft documentation says it returns `WSA_INVALID_PARAMETER`
(= `ERROR_INVALID_PARAMETER`) 87:
https://learn.microsoft.com/windows/win32/api/ws2tcpip/nf-ws2tcpip-inet_ntop#return-value
- lib/inet_ntop: drop redundant `CURL_SETERRNO(ENOSPC)`.
`inet_ntop4()` already sets it before returning `NULL`.
- replace stray `WSAEWOULDBLOCK` with `USE_WINSOCK` macro to detect
winsock2.
- move existing `SOCKE*` mappings from `tests/server` to
`curl_setup_once.h`.
- add missing `EINTR`, `EINVAL` constants for WinCE.
Daniel Stenberg [Wed, 12 Mar 2025 10:22:49 +0000 (11:22 +0100)]
multi: call protocol handler done() if PROTOCONNECT or later
The protocol handlers' done() function would previous get called
unconditionally in multi_done(), no matter how far the easy handle's
state machine has transitioned.
This caused problems in IMAP which in imap_connect() initializes things
that the imap_done() function assumes has occured. I think that seems
like a correct assumption and we should rather make sure that the done()
function is only called if we have reached the PROTOCONNECT state.
Stefan Eissing [Wed, 12 Mar 2025 10:31:21 +0000 (11:31 +0100)]
sendf: client reader line conversion: do not change data->state.infilesize
The line conversion reader, added in crfl and prefer_ascii mode was
incrementing data->state.infilesize for every line end converted. This
results in the wrong size to start a retry of an upload.
Eliminate the increment and check upload size in FTP less precise when
conversions are done.
Viktor Szakats [Tue, 11 Mar 2025 13:13:07 +0000 (14:13 +0100)]
GHA/windows: change openssh server, non-debug-enabled MSVC job, other improvements
MSVC:
- switch jobs to standard openssh server. Reduce exceptions.
- make the SCP/SFTP ignore list more specific and comment with details.
- keep using OpenSSH-Windows for the OpenSSL job, and bump to the
prerelease version.
- disable `ENABLE_DEBUG` for BoringSSL to have such build tested. (This
is the first Windows non-ENABLE_DEBUG build with test runs.)
Takeaways:
- test 612 broken on Windows.
- test 613 broken on Windows with the standard openssh server.
- test 614 broken with libssh and OpenSSH-Windows.
- test 3022 broken with libssh2 and OpenSSH-Windows.
- tests broken with OpenSSH-Windows:
601 603 617 619 621 641 665 2004.
- vcpkg `libssh2[core,zlib]` broken due to:
curl: (67) Authentication failure
MSVC prep steps:
- install base msys2 package to simplify configuration, align with other
jobs and allow to use msys2 packages for tests.
- add support for msys2 openssh server. Keep OpenSSH-Windows as per-job
option. Add support for OpenSSH prerelease versions.
Prerelease does not make a difference in test results, but, stable was
last updated in 2019 (v8.0.0.1) and it seems better to use maintained
release track, with its latest from April 2024 (v9.5.0).
https://community.chocolatey.org/packages/openssh/8.0.0.1
https://community.chocolatey.org/packages/openssh
https://github.com/PowerShell/Win32-OpenSSH
https://github.com/PowerShell/openssh-portable
- add 'libssh' to its job name.
- make `ENABLE_DEBUG` a per-job option.
msys/mingw:
- install `openssh` later and only when necessary.
- downgrade msys2 runtime later. (to follow other jobs)
- disable `CheckSpace` earlier. Also to untie it from the runtime
downgrade step, which we would hopefully drop.
Daniel Stenberg [Tue, 11 Mar 2025 09:34:47 +0000 (10:34 +0100)]
curl: add my_setopt_long() and _offt()
Two new dedicated functions for setting long and curl_off_t options with
curl_easy_setopt(). These make it easier to make sure we pass on the
right option (types) so that the --libcurl code also gets right.
Stefan Eissing [Tue, 11 Mar 2025 09:04:50 +0000 (10:04 +0100)]
wolfssh: fix freeing of resources in disconnect
ssh's disconnect assumed that the session to the server could be
shut down successfully during disconnect. When this failed, e.g.
timed out, memory was leaked.
Stefan Eissing [Mon, 10 Mar 2025 16:08:57 +0000 (17:08 +0100)]
libssh: fix freeing of resources in disconnect
ssh's disconnect assumed that the session to the server could be shut
down successfully during disconnect. When this failed, e.g. timed out,
memory was leaked.
Stefan Eissing [Mon, 10 Mar 2025 15:34:10 +0000 (16:34 +0100)]
libssh2: fix freeing of resources in disconnect
ssh's disconnect assumed that the session to the server could be shut
down successfully during disconnect. When this failed, e.g. timed out,
memory was leaked.
Viktor Szakats [Fri, 31 Jan 2025 22:20:46 +0000 (23:20 +0100)]
build: enable `-Wcast-qual`, fix or silence compiler warnings
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.
Viktor Szakats [Sat, 8 Mar 2025 22:32:10 +0000 (23:32 +0100)]
tidy-up: whitespace, tests/server Makefile.inc, timeval in VS project
- VS projects: drop unused `timediff`.
(used by curltool library, but this build method doesn't build that.)
- tests/server/sws: reflow an `if` for greppability.
- tests/server/Makefile.inc: indent, format
- tests/server/Makefile.inc: merge `USEFUL` and `UTIL` lists.
Viktor Szakats [Sun, 9 Mar 2025 22:12:10 +0000 (23:12 +0100)]
libssh2: fix memory leak in `SSH_SFTP_REALPATH` state
Seen in MSVC libssh2 CI job:
```
test 0615...[SFTP put remote failure]
test 0616...[SFTP retrieval of empty file]
test 0618...[SFTP retrieval of two files]
test 0620...[SFTP retrieval of missing file followed by good file]
test 0622...[SFTP put failure]
test 0637...[SFTP retrieval with invalid X- range]
test 0640...[SFTP --head retrieval]
** MEMORY FAILURE
Leak detected: memory still allocated: 22 bytes
At 2ae5b8a7ab8, there's 22 bytes.
allocated by D:/a/curl/curl/lib/vssh/libssh2.c:2006
```
https://github.com/curl/curl/actions/runs/13752652590/job/38455575042?pr=16636#step:14:3907
https://github.com/curl/curl/actions/runs/13752879003/job/38456075461
https://github.com/curl/curl/actions/runs/13753706458/job/38457888479
Bug: https://github.com/curl/curl/pull/16636#issuecomment-2709086838 Co-authored-by: Daniel Stenberg
Closes #16639
Viktor Szakats [Sun, 9 Mar 2025 12:14:31 +0000 (13:14 +0100)]
runtests: fix SSH server not starting in cases, re-ignore failing vcpkg CI jobs
Replace `Cwd::abs_path()` with `File::Spec->rel2abs()`. The former
requires the file to exist, but in some cases, it's missing.
Seen in MSVC vcpkg jobs using Chocolatey OpenSSH v8.0.0.1 ending up with
`$path=/d/a/curl/curl/bld/tests/log/3/server/ssh_server.pid`, which does
not exist while converting to an absolute path (the path is already
absolute, but the conversion is done unconditionally):
```
Use of uninitialized value in subroutine entry at D:/a/curl/curl/tests/pathhelp.pm line 128.
can't convert empty path at D:/a/curl/curl/tests/pathhelp.pm line 128.
```
Ref: https://github.com/curl/curl/actions/runs/13747741797/job/38444844173#step:14:1233 (master)
Ref: https://github.com/curl/curl/actions/runs/13751862952/job/38453816737#step:14:3185 (trace)
Also ignore 3 new libssh2 jobs failing due to memleak.
It fixes `wait_ms()` to check for, and return `SOCKERRNO`. Fixing error
handling on Windows.
Also:
- tests/server: change callers to check `SOCKERRNO`.
- `wait_ms()`: fix to check for the correct error code on Windows.
Pending for `Curl_wait_ms()`: #16621.
- `Curl_wait_ms()`: tidy-up `Sleep()` argument cast (nit).
- lib/curl_trc: drop an unused header.
Viktor Szakats [Sat, 8 Mar 2025 02:31:45 +0000 (03:31 +0100)]
GHA/windows: re-add GnuTLS for vcpkg, improve perf by building examples less
The GnuTLS MSVC/vcpkg build doesn't actually work on Windows. Let's
restore the build itself, to keep it fit for more testing. With disabled
tests (and examples) to keep it fast and not add to flakiness.
Also:
- enable GnuTLS in the MultiSSL job.
- limit building examples to one normal and one UWP job. It saves
6 x 1-1.5 minutes. Coverage remains the same, because example builds
only depend on the toolchain / target, not on the actual features
(except IPv6, but that's enabled for all.)
Viktor Szakats [Sat, 8 Mar 2025 03:25:03 +0000 (04:25 +0100)]
gtls: fix uninitialized variable
Fix uninitialized variable (warning by MSVC):
```
lib\vtls\gtls.c(1207,1): error C2220: the following warning is treated as an error
lib\vtls\gtls.c(1207,1): warning C4701: potentially uninitialized local variable 'result' used
```
Ref: https://github.com/curl/curl/actions/runs/13733139148/job/38413553053#step:9:31