Removed wrong assert()s
The 'conn' passed in as userdata can be used and there can be other
sessionhandles ('data') than the single one this checked for.
introduced in c6aedf680f. It needs to be CURLM_STATE_LAST big since it
must hande the range 0 .. CURLM_STATE_MSGSENT (18) and CURLM_STATE_LAST
is 19 right now.
Reported-by: Dan Fandrich
Bug: http://curl.haxx.se/mail/lib-2015-10/0069.html
... and assign it from the set.fread_func_set pointer in the
Curl_init_CONNECT function. This A) avoids that we have code that
assigns fields in the 'set' struct (which we always knew was bad) and
more importantly B) it makes it impossibly to accidentally leave the
wrong value for when the handle is re-used etc.
Introducing a state-init functionality in multi.c, so that we can set a
specific function to get called when we enter a state. The
Curl_init_CONNECT is thus called when switching to the CONNECT state.
Bug: https://github.com/bagder/curl/issues/346Closes#346
sk_X509_pop will decrease the size of the stack which means that the loop would
end after having added only half of the certificates.
Also make sure that the X509 certificate is freed in case
SSL_CTX_add_extra_chain_cert fails.
- If a CURLINFO option is unknown return CURLE_UNKNOWN_OPTION.
Prior to this change CURLE_BAD_FUNCTION_ARGUMENT was returned on
unknown. That return value is contradicted by the CURLINFO option
documentation which specifies a return of CURLE_UNKNOWN_OPTION on
unknown.
- Change algorithm init to happen after OpenSSL config load.
Additional algorithms may be available due to the user's config so we
initialize the algorithms after the user's config is loaded.
Bug: https://github.com/bagder/curl/issues/447
Reported-by: Denis Feklushkin
For a single-stream download from localhost, we managed to increase
transfer speed from 1.6MB/sec to around 400MB/sec, mostly because of
this single fix.
... only call it when there is data arriving for another handle than the
one that is currently driving it.
Improves single-stream download performance quite a lot.
Thanks-to: Tatsuhiro Tsujikawa
Bug: http://curl.haxx.se/mail/lib-2015-09/0097.html
If GnuTLS fails to read the certificate then include whatever reason it
provides in the failure message reported to the client.
Signed-off-by: Mike Crowe <mac@mcrowe.com>
The gnutls vtls back-end was previously ignoring any password set via
CURLOPT_KEYPASSWD. Presumably this was because
gnutls_certificate_set_x509_key_file did not support encrypted keys.
gnutls now has a gnutls_certificate_set_x509_key_file2 function that
does support encrypted keys. Let's determine at compile time whether the
available gnutls supports this new function. If it does then use it to
pass the password. If it does not then emit a helpful diagnostic if a
password is set. This is preferable to the previous behaviour of just
failing to read the certificate without giving a reason in that case.
Signed-off-by: Mike Crowe <mac@mcrowe.com>
... even for those that don't support providing anything in the
'internals' struct member since it offers a convenient way for
applications to figure this out.
- Change the designator name we use to show the base64 encoded sha256
hash of the server's public key from 'pinnedpubkey' to
'public key hash'.
Though the server's public key hash is only shown when comparing pinned
public key hashes, the server's hash may not match one of the pinned.
Without this workaround, NSS re-uses a session cache entry despite the
server name does not match. This causes SNI host name to differ from
the actual host name. Consequently, certain servers (e.g. github.com)
respond by 400 to such requests.
Bug: https://bugzilla.mozilla.org/1202264
If the port number in the proxy string ended weirdly or the number is
too large, skip it. Mostly as a means to bail out early if a "bare" IPv6
numerical address is used without enclosing brackets.
Also mention the bracket requirement for IPv6 numerical addresses to the
man page for CURLOPT_PROXY.
Closes#415
Reported-by: Marcel Raad
In some timing-dependnt cases when a 4xx response immediately followed
after a 150 when a STOR was issued, this function would wrongly return
'complete == true' while 'wait_data_conn' was still set.
Closes#405
Reported-by: Patricia Muscalu
RFC 7540 section 8.1.2.2 states: "An endpoint MUST NOT generate an
HTTP/2 message containing connection-specific header fields; any message
containing connection-specific header fields MUST be treated as
malformed"
Closes#401
Introduced in commit 59f3f92ba6 this function is only implemented when
CURL_DISABLE_CRYPTO_AUTH is not defined. As such we shouldn't define
the function in the header file either.
This patch addresses known bug #76, where on 64-bit Windows SOCKET is 64
bits wide, but long is only 32, making CURLINFO_LASTSOCKET unreliable.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
- Add new option CURLOPT_DEFAULT_PROTOCOL to allow specifying a default
protocol for schemeless URLs.
- Add new tool option --proto-default to expose
CURLOPT_DEFAULT_PROTOCOL.
In the case of schemeless URLs libcurl will behave in this way:
When the option is used libcurl will use the supplied default.
When the option is not used, libcurl will follow its usual plan of
guessing from the hostname and falling back to 'http'.
If strict certificate checking is disabled (CURLOPT_SSL_VERIFYPEER
and CURLOPT_SSL_VERIFYHOST are disabled) do not fail if the server
doesn't present a certificate at all.
Closes#392
The multi state machine would otherwise go into the DO_MORE state after
DO, even for the case when the FTP state machine had already performed
those duties, which caused libcurl to get stuck in that state and fail
miserably. This occured for for active ftp uploads.
Reported-by: Patricia Muscalu
Visual Studio complains with a message box:
"Run-Time Check Failure #1 - A cast to a smaller data type has caused a
loss of data. If this was intentional, you should mask the source of
the cast with the appropriate bitmask.
For example:
char c = (i & 0xFF);
Changing the code in this way will not affect the quality of the
resulting optimized code."
This is because only 'val' is cast to unsigned char, so the "& 0xff" has
no effect.
Closes#387
Return 0 instead of NGHTTP2_ERR_CALLBACK_FAILURE if we can't locate the
SessionHandle. Apparently mod_h2 will sometimes send a frame for a
stream_id we're finished with.
Use nghttp2_session_get_stream_user_data and
nghttp2_session_set_stream_user_data to identify SessionHandles instead
of a hash.
Closes#372
Currently when the server responds with 401 on NTLM authenticated
connection (re-used) we consider it to have failed. However this is
legitimate and may happen when for example IIS is set configured to
'authPersistSingleRequest' or when the request goes thru a proxy (with
'via' header).
Implemented by imploying an additional state once a connection is
re-used to indicate that if we receive 401 we need to restart
authentication.
Closes#363
The SSH state machine didn't clear the 'rc' variable appropriately in a
two places which prevented it from looping the way it should. And it
lacked an 'else' statement that made it possible to erroneously get
stuck in the SSH_AUTH_AGENT state.
Reported-by: Tim Stack
Closes#357
connect.c:953:5: warning: initializer element is not computable at load
time
connect.c:953:5: warning: missing initializer for field 'dwMinorVersion'
of 'OSVERSIONINFOEX'
curl_sspi.c:97:5: warning: initializer element is not computable at load
time
curl_sspi.c:97:5: warning: missing initializer for field 'szCSDVersion'
of 'OSVERSIONINFOEX'
Otherwise it would never be called for an HTTP/2 connection, which has
its own disconnect handler.
I spotted this while debugging <https://bugzilla.redhat.com/1248389>
where the http_disconnect() handler was called on an FTP session handle
causing 'dnf' to crash. conn->data->req.protop of type (struct FTP *)
was reinterpreted as type (struct HTTP *) which resulted in SIGSEGV in
Curl_add_buffer_free() after printing the "Connection cache is full,
closing the oldest one." message.
A previously working version of libcurl started to crash after it was
recompiled with the HTTP/2 support despite the HTTP/2 protocol was not
actually used. This commit makes it work again although I suspect the
root cause (reinterpreting session handle data of incompatible protocol)
still has to be fixed. Otherwise the same will happen when mixing FTP
and HTTP/2 connections and exceeding the connection cache limit.
Reported-by: Tomas Tomecek
Bug: https://bugzilla.redhat.com/1248389
Currently, libcurl rejects responses with "Content-Encoding: compress"
when CURLOPT_ACCEPT_ENCODING is set to "". I think that libcurl should
treat the Content-Encoding "compress" the same as other
Content-Encodings that it does not support, e.g. "bzip2". That means
just ignoring it.
MSVC 12 complains:
lib\vtls\openssl.c(1554): warning C4701: potentially uninitialized local
variable 'verstr' used It's a false positive, but as it's normally not,
I have enabled warning-as-error for that warning.
New tool option --ssl-no-revoke.
New value CURLSSLOPT_NO_REVOKE for CURLOPT_SSL_OPTIONS.
Currently this option applies only to WinSSL where we have automatic
certificate revocation checking by default. According to the
ssl-compared chart there are other backends that have automatic checking
(NSS, wolfSSL and DarwinSSL) so we could possibly accommodate them at
some later point.
Bug: https://github.com/bagder/curl/issues/264
Reported-by: zenden2k <zenden2k@gmail.com>
Static analysis indicated that my commit 9008f3d564 ("ntlm_wb: Fix
hard-coded limit on NTLM auth packet size") introduced a potential
memory leak on an error path, because we forget to free the buffer
before returning an error.
Fix this.
Although actually, it never happens in practice because we never *get*
here with state == NTLMSTATE_TYPE1. The state is always zero. That
might want cleaning up in a separate patch.
Reported-by: Terri Oda
setup-vms.h: More symbols for SHA256, hacks for older VAX
openssl.h: Use OpenSSL OPENSSL_NO_SHA256 macro to allow building on VAX.
openssl.c: Use OpenSSL version checks and OPENSSL_NO_SHA256 macro to
allow building on VAX and 64 bit VMS.
setup-vms.h: Symbol case fixups submitted by Michael Steve
build_gnv_curl_pcsi_desc.com: VSI aka as VMS Software, is now the
supplier of new versions of VMS. The install kit needs to accept
VSI as a producer.
Since we do prefix match using given header by application code
against header name pair in format "NAME:VALUE", and VALUE part can
contain ":", we have to careful about existence of ":" in header
parameter. ":" should be allowed to match HTTP/2 pseudo-header field,
and other use of ":" in header must be treated as error, and
curl_pushheader_byname should return NULL. This commit implements
this behaviour.
In 3013bb6 I had changed cookie export to ignore any-domain cookies,
however the logic I used to do so was incorrect, and would lead to a
busy loop in the case of exporting a cookie list that contained
any-domain cookies. The result of that is worse though, because in that
case the other cookies would not be written resulting in an empty file
once the application is terminated to stop the busy loop.
Make sure that the error buffer is always initialized and simplify the
use of it to make the logic easier.
Bug: https://github.com/bagder/curl/issues/318
Reported-by: sneis
The symbol SSL3_MT_NEWSESSION_TICKET appears to have been introduced at
around openssl 0.9.8f, and the use of it in lib/vtls/openssl.c breaks
builds with older openssls (certainly with 0.9.8b, which is the latest
older version I have to try with).
** WORK-AROUND **
The introduced non-blocking general behaviour for Curl_proxyCONNECT()
didn't work for the data connection establishment unless it was very
fast. The newly introduced function argument makes it operate in a more
blocking manner, more like it used to work in the past. This blocking
approach is only used when the FTP data connecting through HTTP proxy.
Blocking like this is bad. A better fix would make it work more
asynchronously.
Bug: https://github.com/bagder/curl/issues/278
This commit is several drafts squashed together. The changes from each
draft are noted below. If any changes are similar and possibly
contradictory the change in the latest draft takes precedence.
Bug: https://github.com/bagder/curl/issues/244
Reported-by: Chris Araman
%%
%% Draft 1
%%
- return 0 if len == 0. that will have to be documented.
- continue on and process the caches regardless of raw recv
- if decrypted data will be returned then set the error code to CURLE_OK
and return its count
- if decrypted data will not be returned and the connection has closed
(eg nread == 0) then return 0 and CURLE_OK
- if decrypted data will not be returned and the connection *hasn't*
closed then set the error code to CURLE_AGAIN --only if an error code
isn't already set-- and return -1
- narrow the Win2k workaround to only Win2k
%%
%% Draft 2
%%
- Trying out a change in flow to handle corner cases.
%%
%% Draft 3
%%
- Back out the lazier decryption change made in draft2.
%%
%% Draft 4
%%
- Some formatting and branching changes
- Decrypt all encrypted cached data when len == 0
- Save connection closed state
- Change special Win2k check to use connection closed state
%%
%% Draft 5
%%
- Default to CURLE_AGAIN in cleanup if an error code wasn't set and the
connection isn't closed.
%%
%% Draft 6
%%
- Save the last error only if it is an unrecoverable error.
Prior to this I saved the last error state in all cases; unfortunately
the logic to cover that in all cases would lead to some muddle and I'm
concerned that could then lead to a bug in the future so I've replaced
it by only recording an unrecoverable error and that state will persist.
- Do not recurse on renegotiation.
Instead we'll continue on to process any trailing encrypted data
received during the renegotiation only.
- Move the err checks in cleanup after the check for decrypted data.
In either case decrypted data is always returned but I think it's easier
to understand when those err checks come after the decrypted data check.
%%
%% Draft 7
%%
- Regardless of len value go directly to cleanup if there is an
unrecoverable error or a close_notify was already received. Prior to
this change we only acknowledged those two states if len != 0.
- Fix a bug in connection closed behavior: Set the error state in the
cleanup, because we don't know for sure it's an error until that time.
- (Related to above) In the case the connection is closed go "greedy"
with the decryption to make sure all remaining encrypted data has been
decrypted even if it is not needed at that time by the caller. This is
necessary because we can only tell if the connection closed gracefully
(close_notify) once all encrypted data has been decrypted.
- Do not renegotiate when an unrecoverable error is pending.
%%
%% Draft 8
%%
- Don't show 'server closed the connection' info message twice.
- Show an info message if server closed abruptly (missing close_notify).
Some servers will request a client certificate, but not require one.
This change allows libcurl to connect to such servers when using
schannel as its ssl/tls backend. When a server requests a client
certificate, libcurl will now continue the handshake without one,
rather than terminating the handshake. The server can then decide
if that is acceptable or not. Prior to this change, libcurl would
terminate the handshake, reporting a SEC_I_INCOMPLETE_CREDENTIALS
error.
and a conversion to markdown. Removed the lib/README.* files. The idea
being to move toward having INTERNALS as the one and only "book" of
internals documentation.
Added a TOC to top of the document.
When CURL_SOCKET_BAD is returned in the callback, it should be treated
as an error (CURLE_COULDNT_CONNECT) if no other socket is subsequently
created when trying to connect to a server.
Bug: http://curl.haxx.se/mail/lib-2015-06/0047.html
- Try building a chain using issuers in the trusted store first to avoid
problems with server-sent legacy intermediates.
Prior to this change server-sent legacy intermediates with missing
legacy issuers would cause verification to fail even if the client's CA
bundle contained a valid replacement for the intermediate and an
alternate chain could be constructed that would verify successfully.
https://rt.openssl.org/Ticket/Display.html?id=3621&user=guest&pass=guest
Prior to this change any-domain cookies (cookies without a domain that
are sent to any domain) were exported with domain name "unknown".
Bug: https://github.com/bagder/curl/issues/292
Follow-up to e8423f9ce1 with discussionis in
https://github.com/bagder/curl/pull/258
This check scans for fopen() with a mode string without 'b' present, as
it may indicate that an FOPEN_* define should rather be used.
- Change fopen calls to use FOPEN_READTEXT instead of "r" or "rt"
- Change fopen calls to use FOPEN_WRITETEXT instead of "w" or "wt"
This change is to explicitly specify when we need to read/write text.
Unfortunately 't' is not part of POSIX fopen so we can't specify it
directly. Instead we now have FOPEN_READTEXT, FOPEN_WRITETEXT.
Prior to this change we had an issue on Windows if an application that
uses libcurl overrides the default file mode to binary. The default file
mode in Windows is normally text mode (translation mode) and that's what
libcurl expects.
Bug: https://github.com/bagder/curl/pull/258#issuecomment-107093055
Reported-by: Orgad Shaneh
Previously, after seeing upgrade to HTTP/2, we feed data followed by
upgrade response headers directly to nghttp2_session_mem_recv() in
Curl_http2_switched(). But it turns out that passed buffer, mem, is
part of stream->mem, and callbacks called by
nghttp2_session_mem_recv() will write stream specific data into
stream->mem, overwriting input data. This will corrupt input, and
most likely frame length error is detected by nghttp2 library. The
fix is first copy the passed data to HTTP/2 connection buffer,
httpc->inbuf, and call nghttp2_session_mem_recv().
Coverity CID 1299424 identified dead code because of checks that could
never equal true (if the mechanism's name was NULL).
Simplified the function by removing a level of pointers and removing the
loop and array that weren't used.
Replace use of assert with code that properly catches bad input at
run-time even in non-debug builds.
This flaw was sort of detected by Coverity CID 1299425 which claimed the
"case RTSPREQ_NONE" was dead code.
Coverity CID 1299426 warned about possible NULL dereference otherwise,
but that would only ever happen if we get invalid HTTP/2 data with
frames for stream 0. Avoid this risk by returning early when stream 0 is
used.
Prior to this change the description for SEC_E_ILLEGAL_MESSAGE was OS
and language specific, and invariably translated to something not very
helpful like: "The message received was unexpected or badly formatted."
Bug: https://github.com/bagder/curl/issues/267
Reported-by: Michael Osipov
With many easy handles using the same connection for multiplexing, it is
important we store and keep the transfer-oriented stuff in the
SessionHandle so that callbacks and callback data work fine even when
many easy handles share the same physical connection.
Previously, when we send all given buffer in data_source_callback, we
return NGHTTP2_ERR_DEFERRED, and nghttp2 library removes this stream
temporarily for writing. This itself is good. If this is the sole
stream in the session, nghttp2_session_want_write() returns zero,
which means that libcurl does not check writeability of the underlying
socket. This leads to very slow upload, because it seems curl only
upload 16k something per 1 second. To fix this, if we still have data
to send, call nghttp2_session_resume_data after nghttp2_session_send.
This makes nghttp2_session_want_write() returns nonzero (if connection
window still opens), and as a result, socket writeability is checked,
and upload speed becomes normal.
