Have curl_multi_init() use a much larger DNS hash table than used for
the easy interface to scale and perform better when used with _many_
host names.
curl_share_init() sets an in-between size.
Inspired-by: Ivan Tsybulin
See #9340Closes#9376
First check for errors and return CURLM_UNRECOVERABLE_POLL
before moving forward and waiting on socket readiness events.
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Reported-by: Daniel Stenberg
Ref: #9361
Follow up to #8961Closes#9372
On Windows revents was not populated for extra_fds if
multi_wait had to wait due to the Curl_poll pre-check
not signalling any readiness. This commit fixes that.
Reviewed-by: Marcel Raad
Reviewed-by: Jay Satiro
Closes#9361
Ẃhen it has been used in the multi interface, it is otherwise left in
the connection cache, can't be reused and nothing will close them since
the easy handle loses the association with the multi handle and thus the
connection cache - until the multi handle is closed or it gets pruned
because the cache is full.
Reported-by: Dominik Thalhammer
Fixes#9335Closes#9342
Add licensing and copyright information for all files in this repository. This
either happens in the file itself as a comment header or in the file
`.reuse/dep5`.
This commit also adds a Github workflow to check pull requests and adapts
copyright.pl to the changes.
Closes#8869
Adds two new error codes: CURLE_UNRECOVERABLE_POLL and
CURLM_UNRECOVERABLE_POLL one each for the easy and the multi interfaces.
Reported-by: Harry Sintonen
Fixes#8921Closes#8961
Several years ago a change was made to block user callbacks from calling
back into the API when not supported (recursive calls). One of the calls
blocked was curl_multi_assign. Recently the blocking was extended to the
multi interface API, however curl_multi_assign may need to be called
from within those user callbacks (eg CURLMOPT_SOCKETFUNCTION).
I can't think of any callback where it would be unsafe to call
curl_multi_assign so I removed the restriction entirely.
Reported-by: Michael Wallner
Ref: https://github.com/curl/curl/commit/b46cfbc
Ref: https://github.com/curl/curl/commit/340bb19
Fixes https://github.com/curl/curl/issues/8480
Closes https://github.com/curl/curl/pull/8483
Fix a bug that does not require a new CVE as discussed on hackerone.com.
Previously `connection_id` was accessed after returning connection to
the shared pool.
Bug: https://hackerone.com/reports/1463013Closes#8355
1. The function would only ever return CURLE_OK anyway
2. Only one caller actually used the return code
3. Most callers did (void)Curl_disconnect()
Closes#8303
This makes most libcurl functions return error if called from within a
callback using the same multi handle. For example timer or socket
callbacks calling curl_multi_socket_action.
Reported-by: updatede on github
Fixes#8282Closes#8286
Since each socket hash entry may themselves have a hash table in them,
the destroying of the socket hash needs to make sure all the subhashes
are also correctly destroyed to avoid leaking memory.
Fixes#8129Closes#8131
The callbacks were partially documented to support this. Now the
behavior is documented and returning error from either of these
callbacks will effectively kill all currently ongoing transfers.
Added test 530 to verify
Reported-by: Marcelo Juchem
Fixes#8083Closes#8089
Triggered before a request is made but after a connection is set up
Changes:
- callback: Update docs and callback for pre-request callback
- Add documentation for CURLOPT_PREREQDATA and CURLOPT_PREREQFUNCTION,
- Add redirect test and callback failure test
- Note that the function may be called multiple times on a redirection
- Disable new 2086 test due to Windows weirdness
Closes#7477
The VALID_SOCK() macro was made to only check for FD_SETSIZE if curl was
built to use select(), even though the curl_multi_fdset() function
always and unconditionally uses FD_SET and needs the check.
Reported-by: 0xee on github
Fixes#7718Closes#7719
- the data needs to be "line-based" anyway since it's also passed to the
debug callback/application
- it makes infof() work like failf() and consistency is good
- there's an assert that triggers on newlines in the format string
- Also removes a few instances of "..."
- Removes the code that would append "..." to the end of the data *iff*
it was truncated in infof()
Closes#7357
- Check whether a connection has succeded before checking whether it's
timed out.
This means if we've connected quickly, but subsequently been
descheduled, we allow the connection to succeed. Note, if we timeout,
but between checking the timeout, and connecting to the server the
connection succeeds, we will allow it to go ahead. This is viewed as
an acceptable trade off.
- Add additional failf logging around failed connection attempts to
propogate the cause up to the caller.
Co-Authored-by: Martin Howarth
Closes#7178
scan-build-6 otherwise warns, saying: warning: The left operand of '>='
is a garbage value otherwise, which is false.
Later scan-builds don't claim this on the same code.
Closes#7248
The libssh2 backend has SSH session associated with the connection but
the callback context is the easy handle, so when a connection gets
attached to a transfer, the protocol handler now allows for a custom
function to get used to set things up correctly.
Reported-by: Michael O'Farrell
Fixes#6898Closes#7078
Also added 'CURL_SMALLSENDS' to make Curl_write() send short packets,
which helped verifying this even more.
Add test 363 to verify.
Reported-by: ustcqidi on github
Fixes#6950Closes#7024
Reset FD_WRITE by sending zero bytes which is permissible
and will be treated by implementations as successful send.
Without this we won't be notified in case a socket is still
writable if we already received such a notification and did
not send any data afterwards on the socket. This would lead
to waiting forever on a writable socket being writable again.
Assisted-by: Tommy Odom
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Tested-by: tmkk on github
Bug: #6146Closes#6245
1. Consolidate pre-checks into a single Curl_poll call:
This is an attempt to restructure the code in Curl_multi_wait
in such a way that less syscalls are made by removing individual
calls to Curl_socket_check via SOCKET_READABLE/SOCKET_WRITABLE.
2. Avoid resetting the WinSock event multiple times:
We finally call WSAResetEvent anyway, so specifying it as
an optional parameter to WSAEnumNetworkEvents is redundant.
3. Wakeup directly in case no sockets are being monitoring:
Fix the WinSock based implementation to skip extra waiting by
not sleeping in case no sockets are to be waited on and just
the WinSock event is being monitored for wakeup functionality.
Assisted-by: Tommy Odom
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Bug: #6146Closes#6245
This reverts commit 2260e0ebe6,
also restoring previous follow up changes which were reverted.
Authored-by: rcombs on github
Authored-by: Marc Hörsken
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Restores #5634
Reverts #6281
Part of #6245
The ConnectionExists() function will note that the new transfer wants
less then h2 and that it can't multiplex it and therefor opt to open a
new connection instead.
Storing a stream error in the per-connection struct was an error that lead to
race conditions as subsequent stream handling could overwrite the error code
before it was used for the stream with the actual problem.
Closes#6910
Both were used for the same purposes and there was no logical separation
between them. Combined, this also saves 16 bytes in less holes in my
test build.
Closes#6798
Otherwise libcurl is likely to reuse the connection again in the next
attempt since the connection reuse logic doesn't take downgrades into
account.
Reported-by: Anthony Ramine
Fixes#6788Closes#6793
... since the state machine might go to RATELIMITING and then back to
PERFORMING doing once-per-transfer inits in that function is wrong and
it caused problems with receiving chunked HTTP and it set the
PRETRANSFER time much too often...
Regression from b68dc34af3 (shipped in 7.75.0)
Reported-by: Amaury Denoyelle
Fixes#6640Closes#6641
While working on documenting the states it dawned on me that step one is
to use more descriptive names on the states. This also changes prefix on
the states to make them shorter in the source.
State names NOT ending with *ing are transitional ones.
Closes#6612
The Curl_easy pointer struct entry in connectdata is now gone. Just
before commit 215db086e0 landed on January 8, 2021 there were 919
references to conn->data.
Closes#6608
Rename it to 'httpwant' and make a cloned field in the state struct as
well for run-time updates.
Also: refuse non-supported HTTP versions. Verified with test 129.
Closes#6585
- Reorder some internal struct members so that less padding is used.
This is an attempt at saving a bit of space by packing some structs
(using pahole to find the holes) where it might make sense to do
so without losing readability.
I.e., I tried to avoid separating fields that seem grouped
together (like the cwd... fields in struct ftp_conn for instance).
Also abstained from touching fields behind conditional macros as
that quickly can get complicated.
Closes https://github.com/curl/curl/pull/6483
By making the `magic` identifier the same size and at the same place
within the structs (easy, multi, share), libcurl will be able to more
reliably detect and safely error out if an application passes in the
wrong handle to APIs. Easier to detect and less likely to cause crashes
if done.
Such mixups can't be detected at compile-time due to them being
typedefed void pointers - unless `CURL_STRICTER` is defined.
Closes#6484
... in most cases instead of 'struct connectdata *' but in some cases in
addition to.
- We mostly operate on transfers and not connections.
- We need the transfer handle to log, store data and more. Everything in
libcurl is driven by a transfer (the CURL * in the public API).
- This work clarifies and separates the transfers from the connections
better.
- We should avoid "conn->data". Since individual connections can be used
by many transfers when multiplexing, making sure that conn->data
points to the current and correct transfer at all times is difficult
and has been notoriously error-prone over the years. The goal is to
ultimately remove the conn->data pointer for this reason.
Closes#6425
... instead of at end of the DO state. This makes the timer more
accurate for the protocols that use the DOING state (such as FTP), and
simplifies how the function (now called init_perform) is called.
The timer will then include the entire procedure up to PERFORM -
including all instructions for getting the transfer started.
Closes#6454
When doing HTTP authentication and a port number set with CURLOPT_PORT,
the code would previously have the URL's port number override as if it
had been a redirect to an absolute URL.
Added test 1568 to verify.
Reported-by: UrsusArctos on github
Fixes#6397Closes#6400
When failing in TOOFAST, the multi_done() wasn't called so the same
cleanup and handling wasn't done like when it fails in PERFORM, which in
the case of FTP could mean that the control connection wouldn't be
marked as "dead" for the CURLE_ABORTED_BY_CALLBACK case. Which caused
ftp_disconnect() to use it to send "QUIT", which could end up waiting
for a response a long time before giving up!
Reported-by: Tomas Berger
Fixes#6333Closes#6337
This reverts commit d2a7d7c185.
This commit also reverts the subsequent follow-ups to that commit, which
were all done within windows #ifdefs that are removed in this
change. Marc helped me verify this.
Fixes#6146Closes#6281
Also skip pre-checking sockets to set timeout_ms to 0
after the first socket has been detected to be ready.
Reviewed-by: rcombs on github
Reviewed-by: Daniel Stenberg
Follow up to #5886
Since the struct is quite large (1 long and 10 ints) we
declare it once at the beginning of the function instead
of multiple times inside loops to avoid stack movements.
Reviewed-by: Viktor Szakats
Reviewed-by: Daniel Stenberg
Closes#5886
Learn from the way Cygwin handles and maps the WinSock events
to simulate correct and complete poll and select behaviour
according to Richard W. Stevens Network Programming book.
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Follow up to #5634Closes#5867
Setting a timeout to INT_MAX could cause an immediate error to get
returned as timeout because of an overflow when different values of
'now' were used.
This is primarily fixed by having Curl_pgrsTime() return the "now" when
TIMER_STARTSINGLE is set so that the parent function will continue using
that time.
Reported-by: Ionuț-Francisc Oancea
Fixes#5583Closes#5847
Check readiness of all sockets before waiting on them
to avoid locking in case the one-time event FD_WRITE
was already consumed by a previous wait operation.
More information about WinSock network events:
https://docs.microsoft.com/en-us/windows/win32/api/
winsock2/nf-winsock2-wsaeventselect#return-value
Closes#5634
This avoids using a pair of TCP ports to provide wakeup functionality
for every multi instance on Windows, where socketpair() is emulated
using a TCP socket on loopback which could in turn lead to socket
resource exhaustion.
A previous version of this patch failed to account for how in WinSock,
FD_WRITE is set only once when writing becomes possible and not again
until after a send has failed due to the buffer filling. This contrasts
to how FD_READ and FD_OOB continue to be set until the conditions they
refer to no longer apply. This meant that if a user wrote some data to
a socket, but not enough data to completely fill its send buffer, then
waited on that socket to become writable, we'd erroneously stall until
their configured timeout rather than returning immediately.
This version of the patch addresses that issue by checking each socket
we're waiting on to become writable with select() before the wait, and
zeroing the timeout if it's already writable.
Assisted-by: Marc Hörsken
Reviewed-by: Marcel Raad
Reviewed-by: Daniel Stenberg
Tested-by: Gergely Nagy
Tested-by: Rasmus Melchior Jacobsen
Tested-by: Tomas Berger
Replaces #5397
Reverts #5632Closes#5634
Previously any connect-only connections in a multi handle would be kept
alive until the multi handle was closed. Since these connections cannot
be re-used, they can be marked for closure when the associated easy
handle is removed from the multi handle.
Closes#5749
Follow-up to c4e6968127
When a new transfer is created, as a resuly of an acknowledged push,
that transfer needs a download buffer allocated.
Closes#5590
This avoids using a pair of TCP ports to provide wakeup functionality
for every multi instance on Windows, where socketpair() is emulated
using a TCP socket on loopback which could in turn lead to socket
resource exhaustion.
Reviewed-by: Gergely Nagy
Reviewed-by: Marc Hörsken
Closes#5397
Now that all functions in select.[ch] take timediff_t instead
of the limited int or long, we can remove type conversions
and related preprocessor checks to silence compiler warnings.
Avoiding conversions from time_t was already done in 842f73de.
Based upon #5262
Supersedes #5214, #5220 and #5221
Follow up to #5343 and #5479Closes#5490
... and free it as soon as the transfer is done. It removes the extra
alloc when a new size is set with setopt() and reduces memory for unused
easy handles.
In addition: the closure_handle now doesn't use an allocated buffer at
all but the smallest supported size as a stack based one.
Closes#5472
A common set of functions instead of many separate implementations for
creating buffers that can grow when appending data to them. Existing
functionality has been ported over.
In my early basic testing, the total number of allocations seem at
roughly the same amount as before, possibly a few less.
See docs/DYNBUF.md for a description of the API.
Closes#5300
More connection cache accesses are protected by locks.
CONNCACHE_* is a beter prefix for the connection cache lock macros.
Curl_attach_connnection: now called as soon as there's a connection
struct available and before the connection is added to the connection
cache.
Curl_disconnect: now assumes that the connection is already removed from
the connection cache.
Ref: #4915Closes#5009
- If an easy handle is owned by a multi different from the one specified
then return CURLM_BAD_EASY_HANDLE.
Prior to this change I assume user error could cause corruption.
Closes https://github.com/curl/curl/pull/5116
- Don't check errno on wakeup socket if sread returned 0 since sread
doesn't set errno in that case.
This is a follow-up to cf7760a from several days ago which fixed
Curl_multi_wait to stop busy looping sread on the non-blocking wakeup
socket if it was closed (ie sread returns 0). Due to a logic error it
was still possible to busy loop in that case if errno == EINTR.
Closes https://github.com/curl/curl/pull/5047
- Do not say that conn->data is "cleared" by multi_done().
If the connection is in use then multi_done assigns another easy handle
still using the connection to conn->data, therefore in that case it is
not cleared.
Closes https://github.com/curl/curl/pull/4901
... since the current transfer is being killed. Setting to NULL is
wrong, leaving it pointing to 'data' is wrong since that handle might be
about to get freed.
Fixes#4845Closes#4858
Reported-by: dmitrmax on github
Previously it was stored in a global state which contributed to
curl_global_init's thread unsafety. This boolean is now instead figured
out in curl_multi_init() and stored in the multi handle. Less effective,
but thread safe.
Closes#4851
A regression made the code use 'multiplexed' as a boolean instead of the
counter it is intended to be. This made curl try to "over-populate"
connections with new streams.
This regression came with 41fcdf71a1, shipped in curl 7.65.0.
Also, respect the CURLMOPT_MAX_CONCURRENT_STREAMS value in the same
check.
Reported-by: Kunal Ekawde
Fixes#4779Closes#4784
