Dan Fandrich [Fri, 23 Aug 2024 23:09:17 +0000 (16:09 -0700)]
tests: tweak use of impacket in smbserver
The missing message only uses a single line now and has a consistent
prefix making it easier for log parsers to skip. Remove a precheck test
looking for impacket in test1451 because it's incomplete, unnecessary
and in the wrong layer; the test harness will automatically discover
that the smbserver won't start and the test was only checking a single
dependency. Also update the tests README to no longer mention Python 2.
The servers were switched to Python 3 several years ago and no attempt
is being made to maintain Python 2 compatibility
Viktor Szakats [Thu, 22 Aug 2024 07:10:07 +0000 (09:10 +0200)]
cmake: sync code between test/example targets
- reuse local variable names.
- sync `PROJECT_LABEL`, add where missing.
- namespace all target names.
- bind header directories to each target.
- tests/server: limit `CURL_STATICLIB` to Windows (as in autotools.)
- drop functions with a single caller.
Dan Fandrich [Fri, 23 Aug 2024 04:24:03 +0000 (21:24 -0700)]
CI: consolidate workflows for source and docs check
A bunch of tiny jobs that run various source or documentation checks are
consolidated into two workflow files: checksrc.yml and checkdocs.yml.
This reduces the proliferation of new files containing one-line checks
and brings those that operate similarly together for better reasoning
about them. The man-examples check is also now running again for the
first time in 7 months.
Various calls to find, xargs and git ls-files are changed where possible
to use NUL line terminators in pipes to avoid issues with oddly-named
files that might find their way into the repo.
Daniel Stenberg [Fri, 23 Aug 2024 08:41:26 +0000 (10:41 +0200)]
urlapi: verify URL *decoded* hostname when set
It was previously wrongly verifying the input in its URL encoded format
when setting the hostname component with curl_url_set(), so it wrongly
rejected '%'.
Now it URL decodes the name appropriately before the check.
Added tests to lib1560 to verify that a fine %-code is okay and that a
bad %-code (that decodes to '%') is rejected.
Viktor Szakats [Thu, 22 Aug 2024 15:05:54 +0000 (17:05 +0200)]
cmake: respect cflags/libdirs of native pkg-config detections
In Find modules with native pkg-config detection (libgsasl, libidn2,
libssh, libuv, nettle) use the C compiler flags returned by pkg-config.
Also use the library paths, and return the pathless library names.
Also:
- add these library paths to `libcurl.pc`/`curl-config`.
- fix libgsasl detection to use the detected header directory.
Viktor Szakats [Thu, 22 Aug 2024 14:14:44 +0000 (16:14 +0200)]
cmake/FindGSS: bring closer to other Find modules
- add pkg-config-specific 'Found' message to the Find module.
- update non-pkg-config 'Found' message to show the flavour instead of
the library name. (= the first value listed after `REQUIRED_VARS`)
- delete extra 'Found' message from `CMakeLists.txt`.
- rename internal result variables to match with default pkg-config
names, in preparation of introducing them in other Find modules:
- `GSS_LINK_DIRECTORIES` -> `GSS_LIBRARY_DIRS`
- `GSS_LINKER_FLAGS` -> `GSS_LDFLAGS`
- `GSS_COMPILER_FLAGS` -> `GSS_CFLAGS`
Daniel Stenberg [Wed, 21 Aug 2024 12:58:57 +0000 (14:58 +0200)]
test1521: verify setting options to NULL better
Previously this test allowed several error values when setting options.
This made this test miss #14629.
Now, errors are generally not accepted for setopts:
- numerical setopts accept CURLE_BAD_FUNCTION_ARGUMENT for funny input
- the first setopt to an option accepts CURLE_NOT_BUILT_IN or
CURLE_UNKNOWN_OPTION for when they are disabled/not built-in
- there is an allowlist concept for some return code for some variables,
managed at the top of the mk-lib1521.pl script
In curl.h: remove the OBSOLETE named values from the setopt list.
Chris Swan [Wed, 21 Aug 2024 13:26:59 +0000 (14:26 +0100)]
docs: Clarify OpenSSF Best Practices vs Scorecard
SECURITY.md has a recently added section titled OpenSSF Scorecard
that actually documents OpenSSF Best Practices. Scorecard [0] is a
different OpenSSF project, that incorporates Best Practices, but is
distinct in its objectives and how it achieves them.
This change clarifies the terminology, and also removes any
implication that Gold Best Practices is an award rather than a self
certification programme.
As curl was a leader in implementing Best Practices some folk may be
more familiar with the earlier Core Infrastructure Initiative (CII)
naming, so a reference to that has been added.
[0] https://scorecard.dev/
Signed-off-by: Chris Swan <478926+cpswan@users.noreply.github.com>
Ref: #14319
Closes #14635
Jan Venekamp [Wed, 21 Aug 2024 17:54:19 +0000 (19:54 +0200)]
tests: constrain http pytest to tests/http directory
Running the http pytest had to be done from tests directory or above,
because the repeat argument fixture was defined in tests/conftest.py.
However, the repeat argument is not needed because its functionality
can be provided by pytest-repeat as documented in the test's
README.md. So, removed the pytest_addoption function for the repeat
argument and the pytest_report_header function is moved to
tests/http/conftest.py.
TODO: Remove repeat argument from all tests. As a stopgap, a
one-element list is defined for it for now.
Viktor Szakats [Wed, 21 Aug 2024 22:21:51 +0000 (00:21 +0200)]
build: make `CURL_FORMAT_CURL_OFF_T[U]` work with mingw-w64 <=7.0.0
Add tweak for mingw-w64 when building tests/http/client programs to
avoid a bogus `-Wformat` warning when using mingw-w64 v7.0.0 or older.
The warning is bogus because these programs use curl's `printf()`
implementation that is guaranteed to support that format spec.
Add this for both CMake and autotools. (But only CMake is CI tested with
an old toolchain.)
Apply the workaround to `docs/examples`, and fix an example to use
curl's `printf()` with `CURL_FORMAT_CURL_OFF_T`.
Reintroduce curl `printf()` calls into `tests/http/client`, via #14625.
Also restore large number masks to a printf, changed earlier in #14382.
Viktor Szakats [Wed, 21 Aug 2024 11:36:29 +0000 (13:36 +0200)]
src: fix potential macro confusion in cmake unity builds
Sources used `lib/curlx.h` with both `ENABLE_CURLX_PRINTF` set and unset
before including it.
In a cmake "unity" batch where the first included source had it unset,
the next sources did not get the macros requested with
`ENABLE_CURLX_PRINTF` because `lib/curl.x` had already been included
without them.
Fix it by by making the macros enabled permanently and globally for
internal sources, and dropping `ENABLE_CURLX_PRINTF`.
This came up while testing unity builds with smaller batches. The full,
default unity build where all `src` is bundled up in a single unit, was
not affected.
Fixes:
```
$ cmake -B build -DCMAKE_UNITY_BUILD=ON -DCMAKE_UNITY_BUILD_BATCH_SIZE=15
$ make -C build
...
curl/src/tool_getparam.c: In function ‘getparameter’:
curl/src/tool_getparam.c:2409:11: error: implicit declaration of function ‘msnprintf’; did you mean ‘vsnprintf’? [-Wimplicit-function-declaration]
2409 | msnprintf(buffer, sizeof(buffer), "%" CURL_FORMAT_CURL_OFF_T "-",
| ^~~~~~~~~
| vsnprintf
curl/src/tool_getparam.c:2409:11: warning: nested extern declaration of ‘msnprintf’ [-Wnested-externs]
[...]
```
Reported-by: Daniel Stenberg
Bug: https://github.com/curl/curl/pull/14626#issuecomment-2301663491
Daniel Stenberg [Wed, 21 Aug 2024 21:35:31 +0000 (23:35 +0200)]
CURLOPT_XFERINFOFUNCTION: clarify the callback return codes
also done in CURLOPT_PROGRESSFUNCTION.md
This changes the wording to say that 1 should be returned instad of
non-zero to return error from the callback. I did this to simplify, even
if other non-zero values still do the same thing I figure we might just
as well just leave out the others from the documentation.
Fixes #14627 Reported-by: Benjamin Riefenstahl Mecom
Closes #14637
This value tells how many sources files to bundle in a single "unity"
compilation unit.
The CMake default is 8 sources, curl's CMake set this to 0, meaning
to bundle all sources into a single unit.
This patch makes it possible to override the 0 value, and potentially
optimize the build process further by better utilizing multiple cores
in conjunction with `make -jN`.
The number of sources in lib is 172 at the time of writing this. For
a 12-core CPU, this can give a job for them all:
`-DCMAKE_UNITY_BUILD_BATCH_SIZE=15`
(Compile time may be affected by a bunch of other factors.)
Viktor Szakats [Tue, 20 Aug 2024 13:44:28 +0000 (15:44 +0200)]
cmake: drop libssh CONFIG-style detection
Drop `find_package(libssh CONFIG)` detection method in favour of
the Find module that supports both `pkg-config`, and CMake-native
(since #14555) detection.
This aligns `libssh` detection with other dependencies. It makes the
build honor custom configuration via `LIBSSH_INCLUDE_DIR`,
`LIBSSH_LIBRARY`.
Also enable libssh in a GHA/macos cmake job for build coverage.
Fixing:
- curl-for-win requiring a hack to configure libssh:
https://github.com/curl/curl-for-win/blob/4f9acbed92fd4aac0e874c9a591bec7d621cd9f2/curl.sh#L255-L263
- after #14555, GHA/windows gnutls vcpkg job no longer auto-detected
libssh, due to a regression missing to enable libssh when
found via `find_package(libssh CONFIG)`.
Ref: https://github.com/curl/curl/actions/runs/10470138955/job/28994650338
Viktor Szakats [Tue, 20 Aug 2024 19:51:44 +0000 (21:51 +0200)]
version: fix shadowing a `libssh.h` symbol
```
/Users/runner/work/curl/curl/lib/version.c: In function 'curl_version_info':
/Users/runner/work/curl/curl/lib/version.c:584:15: error: declaration of 'ssh_buffer' shadows a global declaration [-Werror=shadow]
584 | static char ssh_buffer[80];
| ^~~~~~~~~~
In file included from /Users/runner/work/curl/curl/lib/vssh/ssh.h:35,
from /Users/runner/work/curl/curl/lib/urldata.h:185,
from /Users/runner/work/curl/curl/lib/altsvc.c:32,
from /Users/runner/work/curl/curl/bld/lib/CMakeFiles/libcurl_shared.dir/Unity/unity_0_c.c:4:
/opt/homebrew/include/libssh/libssh.h:99:35: note: shadowed declaration is here
99 | typedef struct ssh_buffer_struct* ssh_buffer;
| ^~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/10477958747/job/29020250670#step:9:48
For libssh, it fixes a "unity" build issue where libssh deprecation
warnings were not suppressed before this patch, because the suppression
macro was only set before just one of the two `libssh.h` includes.
If the other was compiled first in unity mode, the warnings appeared.
Seen in local curl-for-win build (`CW_CONFIG=test-x64-libssh-quictls`)
with libssh 0.11.0. (Also in a GHA/macos cmake job upcoming in #14614)
Use this opportunity to drop duplicate SSH header includes from the SSH
modules. It's enough to include them via the common `ssh.h` header.
Dan Fandrich [Tue, 20 Aug 2024 17:56:59 +0000 (10:56 -0700)]
build: improve compiler version detection portability
POSIX sed doesn't support extended regular expressions, so convert a
call to the basic format. This caused a problem on AIX. Also, use the
detected sed binary name instead of hard-coding one.
Jan Venekamp [Tue, 20 Aug 2024 00:53:19 +0000 (02:53 +0200)]
tests: improve test_17_07_ssl_ciphers
Change TLS proto version on the test httpd server to test setting
combinations of --tls13-ciphers and --ciphers.
To not let the changed config of the httpd server bleed into the next
test, clean and reload on each test. Because a reload is slow, only
do this if the config is different than the loaded config. For this
the httpd.reload_if_config_changed() method is added.
Overloading of autouse fixtures does not seem to work. For the test
httpd server to be reloaded with a clean config in test_18_methods,
to not be affected by the config changes in test_17_ssl_use, the two
class scope fixtures of test_18_methods are now combined.
Jan Venekamp [Mon, 19 Aug 2024 01:59:08 +0000 (03:59 +0200)]
mbedtls: no longer use MBEDTLS_SSL_VERIFY_OPTIONAL
With mbedTLS if the minimum version of TLS is set to 1.3,
MBEDTLS_SSL_VERIFY_OPTIONAL is not available in client mode. See:
https://github.com/Mbed-TLS/mbedtls/blob/2ca6c285/library/ssl_tls.c#L1357
Also, there might be plans to remove it completely in future mbedTLS
versions.
Switch to always use MBEDTLS_SSL_VERIFY_REQUIRED. If verifypeer or
verifyhost are disabled the corresponding error flags are cleared in the
verify callback function. That is also where verification errors are
logged.
Viktor Szakats [Mon, 19 Aug 2024 12:27:51 +0000 (14:27 +0200)]
cmake: migrate dependency detections to Find modules
For: libgsasl, libidn2, libssh, libuv.
The new Find modules retain using `pkg-config` natively, not as a "hint"
for the CMake-native detection. Of the pre-existing Find modules, only
FindNettle, and FindGSS (with customized code) work this way. Align
detection code for the new modules and add version detection for the
CMake-native paths.
Also, add CMake-native detection for `libgsasl`.
The remaining outlier in `CMakeLists.txt` is GnuTLS, which has
a CMake built-in Find module, but which lacks `pkg-config` support,
required for vcpkg. It remains unchanged.
Another part-outlier is `libssh`, which keeps requiring the trick
`find_package(libssh CONFIG QUIET)` for reasons I could not yet figure
out.
Viktor Szakats [Mon, 19 Aug 2024 23:13:14 +0000 (01:13 +0200)]
cmake: add `find_package()` missing from `USE_MSH3` option
The original patch added the Find module and CMake option. But the logic
missed a `find_package(MSH3)` call to use that Find module, leaving the
referenced `MSH3_INCLUDE_DIRS`, `MSH3_LIBRARIES` variables undefined.
Viktor Szakats [Fri, 16 Aug 2024 23:08:45 +0000 (01:08 +0200)]
cmake: limit `pkg-config` to UNIX and MSVC+vcpkg by default
Limits `pkg-config` to UNIX and MSVC with vcpkg, by default. Compared to
curl 8.9.1, this unlocks `pkg-config` on MSVC with vcpkg.
This condition might be updated in the future depending on where
`pkg-config` can be useful without breaking things. (e.g. to non-cross
MINGW, or all MINGW).
In the meantime everyone is free to override the default and test their
build with `pkg-config` by setting the `CURL_USE_PKGCONFIG=ON` CMake
option.
Our convention for naming Find modules (the part after the `Find`
prefix, also called as 'package name') is:
Always start with uppercase. Follow with lowercase, unless there is
a clear preference for a stylized name. E.g. the project itself uses it
that way with a matching `<Name>Config.cmake` file, or we use it that
way elsewhere, or the name is an acronym.
Viktor Szakats [Mon, 19 Aug 2024 10:31:11 +0000 (12:31 +0200)]
cmake: fix Find module and package names
- fix BearSSL warning about name mismatch.
- fix Nettle Find module not found on Linux.
- tidy-up: drop quotes from a package name.
Package names must match case-sensitively to work on all platforms:
- `find_package(<NAME> ...)` in `CMakeLists.txt`.
- `CMake/Find<NAME>.cmake` filenames.
- `find_package_handle_standard_args(<NAME> ...` in Find modules.
- `message(STATUS "Found <NAME> ...` in Find modules.
(to match the message shown by `find_package_handle_standard_args()`)
Daniel Stenberg [Mon, 19 Aug 2024 12:17:55 +0000 (14:17 +0200)]
build: use -Wno-format-overflow
-Wformat-overflow is not a warning that we want enabled as it does not
help us. It can only bring us false positives since it warns on bad uses
of sprintf and vsprintf ("that might overflow the destination buffer").
Two functions we explicitly ban in curl code.
The only way this flag triggers warnings in curl code is false positives
for functions we have marked with the CURL_PRINTF() macro.
Further: it seems -Wformat-trunaction option might in turn also enable
-Wformat-overflow, so if this second option is used, we need to
explicitly set -Wno-format-overflow - not just skip setting
-Wformat-overflow.
Reported-by: Viktor Szakats
Fixes #14168
Closes #14598
Viktor Szakats [Sat, 17 Aug 2024 22:10:39 +0000 (00:10 +0200)]
cmake/FindNettle: skip `pkg-config` for custom configs
If either `NETTLE_INCLUDE_DIR` or `NETTLE_LIBRARY` is set to customize
the `nettle` dependency, skip `pkg-config` and use the CMake-native
detection to honor these custom settings.
Viktor Szakats [Sat, 17 Aug 2024 21:39:49 +0000 (23:39 +0200)]
cmake: tidy up more in Find modules
- add `NAMES` where missing.
- document input variables (including deprecated ones.)
- comment cleanups.
- FindWolfSSL: drop stray `QUIET` from `pkg_check_modules()`.
(`QUIET` may be re-added for all modules in the future.)
Viktor Szakats [Sun, 18 Aug 2024 07:26:22 +0000 (09:26 +0200)]
appveyor: drop uploading artifacts
Uploading artifacts sometimes results in this error:
```
Uploading artifacts...
[1/1] _bld\src\curl.exe (2,022,912 bytes)...100%
Error uploading artifact to the storage: Remote server returned 503: Service Temporarily Unavailable
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/50424126/job/e4envval6xkicv1i#L123
The artifacts are also probably not useful to upload for every run. Also
note that they were missing external DLL dependencies.
Leave the logic there commented, to make it easy to enable as needed for
debugging or testing artifacts locally.
- move `m` library detection to wolfSSL Find module.
`m` is necessary for wolfSSL (wolfcrypt) library functions called by
`libngtcp2_crypto_wolfssl`.
Follow-up to 8577f4ca084b8a3926b869a48a29d41a810eceb5 #14343
- fix comment header about supported `COMPONENT` names.
Justin Maggard [Mon, 5 Aug 2024 17:21:35 +0000 (10:21 -0700)]
mbedtls: add more informative logging
After TLS handshare, indicate which TLS version was negotiated in
addition to the cipher in the handshake completed log message.
Also use the verify callback for certificate logging and collection.
This allows things to work even when MBEDTLS_SSL_KEEP_PEER_CERTIFICATE
is disabled in the mbedtls library.
And lastly, catch certificate validation errors later so we can give the
user more informative error messages that indicate what the failure was
from certificate validation.
Tested on both current LTS versions (2.28 and 3.6).
Stefan Eissing [Thu, 15 Aug 2024 11:16:53 +0000 (13:16 +0200)]
transfer: Curl_sendrecv() and event related improvements
- Renames Curl_readwrite() to Curl_sendrecv() to reflect that it
is mainly about talking to the server, not reads or writes to the
client. Add a `nowp` parameter since the single caller already
has this.
- Curl_sendrecv() now runs all possible operations whenever it is
called and either it had been polling sockets or the 'select_bits'
are set.
POLL_IN/POLL_OUT are not always directly related to send/recv
operations. Filters like HTTP/2, QUIC or TLS may monitor reverse
directions. If a transfer does not want to send (KEEP_SEND), it
will not do so, as before. Same for receives.
- Curl_update_timer() now checks the absolute timestamp of an expiry
and the last/new timeout to determine if the application needs
to stop/start/restart its timer. This fixes edge cases where
updates did not happen as they should have.
- improved --test-event curl_easy_perform() simulation to handle
situations where no sockets are registered but a timeout is
in place.
- fixed bug in events_socket() that complained about removing
a socket that was unknown, when indeed it had removed the socket
just before, only it was the last in the list
- fixed conncache's internal handle to carry the multi instance
(where the cache has one) so that operations on the closure handle
trigger event callbacks correctly.
- fixed conncache to not POLL_REMOVE a socket twice when a conneciton
was closed.
Viktor Szakats [Fri, 16 Aug 2024 13:04:25 +0000 (15:04 +0200)]
cmake: sync up version detection in Find modules
- use the same pattern across all Find modules:
- verify if the version header exists before reading it.
- use a single regex per lookup.
- sync regexes between Find modules.
- use generic temporary variable names.
- improve readability.
- make it simpler to transition to new CMake syntax in the future:
```cmake
file(STRINGS "${CARES_INCLUDE_DIR}/ares_version.h" _version_str REGEX "<...>")
unset(_version_str)
set(CARES_VERSION "${CMAKE_MATCH_1}")
```
Ref: https://cmake.org/cmake/help/latest/policy/CMP0159.html#policy:CMP0159
- fix zstd version detection to be CMake 3.7 compatible.
Required 3.9 before this patch, for the `CMAKE_MATCH_<n>` feature.
Follow-up to c5d506e9bbf0669d7605d1d00865ae7e229b2409 #12200
Viktor Szakats [Thu, 15 Aug 2024 08:47:54 +0000 (10:47 +0200)]
cmake: tidy-up continues
- move variable dump to a GHA foldable group.
- minimize scope for an include().
- rename `HIDES_CURL_PRIVATE_SYMBOLS` to `CURL_HIDES_PRIVATE_SYMBOLS`,
to keep it in the curl namespace.
- drop quotes from a version number.
- add missing `Makefile.inc` var refs to comment.
- FindNGTCP2: rename internal var to underscore/lowercase.
- FindBearSSL, FindGSS: whitespace.