Commit Graph

644 Commits

Author SHA1 Message Date
Daniel Stenberg 51c0ebcff2
http: deal with partial CONNECT sends
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 #6950
Closes #7024
2021-05-08 10:49:16 +02:00
Daniel Stenberg 063d3f3b96
tidy-up: make conditional checks more consistent
... remove '== NULL' and '!= 0'

Closes #6912
2021-04-22 09:10:17 +02:00
Marc Hoersken b36442b243
multi: fix slow write/upload performance on Windows
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: #6146
Closes #6245
2021-04-21 20:32:24 +02:00
Marc Hoersken e92998a312
multi: reduce Win32 API calls to improve performance
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: #6146
Closes #6245
2021-04-21 20:32:14 +02:00
Marc Hoersken bcca174cfa
Revert "Revert 'multi: implement wait using winsock events'"
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
2021-04-21 20:29:36 +02:00
Daniel Stenberg 9c18c0b4dd
multi: don't close connection HTTP_1_1_REQUIRED
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.
2021-04-21 08:20:24 +02:00
Daniel Stenberg 605e842355
http2: move the stream error field to the per-transfer storage
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
2021-04-21 08:20:24 +02:00
Daniel Stenberg b5726e5549
send_speed: simplify the checks for if a speed limit is set
... as we know the value cannot be set to negative: enforced by
setopt()
2021-03-27 12:38:28 +01:00
Daniel Stenberg 95cbcec8f9
urldata: merge "struct DynamicStatic" into "struct UrlState"
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
2021-03-26 23:19:20 +01:00
Daniel Stenberg 7b6bfd2d78
multi: close the connection when h2=>h1 downgrading
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 #6788
Closes #6793
2021-03-25 16:18:13 +01:00
ejanchivdorj a2bbc3ac8c
multi: update pending list when removing handle
when removing a handle, most of the lists are updated but pending list
is not updated. Updating now.

Closes #6713
2021-03-10 14:05:02 +01:00
Daniel Stenberg 0c7d111f4e
urldata: make 'actions[]' use unsigned char instead of int
... as it only needs a few bits per index anyway.

Reviewed-by: Daniel Gustafsson
Closes #6648
2021-02-24 14:16:11 +01:00
Daniel Stenberg bf601472a0
multi: do once-per-transfer inits in before_perform in DID state
... 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 #6640
Closes #6641
2021-02-22 15:54:46 +01:00
Daniel Stenberg 94719e7285
multi: rename the multi transfer states
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
2021-02-16 16:42:46 +01:00
Daniel Stenberg cfff12a0b3
lib: remove 'conn->data' completely
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
2021-02-16 10:48:57 +01:00
Daniel Stenberg 88dd1a8a11
urldata: don't touch data->set.httpversion at run-time
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
2021-02-12 08:13:37 +01:00
Jacob Hoffman-Andrews 246399a874
vtls: initial implementation of rustls backend
This adds a new TLS backend, rustls. It uses the C-to-rustls bindings
from https://github.com/abetterinternet/crustls.

Rustls is at https://github.com/ctz/rustls/.

There is still a fair bit to be done, like sending CloseNotify on
connection shutdown, respecting CAPATH, and properly indicating features
like "supports TLS 1.3 ciphersuites." But it works well enough to make
requests and receive responses.

Blog post for context:
https://www.abetterinternet.org/post/memory-safe-curl/

Closes #6350
2021-02-09 11:06:18 +01:00
Daniel Stenberg 1dc8aa870e
hostip/proxy: remove conn->data use
Closes #6513
2021-01-26 10:04:47 +01:00
Daniel Stenberg 8335c6417e
hostip: remove conn->data from resolver functions
This also moves the 'async' struct from the connectdata struct into the
Curl_easy struct, which seems like a better home for it.

Closes #6497
2021-01-22 08:25:09 +01:00
Erik Olsson 0a5827571f lib: save a bit of space with some structure packing
- 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
2021-01-20 22:52:34 -05:00
Daniel Stenberg 942cf12c2f
urldata: make magic be the first struct field
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
2021-01-20 08:51:01 +01:00
Daniel Stenberg 219d9f8620
transfer: remove conn->data use
Closes #6486
2021-01-19 17:07:02 +01:00
Daniel Stenberg a304051620
lib: more conn->data cleanups
Closes #6479
2021-01-19 09:14:51 +01:00
Daniel Stenberg 215db086e0
lib: pass in 'struct Curl_easy *' to most functions
... 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
2021-01-17 23:56:09 +01:00
Daniel Stenberg b68dc34af3
multi: set the PRETRANSFER time-stamp when we switch to PERFORM
... 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
2021-01-14 23:16:39 +01:00
Daniel Stenberg 78af707caf
multi: remove Curl_ prefix from static functions 2021-01-14 08:09:09 +01:00
Daniel Stenberg 06f1db57ca
multi_runsingle: bail out early on data->conn == NULL
As that's a significant error condition and scan-build warns for NULL
pointer dereferences if we don't.

Closes #6433
2021-01-11 12:33:23 +01:00
Daniel Stenberg 48220f7e3d
multi: skip DONE state if there's no connection left for ftp wildcard
... to avoid running in that state with data->conn being NULL.
2021-01-11 12:33:11 +01:00
Daniel Stenberg 648712eec1
httpauth: make multi-request auth work with custom port
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 #6397
Closes #6400
2021-01-02 23:40:47 +01:00
Daniel Stenberg 3e17c8ab72
multi: when erroring in TOOFAST state, act as for PERFORM
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 #6333
Closes #6337
2020-12-17 13:57:27 +01:00
Daniel Stenberg 2260e0ebe6
Revert "multi: implement wait using winsock events"
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 #6146
Closes #6281
2020-12-06 22:40:38 +01:00
Daniel Stenberg b6b535994e
splay: rename Curl_splayremovebyaddr to Curl_splayremove
... and remove the old unused proto for the old Curl_splayremove
version.

Closes #6269
2020-12-01 08:09:51 +01:00
Daniel Stenberg 4d2f800677
curl.se: new home
Closes #6172
2020-11-04 23:59:47 +01:00
Daniel Stenberg 475c1aba68
checksrc: warn on empty line before open brace
... and fix a few occurances

Closes #6088
2020-10-15 23:32:26 +02:00
Marc Hoersken 1060955a61
multi: align WinSock mask variables in Curl_multi_wait
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
2020-09-17 07:41:14 +02:00
Marc Hoersken 40d67da7ca
multi: reuse WinSock events variable in Curl_multi_wait
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
2020-09-17 07:40:49 +02:00
Daniel Stenberg 9e90ff0839
hash: make it 'struct Curl_hash'
As internal global names should use captical C.

Closes #5906
2020-09-02 22:41:58 +02:00
Daniel Stenberg 9b3f888a00
llist: make it "struct Curl_llist"
As internal global names should use captical C.

Closes #5906
2020-09-02 22:41:58 +02:00
Marc Hoersken 003e81e2a3
multi: handle connection state winsock events
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 #5634
Closes #5867
2020-08-28 17:57:52 +02:00
Daniel Stenberg a2c85bb8e4
Curl_pgrsTime - return new time to avoid timeout integer overflow
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 #5583
Closes #5847
2020-08-28 14:16:41 +02:00
Marc Hoersken 3334ee6bcb
multi: expand pre-check for socket readiness
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
2020-08-25 12:11:45 +02:00
rcombs d2a7d7c185
multi: implement wait using winsock events
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 #5632
Closes #5634
2020-08-25 12:11:24 +02:00
Daniel Stenberg 3c9e021f86
Curl_easy: remember last connection by id, not by pointer
CVE-2020-8231

Bug: https://curl.haxx.se/docs/CVE-2020-8231.html

Reported-by: Marc Aldorasi
Closes #5824
2020-08-17 14:33:09 +02:00
Emil Engler da901fefe0
multi: Remove 10-year old out-commented code
The code hasn't been touched since 2010-08-18

Closes #5805
2020-08-12 15:28:46 +02:00
Daniel Stenberg ca567dc5a4
multi: Condition 'extrawait' is always true
Reported by Codacy.

Reviewed-by: Marcel Raad
Closes #5759
2020-08-01 23:21:29 +02:00
Marc Aldorasi d5bb459ccf
multi_remove_handle: close unused connect-only connections
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
2020-08-01 15:26:08 +02:00
Daniel Stenberg abe59221fc
curl_multi_setopt: fix compiler warning "result is always false"
On systems with 32 bit long the expression is always false. Avoid
the warning.

Reported-by: Gisle Vanem
Bug: 61a08508f6 (commitcomment-40941232)
Closes #5736
2020-07-28 23:37:01 +02:00
Daniel Stenberg 61a08508f6
multi: remove two checks always true
Detected by Codacy
Closes #5676
2020-07-14 00:12:08 +02:00
Daniel Stenberg 25222b14ef
Revert "multi: implement wait using winsock events"
This reverts commit 8bc25c590e.

That commit (from #5397) introduced a regression in 7.71.0.

Reported-by: tmkk on github
Fixes #5631
Closes #5632
2020-06-30 12:27:23 +02:00
Daniel Stenberg ff1c37cbe3
url: make sure pushed streams get an allocated download buffer
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
2020-06-23 15:13:27 +02:00