Use the closure handle for disconnecting connection cache entries so
that anything that happens during the disconnect is not stored and
associated with the 'data' handle which already just finished a transfer
and it is important that details from the unrelated disconnect does not
taint meta-data in the data handle.
Like storing the response code.
This also adjust test 1506. Unfortunately it also removes a key part of
the test that verifies that a connection is closed since when this
output vanishes (because the closure handle is used), we don't know
exactly that the connection actually gets closed in this test...
Reported-by: ohyeaah on github
Fixes#12367Closes#12405
- when a connect immediately goes into DRAINING state, do
not attempt retries in the QUIC connection filter. Instead,
return CURLE_WEIRD_SERVER_REPLY
- When eyeballing, interpret CURLE_WEIRD_SERVER_REPLY as an
inconclusive answer. When all addresses have been attempted,
rewind the address list once on an inconclusive answer.
- refs #11832 where connects were retried indefinitely until
the overall timeout fired
Closes#12400
Move the allocation of the directory name down to after the fopen() call
to allow that shortcut code path to avoid a superfluous malloc+free
cycle.
Follow-up to 73b65e94f3Closes#12398
- add `SingleRequest->download_done` as indicator that
all download bytes have been received
- remove `stop_reading` bool from readwrite functions
- move excess body handling into client download writer
Closes#12371
Because the function renames the temp file to the target name as a last
step, if the file was previously owned by a different user, not ORing
the old mode could otherwise end up creating a file that was no longer
readable by the original owner after save.
Reported-by: Loïc Yhuel
Fixes#12299Closes#12395
Only using random letters in the name plus a ".tmp" extension. Not by
appending characters to the final file name.
Reported-by: Maksymilian Arciemowicz
Closes#12388
Lower the barrier to enable `infof_certstack()` from OpenSSL 3 to
OpenSSL 1.1.x, and LibreSSL 3.6 or upper.
With the caveat, that "group name" and "type name" are missing from
the log output with these TLS backends.
Follow-up to b6e6d4ff8f#12030
Reviewed-by: Daniel Stenberg
Closes#12385
- build quictls with `no-deprecated` in CI to have test coverage for
this OpenSSL 3 configuration.
- don't call `OpenSSL_add_all_algorithms()`, `OpenSSL_add_all_digests()`.
The caller code is meant for OpenSSL 3, while these two functions were
only necessary before OpenSSL 1.1.0. They are missing from OpenSSL 3
if built with option `no-deprecated`, causing build errors:
```
vtls/openssl.c:4097:3: error: call to undeclared function 'OpenSSL_add_all_algorithms'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
vtls/openssl.c:4098:3: error: call to undeclared function 'OpenSSL_add_all_digests'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
```
Ref: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48587418?fullLog=true#L7667
Regression from b6e6d4ff8f#12030
Bug: https://github.com/curl/curl/issues/12380#issuecomment-1822944669
Reviewed-by: Alex Bozarth
- vquic/curl_ngtcp2: fix using `SSL_get_peer_certificate` with
`no-deprecated` quictls 3 builds.
Do it by moving an existing solution for this from `vtls/openssl.c`
to `vtls/openssl.h` and adjusting caller code.
```
vquic/curl_ngtcp2.c:1950:19: error: implicit declaration of function 'SSL_get_peer_certificate'; did you mean 'SSL_get1_peer_certificate'? [-Wimplicit-function-declaration]
```
Ref: https://github.com/curl/curl/actions/runs/6960723097/job/18940818625#step:24:1178
- curl_ntlm_core: fix `-Wunused-parameter`, `-Wunused-variable` and
`-Wunused-function` when trying to build curl with NTLM enabled but
without the necessary TLS backend (with DES) support.
Closes#12384
Windows compilers define `_WIN32` automatically. Windows SDK headers
or build env defines `WIN32`, or we have to take care of it. The
agreement seems to be that `_WIN32` is the preferred practice here.
Make the source code rely on that to detect we're building for Windows.
Public `curl.h` was using `WIN32`, `__WIN32__` and `CURL_WIN32` for
Windows detection, next to the official `_WIN32`. After this patch it
only uses `_WIN32` for this. Also, make it stop defining `CURL_WIN32`.
There is a slight chance these break compatibility with Windows
compilers that fail to define `_WIN32`. I'm not aware of any obsolete
or modern compiler affected, but in case there is one, one possible
solution is to define this macro manually.
grepping for `WIN32` remains useful to discover Windows-specific code.
Also:
- extend `checksrc` to ensure we're not using `WIN32` anymore.
- apply minor formatting here and there.
- delete unnecessary checks for `!MSDOS` when `_WIN32` is present.
Co-authored-by: Jay Satiro
Reviewed-by: Daniel Stenberg
Closes#12376
- have common pattern of `if not match, continue`
- revert pages long if()s to return early
- move dead connection check to later since it may
be relatively expensive
- check multiuse also when NOT building with NGHTTP2
- for MULTIUSE bundles, verify that the inspected
connection indeed supports multiplexing when in use
(bundles may contain a mix of connection, afaict)
Closes#12373
Enable more picky compiler warnings. I've found these options in the
nghttp3 project when implementing the CMake quick picky warning
functionality for it [1].
`-Wunused-macros` was too noisy to keep around, but fixed a few issues
it revealed while testing.
- autotools: reflect the more precisely-versioned clang warnings.
Follow-up to 033f8e2a08#12324
- autotools: sync between clang and gcc the way we set `no-multichar`.
- autotools: avoid setting `-Wstrict-aliasing=3` twice.
- autotools: disable `-Wmissing-noreturn` for MSYS gcc targets [2].
It triggers in libtool-generated stub code.
- lib/timeval: delete a redundant `!MSDOS` guard from a `WIN32` branch.
- lib/curl_setup.h: delete duplicate declaration for `fileno`.
Added in initial commit ae1912cb0d
(1999-12-29). This suggests this may not be needed anymore, but if
it does, we may restore this for those specific (non-Windows) systems.
- lib: delete unused macro `FTP_BUFFER_ALLOCSIZE` since
c1d6fe2aaa.
- lib: delete unused macro `isxdigit_ascii` since
f65f750742.
- lib/mqtt: delete unused macro `MQTT_HEADER_LEN`.
- lib/multi: delete unused macro `SH_READ`/`SH_WRITE`.
- lib/hostip: add `noreturn` function attribute via new `CURL_NORETURN`
macro.
- lib/mprintf: delete duplicate declaration for `Curl_dyn_vprintf`.
- lib/rand: fix `-Wunreachable-code` and related fallouts [3].
- lib/setopt: fix `-Wunreachable-code-break`.
- lib/system_win32 and lib/timeval: fix double declarations for
`Curl_freq` and `Curl_isVistaOrGreater` in CMake UNITY mode [4].
- lib/warnless: fix double declarations in CMake UNITY mode [5].
This was due to force-disabling the header guard of `warnless.h` to
to reapply it to source code coming after `warnless.c` in UNITY
builds. This reapplied declarations too, causing the warnings.
Solved by adding a header guard for the lines that actually need
to be reapplied.
- lib/vauth/digest: fix `-Wunreachable-code-break` [6].
- lib/vssh/libssh2: fix `-Wunreachable-code-break` and delete redundant
block.
- lib/vtls/sectransp: fix `-Wunreachable-code-break` [7].
- lib/vtls/sectransp: suppress `-Wunreachable-code`.
Detected in `else` branches of dynamic feature checks, with results
known at compile-time, e.g.
```c
if(SecCertificateCopySubjectSummary) /* -> true */
```
Likely fixable as a separate micro-project, but given SecureTransport
is deprecated anyway, let's just silence these locally.
- src/tool_help: delete duplicate declaration for `helptext`.
- src/tool_xattr: fix `-Wunreachable-code`.
- tests: delete duplicate declaration for `unitfail` [8].
- tests: delete duplicate declaration for `strncasecompare`.
- tests/libtest: delete duplicate declaration for `gethostname`.
Originally added in 687df5c8c3
(2010-08-02).
Got complicated later: c49e9683b8
If there are still systems around with warnings, we may restore the
prototype, but limited for those systems.
- tests/lib2305: delete duplicate declaration for
`libtest_debug_config`.
- tests/h2-download: fix `-Wunreachable-code-break`.
[1] a70edb08e9/cmake/PickyWarningsC.cmake
[2] https://ci.appveyor.com/project/curlorg/curl/builds/48553586/job/3qkgjauiqla5fj45?fullLog=true#L1675
[3] https://github.com/curl/curl/actions/runs/6880886309/job/18716044703?pr=12331#step:7:72https://github.com/curl/curl/actions/runs/6883016087/job/18722707368?pr=12331#step:7:109
[4] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L204
[5] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L218
[6] https://github.com/curl/curl/actions/runs/6880886309/job/18716042927?pr=12331#step:7:290
[7] https://github.com/curl/curl/actions/runs/6891484996/job/18746659406?pr=12331#step:9:1193
[8] https://github.com/curl/curl/actions/runs/6882803986/job/18722082562?pr=12331#step:33:1870Closes#12331
If curl_off_t and size_t have the same size (which is common on modern
64 bit systems), a condition cannot occur which Coverity pointed
out. Avoid the warning by having the code conditionally only used if
curl_off_t actually is larger.
Follow-up to 1cd2f0072fCloses#12370
- changed header/chunk/handler->readwrite prototypes to accept `buf`,
`blen` and a `pconsumed` pointer. They now get the buffer to work on
and report back how many bytes they consumed
- eliminated `k->str` in SingleRequest
- improved excess data handling to properly calculate with any body data
left in the headerb buffer
- eliminated `k->badheader` enum to only be a bool
Closes#12283
Previously it was wrongly returning CURLE_OUT_OF_MEMORY when the
response did not contain any addresses. Now it more accurately returns
CURLE_COULDNT_RESOLVE_HOST.
Reported-by: lRoccoon on github
Fixes#12365Closes#12366
- refs #12356 where a UAF is reported when closing a connection
with a stream whose easy handle was cleaned up already
- handle DETACH events same as DONE events in h2/h3 filters
Fixes#12356
Reported-by: Paweł Wegner
Closes#12364
- tests: verify CMake `DISABLE` options.
Make an exception for 2 CMake-only ones, and one more that's
using a different naming scheme, also in autotools and source.
- cmake: add support for `CURL_DISABLE_HEADERS_API`.
Suggested-by: Daniel Stenberg
Ref: https://github.com/curl/curl/pull/12345#pullrequestreview-1736238641Closes#12353
The current design of the Hyper integration requires rebuilding the
Hyper clientconn for each request. However, building the clientconn
requires resending the HTTP/2 connection preface, which is incorrect
from a protocol perspective. That in turn causes servers to send GOAWAY
frames, effectively degrading performance to "no connection reuse" in
the best case. It may also be triggering some bugs where requests get
dropped entirely and reconnects take too long.
This doesn't rule out HTTP/2 support with Hyper, but it may take a
redesign of the Hyper integration in order to make things work.
Closes#12191
Instead of a loop to scan over the potentially 30+ scheme names, this
uses a "perfect hash" table. This works fine because the set of schemes
is known and cannot change in a build. The hash algorithm and table size
is made to only make a single scheme index per table entry.
The perfect hash is generated by a separate tool (scripts/schemetable.c)
Closes#12347
- add `struct ssl_peer` to keep hostname, dispname and sni
for a filter
- allocate `sni` for use in VTLS backend
- eliminate `Curl_ssl_snihost()` and its use of the download buffer
- use ssl_peer in SSL and QUIC filters
Closes#12349
Before this patch some source files were overriding gcc warning options,
but without restoring them at the end of the file. In CMake UNITY builds
these options spilled over to the remainder of the source code,
effecitvely disabling them for a larger portion of the codebase than
intended.
`#pragma clang diagnostic` didn't have such issue in the codebase.
Reviewed-by: Marcel Raad
Closes#12352
- cmake: fix casing of `UnixSockets` to match the rest of the codebase.
- curl-compilers.m4: fix casing in a comment.
- setup-win32: delete unused Windows version constant aliases.
Reviewed-by: Marcel Raad
Closes#12351
Fixes:
```
./lib/url.c:178:56: warning: use of an empty initializer is a C2x extension [-Wc2x-extensions]
178 | static const struct Curl_handler * const protocols[] = {
| ^
./lib/url.c:178:56: warning: zero size arrays are an extension [-Wzero-length-array]
```
Closes#12344
Fixes:
```
./lib/url.c:456:35: error: no member named 'formp' in 'struct UrlState'
456 | Curl_mime_cleanpart(data->state.formp);
| ~~~~~~~~~~~ ^
```
Regression from 74b87a8af1#11682Closes#12343
As the blob pointers are to be duplicated, the function must not return
mid-function with lingering pointers to the old handle's allocated data,
as that would lead to double-free in OOM situations.
Make sure to clear all destination pointers first to avoid this risk.
Closes#12337
Fix compiler warnings in builds with disabled auths, NTLM and SPNEGO.
E.g. with `CURL_DISABLE_BASIC_AUTH` + `CURL_DISABLE_BEARER_AUTH` +
`CURL_DISABLE_DIGEST_AUTH` + `CURL_DISABLE_NEGOTIATE_AUTH` +
`CURL_DISABLE_NTLM` on non-Windows.
```
./curl/lib/http.c:737:12: warning: unused variable 'result' [-Wunused-variable]
CURLcode result = CURLE_OK;
^
./curl/lib/http.c:995:18: warning: variable 'availp' set but not used [-Wunused-but-set-variable]
unsigned long *availp;
^
./curl/lib/http.c:996:16: warning: variable 'authp' set but not used [-Wunused-but-set-variable]
struct auth *authp;
^
```
Regression from e92edfbef6#11490Fixes#12228Closes#12335
Builds with libssh2 + `-DCURL_DISABLE_DIGEST_AUTH=ON` +
`-DCURL_DISABLE_AWS=ON` in combination with either Schannel on Windows,
or `-DCURL_DISABLE_NTLM=ON` on other operating systems failed while
compiling due to a missing HMAC declaration.
The reason is that HMAC is required by `lib/sha256.c` which publishes
`Curl_sha256it()` which is required by `lib/vssh/libssh2.c` when
building for libssh2 v1.8.2 (2019-05-25) or older.
Make sure to compile the HMAC bits for a successful build.
Both HMAC and `Curl_sha256it()` rely on the same internals, so splitting
them into separate sources isn't practical.
Fixes:
```
[...]
In file included from ./curl/_x64-win-ucrt-cmake-llvm-bld/lib/CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c:310:
./curl/lib/sha256.c:527:42: error: array has incomplete element type 'const struct HMAC_params'
527 | const struct HMAC_params Curl_HMAC_SHA256[] = {
| ^
./curl/lib/curl_sha256.h:34:21: note: forward declaration of 'struct HMAC_params'
[...]
```
Regression from e92edfbef6#11490Fixes#12273Closes#12332
Fixes memory-leak when OOM mid-function
Use plain free instead of safefree, since the entire struct is
freed below.
Remove some free calls that is already freed in Curl_freeset()
Closes#12329
1. Because the value is not strictly set with a setopt option.
2. Because otherwise when duping a handle when all the set.* fields are
first copied and an error happens (think out of memory mid-function),
the function would easily free the list *before* it was deep-copied,
which could lead to a double-free.
Closes#12323
autotools passes `LDFLAGS` automatically linker commands. curl's
`lib/Makefile.am` customizes libcurl linker flags. In that
customization, it added `LDFLAGS` to the custom flags. This resulted in
passing `LDFLAGS` _twice_ to the `libtool` command.
Most of the time this is benign, but some `LDFLAGS` options can break
the build when passed twice. One such example is passing `.o` files,
e.g. `crt*.o` files necessary when customizing the C runtime, e.g. for
MUSL builds.
Passing them twice resulted in duplicate symbol errors:
```
libtool: link: clang-15 --target=aarch64-unknown-linux-musl [...] /usr/lib/aarch64-linux-musl/crt1.o [...] /usr/lib/aarch64-linux-musl/crt1.o [...]
ld.lld-15: error: duplicate symbol: _start
>>> defined at crt1.c
>>> /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
>>> defined at crt1.c
>>> /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
[...]
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
This behaviour came with commit 1a593191c2
(2013-07-23) as a fix for bug https://curl.haxx.se/bug/view.cgi?id=1217.
The patch was a works-for-me hack that ended up merged in curl:
https://sourceforge.net/p/curl/bugs/1217/#06ef
With the root cause remaining unclear.
Perhaps the SUNPro 12 linker was sensitive to `-L` `-l` order, requiring
`-L` first? This would be unusual and suggests a bug in either the
linker or in `libtool`.
The curl build does pass the list of detected libs via its own
`LIBCURL_LIBS` variable, which ends up before `LDFLAGS` on the `libtool`
command line, but it's the job of `libtool` to ensure that even
a peculiar linker gets the options in the expected order. Also because
autotools passes `LDFLAGS` last, making it hardly possible to pass
anything after it.
Perhaps in the 10 years since this issue, this already got a fix
upstream.
This patch deletes `LDFLAGS` from our customized libcurl options,
leaving a single copy of them as passed by autotools automatically.
Reverts 1a593191c2Closes#12310
To allow passing `LDFLAGS` specific to libcurl (`CURL_LDFLAGS_LIB`) and
curl tool (`CURL_LDFLAGS_BIN`).
This makes it possible to build libcurl and curl with a single
invocation with lib- and tool-specific custom linker flags.
Such flag can be enabling `.map` files, a `.def` file for libcurl DLL,
controlling static/shared, incl. requesting a static curl tool (with
`-static-libtool-libs`) while building both shared and static libcurl.
curl-for-win uses the above and some more.
These options are already supported in `Makefile.mk`. CMake has built-in
variables for this.
Closes#12312
