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
To make it work properly with curl_easy_duphandle(). This, because
duphandle duplicates the entire 'UserDefined' struct by plain copy while
'hstslist' is a linked curl_list of file names. This would lead to a
double-free when the second of the two involved easy handles were
closed.
Closes#12315
- Increase the maximum request method name length from 11 to 23.
For HTTP/1.1 and earlier there's not a specific limit in libcurl for
method length except that it is limited by the initial HTTP request
limit (DYN_HTTP_REQUEST). Prior to fc2f1e54 HTTP/2 was treated the same
and there was no specific limit.
According to Internet Assigned Numbers Authority (IANA) the longest
registered method is UPDATEREDIRECTREF which is 17 characters.
Also there are unregistered methods used by some companies that are
longer than 11 characters.
The limit was originally added by 61f52a97 but not used until fc2f1e54.
Ref: https://www.iana.org/assignments/http-methods/http-methods.xhtml
Closes https://github.com/curl/curl/pull/12311
In recent quiche, transport parameter API is separated
with quiche_conn_peer_transport_params().
(https://github.com/cloudflare/quiche/pull/1575)
It breaks with bulding with latest(post 0.18.0) quiche.
Closes#12180
GCC 14 introduces a new -Walloc-size included in -Wextra which gives:
```
src/tool_operate.c: In function ‘add_per_transfer’:
src/tool_operate.c:213:5: warning: allocation of insufficient size ‘1’ for type ‘struct per_transfer’ with size ‘480’ [-Walloc-size]
213 | p = calloc(sizeof(struct per_transfer), 1);
| ^
src/var.c: In function ‘addvariable’:
src/var.c:361:5: warning: allocation of insufficient size ‘1’ for type ‘struct var’ with size ‘32’ [-Walloc-size]
361 | p = calloc(sizeof(struct var), 1);
| ^
```
The calloc prototype is:
```
void *calloc(size_t nmemb, size_t size);
```
So, just swap the number of members and size arguments to match the
prototype, as we're initialising 1 struct of size `sizeof(struct
...)`. GCC then sees we're not doing anything wrong.
Closes#12292
This patch makes the following changes:
- adds the option `CURL_DISABLE_INSTALL` - to disable 'install' targets.
- Removes the target `curlu` when the option `BUILD_TESTING` is set to
`OFF` - to prevent it from being loaded in Visual Studio.
Closes#12287
Uniformly use `OPENSSL_VERSION_NUMBER` to check for OpenSSL version.
Before this patch some places used `OPENSSL_VERSION_MAJOR`.
Also fix `lib/md4.c`, which included `opensslconf.h`, but that doesn't
define any version number in these implementations: BoringSSL, AWS-LC,
LibreSSL, wolfSSL. (Only in mainline OpenSSL/quictls). Switch that to
`opensslv.h`. This wasn't causing a deeper problem because the code is
looking for v3, which is only provided by OpenSSL/quictls as of now.
According to https://github.com/openssl/openssl/issues/17517, the macro
`OPENSSL_VERSION_NUMBER` is safe to use and not deprecated.
Reviewed-by: Marcel Raad
Closes#12298
Some servers don't support the ALPN protocol "http/1.0" (e.g. IIS 10),
avoid it and use "http/1.1" instead.
This reverts commit df856cb5c9 (#10183).
Fixes#12259Closes#12285
- Pelles C: Unclear status, failed to obtain a fresh copy a few months
ago. Possible website is HTTP-only. ~10 years ago I left this compiler
dealing with crashes and other issues with no response on the forum
for years. It has seen some activity in curl back in 2021.
- LCC: Last stable release in September 2002.
- Salford C: Misses winsock2 support, possibly abandoned? Last mentioned
in 2006.
- Borland C++: We dropped Borland C++ support in 2018.
- MS Visual C++ 6.0: Released in 1998. curl already requires VS 2010
(or possibly 2008) as a minimum.
Closes#12222
We use `stdint.h` unconditionally in all places except one. These uses
are imposed by external dependencies / features. nghttp2, quic, wolfSSL
and `HAVE_MACH_ABSOLUTE_TIME` do require this C99 header. It means that
any of these features make curl require a C99 compiler. (In case of
MSVC, this means Visual Studio 2010 or newer.)
This patch changes the single use of `stdint.h` guarded by
`HAVE_STDINT_H` to use `stdint.h` unconditionally. Also stop using
`inttypes.h` as an alternative there. `HAVE_INTTYPES_H` wasn't used
anywhere else, allowing to delete this feature check as well.
Closes#12275
This PR has these changes:
Renaming of unencode_* to cwriter, e.g. client writers
- documentation of sendf.h functions
- move max decode stack checks back to content_encoding.c
- define writer phase which was used as order before
- introduce phases for monitoring inbetween decode phases
- offering default implementations for init/write/close
Add type paramter to client writer's do_write()
- always pass all writes through the writer stack
- writers who only care about BODY data will pass other writes unchanged
add RAW and PROTOCOL client writers
- RAW used for Curl_debug() logging of CURLINFO_DATA_IN
- PROTOCOL used for updates to data->req.bytecount, max_filesize checks and
Curl_pgrsSetDownloadCounter()
- remove all updates of data->req.bytecount and calls to
Curl_pgrsSetDownloadCounter() and Curl_debug() from other code
- adjust test457 expected output to no longer see the excess write
Closes#12184
autotools unexpectedly detects `arc4random` because it is also looking
into dependency libs. One dependency, LibreSSL, happens to publish an
`arc4random` function (via its shared lib before v3.7, also via static
lib as of v3.8.2). When trying to use this function in `lib/rand.c`,
its protoype is missing. To fix that, curl included a prototype, but
that used a C99 type without including `stdint.h`, causing:
```
../../lib/rand.c:37:1: error: unknown type name 'uint32_t'
37 | uint32_t arc4random(void);
| ^
1 error generated.
```
This patch improves this by dropping the local prototype and instead
limiting `arc4random` use for non-OpenSSL builds. OpenSSL builds provide
their own random source anyway.
The better fix would be to teach autotools to not link dependency libs
while detecting `arc4random`.
LibreSSL publishing a non-namespaced `arc4random` tracked here:
https://github.com/libressl/portable/issues/928
Regression from 755ddbe901#10672
Reviewed-by: Daniel Stenberg
Fixes#12257Closes#12274
This change fixes a compiler warning with gcc-12.2.0 when
`-DCURL_DISABLE_BEARER_AUTH=ON` is used.
/home/tox/src/curl/lib/http.c: In function 'Curl_http_input_auth':
/home/tox/src/curl/lib/http.c:1147:12: warning: suggest braces around empty body in an 'else' statement [-Wempty-body]
1147 | ;
| ^
Closes#12262
Since vanilla OpenSSL does not support the QUIC API I think it helps
users to identify the correct OpenSSL fork in version output. The best
(crude) way to do that right now seems to be to check if ngtcp2 support
is enabled.
Closes#12270
- tunnel https proxy used for http: transfers does
no check if proxy-ssl configuration matches
- test cases added, test_10_12 fails on 8.4.0
Closes#12255
