Since it can't be NULL and it makes Coverity believe we lack proper NULL
checks. Verified by test 659, landed in commit 15401fa886.
Pointed out by Coverity CID 1442746.
Assisted-by: Dan Fandrich
Fixes#3617Closes#3642
If the state machine isn't complete, didn't fail and it didn't return
due to blocking it can just as well loop again.
This addresses the problem with SFTP directory listings where we would
otherwise return back to the parent and as the multi state machine
doesn't have any code for using CURLM_CALL_MULTI_PERFORM for as long the
doing phase isn't complete, it would return out when in reality there
was more data to deal with.
Fixes#3506Closes#3644
- Change closure handle to receive verbose setting from the easy handle
most recently added via curl_multi_add_handle.
The closure handle is a special easy handle used for closing cached
connections. It receives limited settings from the easy handle most
recently added to the multi handle. Prior to this change that did not
include verbose which was a problem because on connection shutdown
verbose mode was not acknowledged.
Ref: https://github.com/curl/curl/pull/3598
Co-authored-by: Daniel Stenberg
Closes https://github.com/curl/curl/pull/3618
- no need to have them protocol specific
- no need to set pointers to them with the Curl_setup_transfer() call
- make Curl_setup_transfer() operate on a transfer pointer, not
connection
- switch some counters from long to the more proper curl_off_t type
Closes#3627
When a transfer is done, the resolver thread will be brought down. That
could accidentally generate an error message in the error buffer even
though this is not an error situationand the transfer would still return
OK. An application that still reads the error buffer could find a
"Could not resolve host: [host name]" message there and get confused.
Reported-by: Michael Schmid
Fixes#3629Closes#3630
Without it set, we would unwillingly triger the "HTTP error before end
of send, stop sending" condition even if the entire POST body had been
sent (since it wouldn't know the expected size) which would
unnecessarily log that message and close the connection when it didn't
have to.
Reported-by: Matt McClure
Bug: https://curl.haxx.se/mail/archive-2019-02/0023.htmlCloses#3624
Follow-up to 8eddb8f425.
If the cookieinfo pointer is NULL there really is nothing to save.
Without this fix, we got a problem when a handle was using shared object
with cookies and is told to "FLUSH" it to file (which worked) and then
the share object was removed and when the easy handle was closed just
afterwards it has no cookieinfo and no cookies so it decided to save an
empty jar (overwriting the file just flushed).
Test 1905 now verifies that this works.
Assisted-by: Michael Wallner
Assisted-by: Marcel Raad
Closes#3621
This allows the compiler to pack and align the structs better in
memory. For a rather feature-complete build on x86_64 Linux, gcc 8.1.2
makes the Curl_easy struct 4.9% smaller. From 6312 bytes to 6000.
Removed an unused struct field.
No functionality changes.
Closes#3610
Instead of using a fixed 256 byte buffer in the connectdata struct.
In my build, this reduces the size of the connectdata struct by 11.8%,
from 2160 to 1904 bytes with no functionality or performance loss.
This also fixes a bug in schannel's Curl_verify_certificate where it
called Curl_sspi_strerror when it should have called Curl_strerror for
string from GetLastError. the only effect would have been no text or the
wrong text being shown for the error.
Co-authored-by: Jay Satiro
Closes#3612
Add support for Ephemeral elliptic curve Diffie-Hellman key exchange
algorithm option when selecting ciphers. This became available on the
Win10 SDK.
Closes https://github.com/curl/curl/pull/3608
Failing to do so would make the CURLINFO_TOTAL_TIME timeout to not get
updated correctly and could end up getting reported to the application
completely wrong (way too small).
Reported-by: accountantM on github
Fixes#3602Closes#3605
- Split off connection shutdown procedure from Curl_disconnect into new
function conn_shutdown.
- Change the shutdown procedure to close the sockets before
disassociating the transfer.
Prior to this change the sockets were closed after disassociating the
transfer so SOCKETFUNCTION wasn't called since the transfer was already
disassociated. That likely came about from recent work started in
Jan 2019 (#3442) to separate transfers from connections.
Bug: https://curl.haxx.se/mail/lib-2019-02/0101.html
Reported-by: Pavel Löbl
Closes https://github.com/curl/curl/issues/3597
Closes https://github.com/curl/curl/pull/3598
RFC 7540 says we should verify that the push is for an "authoritative"
server. We make sure of this by only allowing push with an :athority
header that matches the host that was asked for in the URL.
Fixes#3577
Reported-by: Nicolas Grekas
Bug: https://curl.haxx.se/mail/lib-2019-02/0057.htmlCloses#3581
The variable wasn't properly reset within the loop and thus could remain
set for sockets that hadn't been set before and miss notifying the app.
This is a follow-up to 4c35574 (shipped in curl 7.64.0)
Reported-by: buzo-ffm on github
Detected-by: Jan Alexander Steffens
Fixes#3585Closes#3589
- rename 'n' to buflen in functions, and use size_t for them. Don't pass
in negative buffer lengths.
- move most function comments to above the function starts like we use
to
- remove several unnecessary typecasts (especially of NULL)
Reviewed-by: Patrick Monnerat
Closes#3582
Previously the function would edit the provided header in-place when a
semicolon is used to signify an empty header. This made it impossible to
use the same set of custom headers in multiple threads simultaneously.
This approach now makes a local copy when it needs to edit the string.
Reported-by: d912e3 on github
Fixes#3578Closes#3579
- Change the behavior of win32_init so that the required initialization
procedures are not affected by CURL_GLOBAL_WIN32 flag.
libcurl via curl_global_init supports initializing for win32 with an
optional flag CURL_GLOBAL_WIN32, which if omitted was meant to stop
Winsock initialization. It did so internally by skipping win32_init()
when that flag was set. Since then win32_init() has been expanded to
include required initialization routines that are separate from
Winsock and therefore must be called in all cases. This commit fixes
it so that CURL_GLOBAL_WIN32 only controls the optional win32
initialization (which is Winsock initialization, according to our doc).
The only users affected by this change are those that don't pass
CURL_GLOBAL_WIN32 to curl_global_init. For them this commit removes the
risk of a potential crash.
Ref: https://github.com/curl/curl/pull/3573
Fixes https://github.com/curl/curl/issues/3313
Closes https://github.com/curl/curl/pull/3575
The draft-ietf-httpbis-rfc6265bis-02 draft, specify a set of prefixes
and how they should affect cookie initialization, which has been
adopted by the major browsers. This adds support for the two prefixes
defined, __Host- and __Secure, and updates the testcase with the
supplied examples from the draft.
Closes#3554
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
If mbedtls_ssl_get_session() fails, it may still have allocated
memory that needs to be freed to avoid leaking. Call the library
API function to release session resources on this errorpath as
well as on Curl_ssl_addsessionid() errors.
Closes: #3574
Reported-by: Michał Antoniak <M.Antoniak@posnet.com>
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
... and avoid use of static variables that aren't thread safe.
Fixes regression from e9ababd4f5 (present in the 7.64.0 release)
Reported-by: Paul Groke
Fixes#3572Closes#3573
- Save the original conn->data before it's changed to the specified
data transfer for the connection check and then restore it afterwards.
This is a follow-up to 38d8e1b 2019-02-11.
History:
It was discovered a month ago that before checking whether to extract a
dead connection that that connection should be associated with a "live"
transfer for the check (ie original conn->data ignored and set to the
passed in data). A fix was landed in 54b201b which did that and also
cleared conn->data after the check. The original conn->data was not
restored, so presumably it was thought that a valid conn->data was no
longer needed.
Several days later it was discovered that a valid conn->data was needed
after the check and follow-up fix was landed in bbae24c which partially
reverted the original fix and attempted to limit the scope of when
conn->data was changed to only when pruning dead connections. In that
case conn->data was not cleared and the original conn->data not
restored.
A month later it was discovered that the original fix was somewhat
correct; a "live" transfer is needed for the check in all cases
because original conn->data could be null which could cause a bad deref
at arbitrary points in the check. A fix was landed in 38d8e1b which
expanded the scope to all cases. conn->data was not cleared and the
original conn->data not restored.
A day later it was discovered that not restoring the original conn->data
may lead to busy loops in applications that use the event interface, and
given this observation it's a pretty safe assumption that there is some
code path that still needs the original conn->data. This commit is the
follow-up fix for that, it restores the original conn->data after the
connection check.
Assisted-by: tholin@users.noreply.github.com
Reported-by: tholin@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/3542Closes#3559
On non-ascii platforms, the chunked hex header was measured for char code
conversion length, even for chunked trailers that do not have an hex header.
In addition, the efective length is already known: use it.
Since the hex length can be zero, only convert if needed.
Reported by valgrind.
Convert numerous infof() calls into debug-build only messages since they
are annoyingly verbose for regular applications. Removed a few.
Bug: https://curl.haxx.se/mail/lib-2019-02/0027.html
Reported-by: Volker Schmid
Closes#3552
There is no benefit to holding the data sharelock when freeing the
addrinfo in case it fails, so ensure releaseing it as soon as we can
rather than holding on to it. This also aligns the code with other
consumers of sharelocks.
Closes#3516
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
The http2 code for connection checking needs a transfer to use. Make
sure a working one is set before handler->connection_check() is called.
Reported-by: jnbr on github
Fixes#3541Closes#3547
urlapi: turn three local-only functions into statics
conncache: make conncache_find_first_connection static
multi: make detach_connnection static
connect: make getaddressinfo static
curl_ntlm_core: make hmac_md5 static
http2: make two functions static
http: make http_setup_conn static
connect: make tcpnodelay static
tests: make UNITTEST a thing to mark functions with, so they can be static for
normal builds and non-static for unit test builds
... and mark Curl_shuffle_addr accordingly.
url: make up_free static
setopt: make vsetopt static
curl_endian: make write32_le static
rtsp: make rtsp_connisdead static
warnless: remove unused functions
memdebug: remove one unused function, made another static
If the incoming len 5, but the buffer does not have a termination
after 5 bytes, the strtol() call may keep reading through the line
buffer until is exceeds its boundary. Fix by ensuring that we are
using a bounded read with a temporary buffer on the stack.
Bug: https://curl.haxx.se/docs/CVE-2019-3823.html
Reported-by: Brian Carpenter (Geeknik Labs)
CVE-2019-3823
Attempt to add support for Secure Channel binding when negotiate
authentication is used. The problem to solve is that by default IIS
accepts channel binding and curl doesn't utilise them. The result was a
401 response. Scope affects only the Schannel(winssl)-SSPI combination.
Fixes https://github.com/curl/curl/issues/3503
Closes https://github.com/curl/curl/pull/3509
Stick to "Schannel" everywhere. The configure option --with-winssl is
kept to allow existing builds to work but --with-schannel is added as an
alias.
Closes#3504
mbedTLS doesn't have a sigpipe management. If a write/read occurs when
the remote closes the socket, the signal is raised and kills the
application. Use the curl mecanisms fix this behavior.
Signed-off-by: Jeremie Rapin <j.rapin@overkiz.com>
Closes#3502
Compiling with msvc /analyze and a recent Windows SDK warns against
using GetTickCount (Suggests to use GetTickCount64 instead.)
Since GetTickCount is only being used when GetTickCount64 isn't
available, I am disabling that warning.
Fixes https://github.com/curl/curl/issues/3437
Closes https://github.com/curl/curl/pull/3440
CURLOPT_SSH_KNOWNHOSTS and CURLOPT_SSH_KEYFUNCTION are supported for
libssh as well. So accepting these options only when compiling with
libssh2 is wrong here.
Fixes#3493Closes#3494
By default, libssh creates a new socket, instead of using the socket
created by curl for SSH connections.
Pass the socket created by curl to libssh using ssh_options_set() with
SSH_OPTIONS_FD directly after ssh_new(). So libssh uses our socket
instead of creating a new one.
This approach is very similar to what is done in the libssh2 code, where
the socket created by curl is passed to libssh2 when
libssh2_session_startup() is called.
Fixes#3491Closes#3495
There is no real gain in performing memcmp() comparisons on single
characters, so change these to array subscript inspections which
saves a call and makes the code clearer.
Closes#3486
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Reviewed-by: Jay Satiro <raysatiro@yahoo.com>
Windows extended potection (aka ssl channel binding) is required
to login to ntlm IIS endpoint, otherwise the server returns 401
responses.
Fixes#3280Closes#3321
When a ssh session startup fails, it is useful to know why it has
failed. This commit changes the message from:
"Failure establishing ssh session"
to something like this, for example:
"Failure establishing ssh session: -5, Unable to exchange encryption keys"
Closes#3481
.... to not pass in a const in the second argument as that's not how it
is supposed to be used and might cause compiler warnings.
Reported-by: Pavel Pavlov
Fixes#3477Closes#3478
extract_if_dead() dead is called from two functions, and only one of
them should get conn->data updated and now neither call path clears it.
scan-build found a case where conn->data would be NULL dereferenced in
ConnectionExists() otherwise.
Closes#3473
Make sure that this function sets a proper "live" transfer for the
connection before calling the protocol-specific connection check
function, and then clear it again afterward as a non-used connection has
no current transfer.
Reported-by: Jeroen Ooms
Reviewed-by: Marcel Raad
Reviewed-by: Daniel Gustafsson
Fixes#3463Closes#3464
We use "conn" everywhere to be a pointer to the connection.
Introduces two functions that "attaches" and "detaches" the connection
to and from the transfer.
Going forward, we should favour using "data->conn" (since a transfer
always only has a single connection or none at all) to "conn->data"
(since a connection can have none, one or many transfers associated with
it and updating conn->data to be correct is error prone and a frequent
reason for internal issues).
Closes#3442
Fixes#3436Closes#3448
Problem 1
After LOTS of scratching my head, I eventually realized that even when doing
10 uploads in parallel, sometimes the socket callback to the application that
tells it what to wait for on the socket, looked like it would reflect the
status of just the single transfer that just changed state.
Digging into the code revealed that this was indeed the truth. When multiple
transfers are using the same connection, the application did not correctly get
the *combined* flags for all transfers which then could make it switch to READ
(only) when in fact most transfers wanted to get told when the socket was
WRITEABLE.
Problem 1b
A separate but related regression had also been introduced by me when I
cleared connection/transfer association better a while ago, as now the logic
couldn't find the connection and see if that was marked as used by more
transfers and then it would also prematurely remove the socket from the socket
hash table even in times other transfers were still using it!
Fix 1
Make sure that each socket stored in the socket hash has a "combined" action
field of what to ask the application to wait for, that is potentially the ORed
action of multiple parallel transfers. And remove that socket hash entry only
if there are no transfers left using it.
Problem 2
The socket hash entry stored an association to a single transfer using that
socket - and when curl_multi_socket_action() was called to tell libcurl about
activities on that specific socket only that transfer was "handled".
This was WRONG, as a single socket/connection can be used by numerous parallel
transfers and not necessarily a single one.
Fix 2
We now store a list of handles in the socket hashtable entry and when libcurl
is told there's traffic for a particular socket, it now iterates over all
known transfers using that single socket.
Added Curl_resolver_kill() for all three resolver modes, which only
blocks when necessary, along with test 1592 to confirm
curl_multi_remove_handle() doesn't block unless it must.
Closes#3428Fixes#3371
When building with Unicode on MSVC, the compiler warns about freeing a
pointer to const in Curl_unicodefree. Fix this by declaring it as
non-const and casting the argument to Curl_convert_UTF8_to_tchar to
non-const too, like we do in all other places.
Closes https://github.com/curl/curl/pull/3435
The previous fix for parsing IPv6 URLs with a zone index was a paddle
short for URLs without an explicit port. This patch fixes that case
and adds a unit test case.
This bug was highlighted by issue #3408, and while it's not the full
fix for the problem there it is an isolated bug that should be fixed
regardless.
Closes#3411
Reported-by: GitYuanQu on github
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
