Also enable `bugprone-suspicious-realloc-usage` clang-tidy option
to verify.
Fixing:
```
tests/server/rtspd.c:328:37: error: 'req->rtp_buffer' may be set to null if 'realloc' fails,
which may result in a leak of the original buffer
[bugprone-suspicious-realloc-usage,-warnings-as-errors]
328 | req->rtp_buffer = realloc(req->rtp_buffer,
| ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
```
Ref: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-realloc-usage.htmlCloses#20621
Seen with mbedTLS 4.0.0. mbedTLS 4.0.0 renamed `mbedcrypto` lib to
`tfpsacrypto`, while also keeping a copy under the old name to aid
transition. However, this compatibility logic is broken for MSVC static
builds, and the old name missing.
Work around by looking for the new name in the raw detection codepath.
Note that using `pkg-config`-based detection also works as a workaround.
Reported-by: tawmoto on github
Fixes#20616
Ref: https://github.com/Mbed-TLS/mbedtls/blob/v4.0.0/library/CMakeLists.txt#L275-L282
Ref: https://github.com/Mbed-TLS/mbedtls/issues/10605Closes#20617
Fix bigger and smaller kinks in how clang-tidy is configured and used.
Sync behavior more between autotools and cmake, lib/src and tests. Bump
clang-tidy minimum version and prepare logic to allow using clang-tidy
to a fuller extent.
- move clang-tidy settings from builds to a new `.clang-tidy.yml`.
To make it easy to see and edit checks at one place. Also to allow
using the `--checks=` option internally to silence tests-specific
checks. (clang-tidy does not support multiple `--check=` options via
the command-line.)
Use explicit `--config-file=` option to point to the configuration.
- .clang-tidy.yml: link to documentation.
- suppress `clang-diagnostic-nullability-extension` due to a false
positive in libtests with `CURL_WERROR=ON` and `PICKY_COMPILER=OFF`.
- .clang-tidy.yml: enable `portability-*`, `misc-const-correctness`.
- drop `--quiet` clang-tidy option by default to make its working a bit
more transparent. The extra output is minimial.
- consistently use double-dashes in clang-tidy command-line options.
Supported by clang-tidy 9.0.0+ (2019-09-19). Before this patch single
and double were used arbitrarily.
- src/tool_parsecfg: silence false positive `clang-analyzer-unix.Stream`.
Seen with clang 18 + clang-tidy 19 and 20 (only with autotools.)
- INTERNALS: require clang-tidy 14.0.0+. For the `--config-file` option.
- INTERNALS: recommend clang-tidy 19.1.0+, to avoid bogus
`clang-analyzer-valist.Uninitialized` warnings. (bug details below)
autotools:
- allow configuring the clang-tidy tool via `CLANG_TIDY` env.
Also to use in GHA to point to a suffixed clang-tody tool.
- fix to pass CFLAGS to lib, src sources.
(keep omitting them when using a non-clang compiler.)
- fix to pass `--warnings-as-errors=*` in quotes to avoid globbing.
cmake:
- fix to not pass an empty `-I` to clang-tidy.
- fix to pass CFLAGS (picky warnings) to clang-tidy for test sources.
(keep omitting them when using a non-clang compiler.)
- fix to disable `clang-diagnostic-unused-function` for test sources.
(tests have static entry points, which trigger this check when
checking them as individidual sources.)
- fix forwarding `CURL_CLANG_TIDYFLAGS` to clang-tidy.
- force disable picky warnings when running clang-tidy with a non-clang
compiler. To not pass these flags when checking lib and src.
CI:
- GHA/linux: avoid clang-tidy bug by upgrading to v19, and drop the
workaround.
- GHA/linux: switch to clang from gcc in the clang-tidy job. Using gcc
doesn't allow passing CFLAGS to clang-tidy, making it less effective.
(My guess this was one factor contributing to this job often missing
to find certain issues compared to GHA/macos.)
I recomment using clang-tidy with a clang compiler, preferably the same
version or one that's compatible. Other cases are best effort, and may
fail if a C flag is passed to clang-tidy that it does not understand.
Picky warnings are mostly omitted when using a non-clang compiler,
reducing its usefulness.
Details and reproducer for the v18 (and earlier) clang-tidy bug,
previously affecting the GHA/linux job:
clang-tidy <=18 emits false warnings way when passing multiple C sources
at once (as done with autotools):
```sh
cat > src1.c <<EOF
#include <string.h>
static void dummy(void *p) { memcmp(p, p, 0); }
EOF
cat > src2.c <<EOF
#include <stdarg.h>
void vafunc(int option, ...)
{
va_list param;
va_start(param, option);
if(option)
(void)va_arg(param, int);
va_end(param);
}
EOF
/opt/homebrew/opt/llvm@18/bin/clang-tidy --checks=clang-analyzer-valist.Uninitialized src1.c src2.c
# src2.c:7:11: warning: va_arg() is called on an uninitialized va_list [clang-analyzer-valist.Uninitialized]
```
Follow-up to e86542038d#17047Closes#20605
- fix internal macro `AN_APPLE_OS` reused between sources without
resetting it. It may potentially have left the system sha256
function unused.
- fix to define `WOLFSSL_OPTIONS_IGNORE_SYS` so that it always applies
to wolfSSL headers, also during feature detection.
- md4, md5, sha256: simplify fallback logic.
- delete 20+ unused macros.
- scope or move macros to avoid `-Wunused-macros` warnings.
- examples: delete unused code.
The warning detects macros defined but not used within the same C
source. It does not warn for macros defined in headers. It also works
with unity builds, but to a lesser extent.
Closes#20593
Any failf() that fill the errorbuf need to be forgotten once happy
eyeballing finds a succssful winner. Because the errorbuf, once set, is
not overwritten with future error information.
Adds test_05_05 to verify.
Reported-by: Tim Friedrich Brüggemann
Fixes#20608Closes#20613
To fix potential `-Wdisabled-macro-expansion` warnings when using these
macros within other macros. Fixing for example:
```
lib/doh.c:328:3: error: disabled expansion of recursive macro [clang-diagnostic-disabled-macro-expansion,-warnings-as-errors]
328 | ERROR_CHECK_SETOPT(CURLOPT_URL, url);
| ^
lib/doh.c:271:14: note: expanded from macro 'ERROR_CHECK_SETOPT'
271 | result = curl_easy_setopt((CURL *)doh, x, y); \
| ^
include/curl/curl.h:3332:44: note: expanded from macro 'curl_easy_setopt'
3332 | #define curl_easy_setopt(handle,opt,param) curl_easy_setopt(handle,opt,param)
| ^
[...]
```
Also update comments on why curl continues to disable
`-Wdisabled-macro-expansion` and `-Wused-but-marked-unused` warnings.
Follow-up to 92f215fea1#18477Closes#20597
The change was valid, but caused an annoying warning with perfectly
working non-binutils ld linkers:
```
ld: warning: ignoring duplicate libraries: 'my/path/usr/local/lib/libcrypto.a'
```
(seen with Apple clang, when using static `libcrypto.a`)
It means that for the binutil ld hack to work at consumption-time, curl
must be built with the same picky binutils (gcc) toolchain.
Reverts 795433b923#20434Closes#20594
Replacing API calls deprecated by OpenSSL 3, and also missing
from OpenSSL 3 no-deprecated builds, fixing builds with the latter:
`PEM_read_bio_RSAPrivateKey()`, `RSA_free()`,
`SSL_CTX_use_RSAPrivateKey()`
Also: rename callback to match its `cacertinmem.c` sibling.
Fixes#20595Closes#20596
Previous tag v6 changed upstream and points to a different commit. This
made zizmor unhappy. Previous commit is now tagged v6.0 in case we need
it.
Closes#20591
Refactor and simplify the Schannel code, primarily by reducing
duplicated buffer-management and credential-setup logic.
- split client certificate selection into get_client_cert() and SSPI
credential acquisition into acquire_sspi_handle()
- introduce a struct sbuffer for encrypted/decrypted buffering
- Add ensure_encoding_size() and ensure_decoding_size() helpers to
centralize buffer growth/realloc decisions
- Tighten variable scopes and tidy indentation/logging in the handshake
and receive/decrypt loops.
- Update comments and adjusts some receive error-condition handling to
better preserve buffered-data behavior.
Closes#20569
To reduce complexity.
- is_finished() checks if the individual transfer is done
- handle_completed() is the logic that runs for a completed
transfer
Closes#20573
Upgrade a GHA/windows job to VS2026 (from VS2022), using a runner image
released a week ago. It also comes with the same Windows SDK as VS2022:
v10.0.26100.0.
The runner image uses Windows 2025 unfortunately, which makes the job
run significantly slower than before this patch:
- configure: 49s -> 1m10s
- build: 3s -> 5s
- install test prereqs: 23s -> 27s
- run tests: 3m18s -> 4m11s
- build examples: 15s -> 25s
It's a shame.
Also:
- cmake: enable picky warnings for VS2026 internal version 19.50.
Build is clean with existing options.
- GHA/windows: make the built-in OpenSSH intall path recognize
the windows-2025-vs2026 image as windows-2025.
- windows-2025-vs2026 is able to load the cached stunnel made on
the windows-2022 runner.
- disk use of the build is almost identical to VS2022.
Before: https://github.com/curl/curl/actions/runs/21955482367/job/63418133880
After: https://github.com/curl/curl/actions/runs/21957589847/job/63426546943
Ref: 71f0157880/images/windows/Windows2025-VS2026-Readme.md
Ref: #20575Closes#20577
To simplify setting BoringSSL version, using:
`-DBORINGSSL_VERSION=0.20260211.0`
or
`-DBORINGSSL_VERSION=${boringssl_version}`
Previously it could be set via C flags, using complicated shell quotes:
`-DCMAKE_C_FLAGS="-DCURL_BORINGSSL_VERSION=\\\"${boringssl_version}\\\""`
(the C flags method remains, also for autotools)
It'd be nice if BoringSSL published its version not just via
`MODULE.bazel` in its source tree, but from its public headers, to make
these workarounds unnecessary.
Also:
- GHA/http3-linux: test both options.
Closes#20571
- openssl: move and expand explanatory comment.
- openssl: drop duplicate workaround.
- schannel: drop workaround. Unnecessary, because OpenSSL headers are
not included in or after schannel code.
- schannel: drop explicit `wincrypt.h` include. It's indirectly
included by system `<schannel.h>`.
- ldap: drop explicit `wincrypt.h` include.
It isn't used there, and also not required for the workaround.
`winldap.h` keeps including it indirectly.
Tested with BoringSSL and AWS-LC (MultiSSL with Schannel), also LDAP
enabled, and H3, unity and non-unity, and all tested cases build fine.
In lib in general, the point is to have the `#undef`s between the first
`wincrypt.h` include [1] and the first OpenSSL include [2], within a
single compilation unit. For non-unity builds the only such source is
`openssl.c`. For unity ones, depending on batch size, in theory we
should `#undef` after each `wincrypt.h` include. In practice this is
overkill and most cases are covered by `#undef`-fing _first_ in
`vtls/openssl.c`, and `#undef` in `ldap.c`. It's not impossible that we
need to add more undefs after further `wincrypt.h` includes to cover so
far undiscovered build cases [3]. Though I could not find more with the
current sources and source order.
It's also an option to include OpenSSL first, then `wincrypt.h`, as
done in libtests, but for lib and `vtls/openssl.c` it's more practical
to do the opposite.
[1] can be indirect, e.g. via `iphlpapi.h`, `schannel.h`, `winldap.h`.
[2] in
- BoringSSL/AWS-LC: any include (due to `openssl/base.h`).
Original fix removed by BoringSSL in year
[2014](ded93581f1 (diff-878093ea6426091505b4c49c59b78924f42859af0eb4ce39b8089bda9577e013)).
- OpenSSL: `openssl/ssl.h`, `openssl/x509v3.h`, and some more affected,
and including `openssl/ossl_typ.h` does the `#undef` automatically.
Since [3.1.0+](fbb9a1f997)
each inclusion does the `#undef`, in 3.0.x (and earlier) only
the first inclusion did. Initially fixed in
[0.9.6d](1955b87423)
- LibreSSL [2.3.0+](0fa826d34f):
not affected, though to suppress another warning 3.8.2+ and
a [define](e7fe6caab2)
is necessary.
[3] `lib/Makefile.inc` defines the order of unity sources.
For libtests, the case is simpler: There is always one compilation unit,
with a fixed order, and at the moment `cli_hx_download.c` is including
OpenSSL first, then wincrypt, and in this order they don't bother each
other. Also, at the moment `lib758.c` is the only other OpenSSL header
user, but it's compiled after `cli_hx_download.c` so the include is
skipped there. We may need to revisit this if either header gets
included before it.
All this said it'd be nice if BoringSSL/AWS-LC restored the built-in
workaround to behave like LibreSSL and OpenSSL and not require local
workarounds like these.
Ref: https://github.com/curl/curl/pull/20556#issuecomment-3888425644
Follow-up to 4c46c829f5#9110
Follow-up to fbe07c6829#5669#5857Closes#20567
- define `SECURITY_WIN32` globally in `curl_setup.h`.
To make sure it applies to all includes.
- document which Windows headers require `SECURITY_WIN32`.
- stop suppressing MSVC warning:
`C4201 is: nonstandard extension used : nameless struct/union`
The warning is no longer seen in supported build envs with the current
codebase.
Follow-up to 8beff43559#8419
- document why `SCHANNEL_USE_BLACKLISTS` is needed.
- just define `SCHANNEL_USE_BLACKLISTS`, drop the unnecessary value `1`.
- stop defining unused `SCH_CRED_MAX_SUPPORTED*` fallback macros.
Follow-up to 8beff43559#8419
- document why `subauth.h` is included (where missing).
- move and de-dupe `subauth.h` include into `curl_setup.h`, limit to
Schannel builds.
- stop include `schnlsp.h`. It is a 1-to-1 compatibility wrapper for
`schannel.h`.
- curl_sspi.h: clarify comment about `SP_NAME_` macros.
They are local macros, their SDK names are different and curl does not
use them.
- curl_sspi.h: drop superfluous includes `security.h` and `rpc.h`.
Cherry-picked from #20556Closes#20564
Drop detecting it at configure time, along with the interim macro
`HAVE_MSG_NOSIGNAL`. There is no longer a reason for this workaround,
and allows to save the work at configure time and simplify.
Also say in a comment that `sys/socket.h` is defining this macro.
Follow-up to 77b3bc239dCloses#20559
Originally split in 2006, but the issues cited are no longer present in
current code. As of now both `curl_setup.h` and `curl_setup_once.h` are
included once per compiler invocation, without recursion. The latter is
a sub-header of the former with no clear distinction in their contents.
Merge them to avoid having to decide where to put new global PP logic.
Also to make it easier to overview what gets defined/included globally
and in what order. (Perhaps even allowing some tidying up here.)
Follow-up to 77b3bc239dCloses#20555
- For compatibility reasons send both ALPN ids http/1.0 and http/1.1 for
HTTP/1.0 requests.
Prior to this change for compatibility reasons curl would send ALPN
http/1.1 for HTTP/1.0 requests, since some servers do not recognize
ALPN http/1.0. However some servers may recognize only ALPN http/1.0 for
HTTP/1.0 requests. Therefore curl now sends both.
Reported-by: programmerlexi@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/20487
Closes https://github.com/curl/curl/pull/20533
