The error_do_write() function may very well return witout needing the
listing of all encoding types so postpone that call until it is needed.
Closes#14831
- For the threaded resolver backend on Windows, revert back to
exclusively use the threaded resolver with libcurl-owned threading
instead of GetAddrInfoExW with Windows-owned threading.
Winsock (the Windows sockets library) has a bug where it does not wait
for all of the name resolver threads it is managing to terminate before
returning from WSACleanup. The threads continue to run and may cause a
crash.
This commit is effectively a revert of several commits that encompass
all GetAddrInfoExW code in libcurl. A manual review of merge conflicts
was used to resolve minor changes that had modified the code for
aesthetic or build reasons in other commits.
Prior to this change if libcurl was built with the threaded resolver
backend for Windows, and Windows 8 or later was the operating system at
runtime, and the caller was not impersonating another user, then libcurl
would use GetAddrInfoExW to handle asynchronous name lookups.
GetAddrInfoExW support was added in a6bbc87f, which preceded 8.6.0, and
prior to that the threaded resolver backend used libcurl-owned threading
exclusively on Windows.
Reported-by: Ionuț-Francisc Oancea
Reported-by: Razvan Pricope
Ref: https://developercommunity.visualstudio.com/t/ASAN:-heap-use-after-free-in-NdrFullPoin/10654169
Fixes https://github.com/curl/curl/issues/13509#issuecomment-2225338110
Closes https://github.com/curl/curl/pull/14794
---
Revert "asyn-thread: avoid using GetAddrInfoExW with impersonation"
This reverts commit 0caadc1f24.
Conflicts:
lib/system_win32.c
--
Revert "asyn-thread: fix curl_global_cleanup crash in Windows"
This reverts commit 428579f5d1.
--
Revert "system_win32: fix a function pointer assignment warning"
This reverts commit 26f002e02e.
--
Revert "asyn-thread: use GetAddrInfoExW on >= Windows 8"
This reverts commit a6bbc87f9e.
Conflicts:
lib/asyn-thread.c
lib/system_win32.c
--
The POP3 LIST command is not multi-line when having an argument. Fix the
definition to correct the behaviour.
Reported-by: ralfjunker on github
Fixes#14801Closes#14808
Follow-up to a07ba37b5e which did not
solve the issue of corrent polling for FTP active data connections.
Added test cases for active up-/download.
Closes#14786
Since the value is already supposed to be correct, verify that with and
assert instead of doing an assign that is not needed.
Bonus: remove unnecessary clearing of data
Closes#14784
If there is a (memory) error when creating the certinfo data, the code
would previously continue which could lead to a partial/broken response.
Now, the first error aborts and cleans up the entire thing.
A certinfo "collection" error is however still not considered an error
big enough to stop the handshake.
Bonus 1: made two functions static (and removed the Curl_ prefix) that
were not used outside of openssl.c
Bonus 2: removed the unused function Curl_ossl_set_client_cert
Closes#14780
When FTP does an active data connection, the socket connection
filter is instantiated with a listening socket. When the filter
adjusts its pollset, it needs to POLLIN, not OUT.
Bug: https://curl.se/mail/lib-2024-08/0023.html
Reported-by: Yoshimasa Ohno
Closes#14766
Always try ipv6 addresses first, ipv4 second after a delay.
If neither ipv4/6 are amongst the supplied addresses, start a happy
eyeballer for the first address family present. This is for AF_UNIX
connects.
Fixes#14761
Reported-by: janedenone on hackerone
Closes#14768
The SSL_Session object is mutated during connection inside openssl,
and it might not be thread-safe. Besides, according to documentation
of openssl:
```
SSL_SESSION objects keep internal link information about the session
cache list, when being inserted into one SSL_CTX object's session
cache. One SSL_SESSION object, regardless of its reference count,
must therefore only be used with one SSL_CTX object (and the SSL
objects created from this SSL_CTX object).
```
If I understand correctly, it is not safe to share it even in a
single thread.
Instead, serialize the SSL_SESSION before adding it to the cache,
and deserialize it after retrieving it from the cache, so that no
concurrent write to the same object is infeasible.
Also
- add a ci test for thread sanitizer
- add a test for sharing ssl sessions concurrently
- avoid redefining memory functions when not building libcurl, but
including the soruce in libtest
- increase the concurrent connections limit in sws
Notice that there are fix for a global data race for openssl which
is not yet release. The fix is cherry pick for the ci test with
thread sanitizer.
d8def79838Closes#14751
Small but, instead of sending the initial data though the connection
method, send it to the next filter in the chain. While the connection
methods accomodates for such use, by ignoring unconnected filters, it is
better to follow the filter chain explicitly.
Closes#14756
Change mingw-w64 printf format checks in public curl headers to use
`__MINGW_PRINTF_FORMAT` instead of `gnu_printf`. This syncs the format
checker with format string macros published via `curl/system.h`. (Also
disable format checks for mingw-w64 older than 3.0.0 (2013-09-20) and
classic-mingw, which do not support this macro.)
This fixes bogus format checker `-Wformat` warnings in 3rd party code
using curl format strings with the curl printf functions, when using
mingw-w64 7.0.0 (2019-11-10) and older (with GCC, MSVCRT).
It also allows to delete two workaounds for this within curl itself:
- setting `-D__USE_MINGW_ANSI_STDIO=1` for mingw-w64 via cmake and
configure for `docs/examples` and `tests/http/clients`.
Ref: c730c8549b#14640
The format check macro is incompatible (depending on mingw-w64 version
and configuration) with the C99 `%z` (`size_t`) format string used
internally by curl.
To work around this problem, override the format check style in curl
public headers to use `gnu_printf`. This is compatible with `%z` in all
mingw-w64 versions and allows keeping the C99 format strings internally.
Also:
- lib/ws.c: add missing space to an error message.
- docs/examples/ftpgetinfo.c: fix to use standard printf.
Ref: #14643 (take 1)
Follow-up to 3829759bd0#12489Closes#14703
It could previously be set with configure/cmake and used in rare cases
for reading randomness: with ancient mbedTLS or rustls without
arc4random.
We now get randomness in this order:
1. The TLS library's way to provide random
2. On Windows: Curl_win32_random
3. if arc4random exists, use that
4. weak non-crytographically strong pseudo-random
Closes#14749
Normally, when a connection's filters have all connected, the
multiplex status is determined. However, HTTP/2 Upgrade:
requests will only do this when the first server response
has been received.
The current connection reuse mechanism does not accomodate
that and when the time between connect and response is large
enough, connection reuse may not happen as desired.
See test case 2405 failures, such as in
https://github.com/curl/curl/actions/runs/10629497461/job/29467166451
Add 'conn->bits.asks_multiplex' as indicator that a connection is
still being evaluated for mulitplexing, so that new transfers
may wait on this to be cleared.
Closes#14739
The symbols have not been in use for 17+ years and they did not do
anything for several years before that, but apparently there are still
code using them.
Follow-up to 3b057d4b7aFixes#14747
Reported-by: Kai Pastor
Closes#14748
Existing C macro lacked build-level counterparts.
Add them in this patch.
- cmake: `-DCURL_DISABLE_SHA512_256=ON`
- autotools: `--disable-sha512-256`
Also drop the checker exception from `test1165.pl`.
Follow-up to cbe41d151d#12897Closes#14753
Also fix issues:
- cmake: fix `CURL_DISABLE_HTTP_AUTH` option
- cmake: fix `CURL_DISABLE_SHUFFLE_DNS` option
Fixes:
```
Present in CMakeLists.txt, not propagated via curl_config.h.cmake: CURL_DISABLE_HTTP_AUTH
Present in CMakeLists.txt, not propagated via curl_config.h.cmake: CURL_DISABLE_SHUFFLE_DNS
```
Ref: https://github.com/curl/curl/actions/runs/10655027540/job/29532054141?pr=14754#step:11:2090Closes#14754
Some POP3 commands are multi-line, e.g. have responses terminated by a
last line with '.', but some are not. Define the known command
properties and fix response handling.
Add test case for STAT.
Fixes#14677
Reported-by: ralfjunker on github
Closes#14707
Previously this functionality was limited to platforms that not already
use CRLF as native line endings.
TODO: 4.5 ASCII support now considered fixed
Closes#14717
When a OpenSSL quic connection filter is aborted early, as the
server was not responding, the ssl instances where not closed
as they should.
Fixes#14720
Reported-by: ralfjunker on github
Closes#14724
Turns out `gnutls_record_send()` does really what the name says: it
sends exactly one TLS record. If more than 16k are there to send, it
needs to be called again with new buffer offset and length.
Continue sending record until the input is all sent or a EAGAIN (or
fatal error) is returned by gnutls.
Closes#14722
Since the introduction of client writers, we check the body length in
the PROTOCOL phase and do FTP lineend conversions laster in the
CONTENT_DECODING phase. This means we no longer need to count the
conversions for length checks.
Closes#14709
Update IP related information at the connection and the transfer in two
places only: once the filter chain connects and when a transfer is added
to a connection. The latter only updates on reuse when the filters
already are connected.
The only user of that information before a full connect is the HAProxy
filter. Add cfilter CF_QUERY_IP_INFO query to let it find the
information from the filters "below".
This solves two issues with the previous version:
- updates where often done twice with the same info
- happy eyeballing filter "forks" could overwrite each others
updates before the full winner was determined.
Closes#14699
When we downloaded all we wanted, and we did not want a response body,
and no Trailer: has been announced, and the receive gives EAGAIN, do not
hang around unnecessarily.
Some servers are buggy in HEAD processing and fail to send the HTTP/2
EOS. Since we do not need any more data, end the request right there.
This will cause us to send a RST_STREAM to the server.
Fixes#14670
Reported-by: Gruber Glass
Closes#14685
This is a better match for what they do and the general "cpool"
var/function prefix works well.
The pool now handles very long hostnames correctly.
The following changes have been made:
* 'struct connectdata', e.g. connections, keep new members
named `destination` and ' destination_len' that fully specifies
interface+port+hostname of where the connection is going to.
This is used in the pool for "bundling" of connections with
the same destination. There is no limit on the length any more.
* Locking: all locks are done inside conncache.c when calling
into the pool and released on return. This eliminates hazards
of the callers keeping track.
* 'struct connectbundle' is now internal to the pool. It is no
longer referenced by a connection.
* 'bundle->multiuse' no longer exists. HTTP/2 and 3 and TLS filters
no longer need to set it. Instead, the multi checks on leaving
MSTATE_CONNECT or MSTATE_CONNECTING if the connection is now
multiplexed and new, e.g. not conn->bits.reuse. In that case
the processing of pending handles is triggered.
* The pool's init is provided with a callback to invoke on all
connections being discarded. This allows the cleanups in
`Curl_disconnect` to run, wherever it is decided to retire
a connection.
* Several pool operations can now be fully done with one call.
Pruning dead connections, upkeep and checks on pool limits
can now directly discard connections and need no longer return
those to the caller for doing that (as we have now the callback
described above).
* Finding a connection for reuse is now done via `Curl_cpool_find()`
and the caller provides callbacks to evaluate the connection
candidates.
* The 'Curl_cpool_check_limits()' now directly uses the max values
that may be set in the transfer's multi. No need to pass them
around. Curl_multi_max_host_connections() and
Curl_multi_max_total_connections() are gone.
* Add method 'Curl_node_llist()' to get the llist a node is in.
Used in cpool to verify connection are indeed in the list (or
not in any list) as they need to.
I left the conncache.[ch] as is for now and also did not touch the
documentation. If we update that outside the feature window, we can
do this in a separate PR.
Multi-thread safety is not achieved by this PR, but since more details
on how pools operate are now "internal" it is a better starting
point to go for this in the future.
Closes#14662
1. GHA/windows: enable WinIDN in Linux cross-builds.
(to reveal the issue in CI.)
2. fix compiler warning when building with mingw-w64 supporting
WinIDN, while targeting pre-Vista Windows, with a `WINVER` set to
target Vista or newer. (Such was Ubuntu's mingw-w64 with the
classic-mingw-specific trick in point 3 of this PR.)
```
../../lib/idn.c:154:23: error: redundant redeclaration of ‘IdnToAscii’ [-Werror=redundant-decls]
154 | WINBASEAPI int WINAPI IdnToAscii(DWORD dwFlags,
| ^~~~~~~~~~
In file included from /usr/share/mingw-w64/include/windows.h:73,
from /usr/share/mingw-w64/include/winsock2.h:23,
from ../../lib/setup-win32.h:91,
from ../../lib/curl_setup.h:308,
from ../../lib/idn.c:29:
/usr/share/mingw-w64/include/winnls.h:1075:30: note: previous declaration of ‘IdnToAscii’ was here
1075 | WINNORMALIZEAPI int WINAPI IdnToAscii (DWORD dwFlags, LPCWSTR lpUnicodeCharStr, int cchUnicodeChar, LPWSTR lpASCIICharStr, int cchASCIIChar);
| ^~~~~~~~~~
[...same for IdnToUnicode...]
```
Ref: https://github.com/curl/curl/actions/runs/10542832783/job/29210098553#step:7:89
3. drop `WINVER` override for classic-mingw. curl no longer supports
building with classic-mingw.
Reverts 37f1c21cb9#7581
4. sync `if IdnToUnicode can be linked` detection snippet with the live
code in `lib/idn.c`. It fixes detection for the scenario in point 2.
5. delete unused `WINIDN_DIR` variable.
Bug: https://github.com/curl/curl/pull/12606#issuecomment-1885381038
Previous abandoned attempt: #12684
Reviewed-by: Jay Satiro
Closes#14680
Before this patch, `libhostname.so` and `chkhostname` were a test
facility for overriding `gethostname()` in non-debug builds on
Linux and other Unix platforms supporting `LD_PRELOAD`.
`gethostname()` has a single use with SMTP.
The alternative way to override `gethostname()` is building in debug
mode, which allows to do this via the `CURL_GETHOSTNAME` env, on all
platforms.
Drop the `LD_PRELOAD` solution in favour of the above.
Also:
- delete inactive NTLM code with a `gethostname()` call made from it.
- streamline NTLM code by dropping a `printf()` and a macro.
- tests: stop setting `CURL_GETHOSTNAME` where unnecessary.
Closes#14695
Before this change, calling curl_share_setopt w/ CURL_LOCK_DATA_CONNECT
a second time would re-initialize the connection cache, rather than use
the existing one.
After this change, calling curl_share_setopt w/ CURL_LOCK_DATA_CONNECT
multiple times will have no effect after the first call.
Closes#14696
Curl_xfer_send and Curl_xfer_recv had commented FIXMEs about protocol
setting up the transfers badly, but in reality these functions are too
low-level to be able to depend on the protocol transfer setups having
been done yet. Removed.
The functions had checks for data and data->conn that I convered to
asserts since they SHOULD always be valid in this function. The same
goes for the runtime check for buffer_size > 0 that I also converted to
an assert since that should never be set to an invalid value.
Closes#14688
Remove the "hardcoded" logic for the pop3 transfer handler and instead
use the generic protocol handler write_resp function.
Remove the check for 'data->req.ignorebody' because I cannot find a code
flow where this is set for POP3.
Closes#14684
- configure: disable pthreads by default on Windows.
- configure: disable detecting `fseeko()` on Windows.
(It exists in mingw-w64 2.0.0 and newer, but it's permanently ignored
in CMake, as this function is never necessary on Windows.)
- extend existing exceptions with their Windows variants.
- `lib/formdata.c`: prioritize `_fseeki64()` over `fseeko()`.
To reduce the difference between Windows builds, which now all use
`_fseeki64()`.
- cmake: perm-enable `HAVE_DIRENT_H` and `HAVE_OPENDIR` for mingw-w64,
to match configure.
Follow-up to bfe54b0e88#13137
This in theory could make the dir listing feature work in mingw-w64
build, but in my tests (on WINE) it failed at the preceding `open()`
call.
- cmake: perm-enable `HAVE_STRINGS_H` and `HAVE_UTIME_H` for mingw-w64,
to match configure. (They are wrappers and make no difference in the build.)
Also:
- configure: sync `USE_MANUAL` macro with cmake, by only setting it for
`src`. Drop checker exception.
- CI: use `--disable-dependency-tracking` in existing jobs.
- CI: install packages before git checkout, in existing jobs.
Closes#14678
- show `OpenSSL v3+` when detected (as in `./configure`).
(this string also makes its way to `curl-config`.)
- prefer `unset(VAR)` over `set(VAR)`.
Same effect, but `unset()` tells the intent unambiguously.
https://cmake.org/cmake/help/latest/command/set.html
- drop "implementation" from an `option()` description.
- FindGSS: replace legacy keyword alias with modern alternative.
https://cmake.org/cmake/help/latest/command/get_filename_component.html
- move `CURL_STATIC_CRT` logic next to its `option()`.
- improve order of `libcurl.pc`/`curl-config` variable init lines.
- tests: drop/shorten custom target names.
They inflated generated make files by 550KB.
Keep target name logic for sync between code snippets.
Follow-up to a2ef5d36b3#14660
- clear a variable after use.
- restore `STATUS` for `Features:`/`Protocols:` `message()`s:
Without it the output goes to stderr, and appears in red in CMake GUI.
It doesn't seem possible to show a line on stdout without leading
underscores to match `curl -V` and `./configure` output.
Partial revert of acbc6b703f#14197
- WindowsCache: move `HAVE_LINUX_TCP_H` into the header group.
- move strings to the same line as their `STRING` keyword.
- formatting in generated code.
- delete bogus comment.
- unfold lines for readability.
- fix a too long line. (for cmakelint)
- missing quotes, whitespace, comments.
Closes#14610
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.
Regression from 0a0c9b6dfa, shipped in 8.0.0
Fixes#14656
Reported-by: Venkat Krishna R
Closes#14657
