Commit Graph

306 Commits

Author SHA1 Message Date
Daniel Stenberg 252790c533
http2: call the handle-closed function correctly on closed stream
This was this one condition where the stream could be closed due to an
error and the function would still wrongly just return 0 for it.

Reported-by: Gergely Nagy
Fixes #6862
Closes #6910
2021-04-21 08:20:24 +02: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
Robert Ronto 068661bb02
http2: don't set KEEP_SEND when there's no more data to be sent
this should fix an issue where curl sometimes doesn't send out a request
with authorization info after a 401 is received over http2

Closes #6747
2021-03-15 23:29:35 +01:00
oxalica d1f40078c1
http2: fail if connection terminated without END_STREAM
Closes #6736
2021-03-12 17:54:39 +01:00
Daniel Stenberg 723c0e75fa
http2: remove conn->data use
... but instead use a private alternative that points to the "driving
transfer" from the connection. We set the "user data" associated with
the connection to be the connectdata struct, but when we drive transfers
the code still needs to know the pointer to the transfer. We can change
the user data to become the Curl_easy handle, but with older nghttp2
version we cannot dynamically update that pointer properly when
different transfers are used over the same connection.

Closes #6520
2021-02-15 16:33:53 +01:00
Daniel Stenberg 1c3def53c7
urldata: remove duplicate 'upkeep_interval_ms' from connectdata
... and rely only on the value already set in Curl_easy.

Closes #6534
2021-01-27 09:19:08 +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 aba01da639
failf: remove newline from formatting strings
... as failf adds one itself.

Also: add an assert() to failf() that triggers on a newline in the
format string!

Closes #6365
2020-12-25 23:42:16 +01:00
Daniel Stenberg c7f95fa0cc
h2: do not wait for RECV on paused transfers
... as the socket might be readable all the time when paused and thus
causing a busy-loop.

Reported-by: Harry Sintonen
Reviewed-by: Jay Satiro
Fixes #6356
Closes #6357
2020-12-22 12:44:18 +01:00
Rikard Falkeborn 920f49a20b
infof/failf calls: fix format specifiers
Update a few format specifiers to match what is being printed.

Closes #6241
2020-11-24 13:18:41 +01:00
Daniel Stenberg a95a6ce6b8
urldata: remove 'void *protop' and create the union 'p'
... to avoid the use of 'void *' for the protocol specific structs done
per transfer.

Closes #6238
2020-11-23 16:16:16 +01:00
Daniel Stenberg 4d2f800677
curl.se: new home
Closes #6172
2020-11-04 23:59:47 +01:00
Daniel Stenberg d70a5b5a0f
sendf: move the verbose-check into Curl_debug
Saves us from having the same check done everywhere.

Closes #6159
2020-11-02 22:49:36 +01:00
Daniel Stenberg 6434a73984
Curl_handler: add 'family' to each protocol
Makes get_protocol_family() faster and it moves the knowledge about the
"families" to each protocol handler, where it belongs.

Closes #5986
2020-09-21 23:30:16 +02:00
Laramie Leavitt 25a25f45ae
http: consolidate nghttp2_session_mem_recv() call paths
Previously there were several locations that called
nghttp2_session_mem_recv and handled responses slightly differently.
Those have been converted to call the existing
h2_process_pending_input() function.

Moved the end-of-session check to h2_process_pending_input() since the
only place the end-of-session state can change is after nghttp2
processes additional input frames.

This will likely fix the fuzzing error. While I don't have a root cause
the out-of-bounds read seems like a use after free, so moving the
nghttp2_session_check_request_allowed() call to a location with a
guaranteed nghttp2 session seems reasonable.

Also updated a few nghttp2 callsites to include error messages and added
a few additional error checks.

Closes #5648
2020-09-10 17:43:47 +02:00
Daniel Stenberg 7f187d897c
h2: repair trailer handling
The previous h2 trailer fix in 54a2b63 was wrong and caused a
regression: it cannot deal with trailers immediately when read since
they may be read off the connection by the wrong 'data' owner.

This change reverts the logic back to gathering all trailers into a
single buffer, like before 54a2b63.

Reported-by: Tadej Vengust
Fixes #5663
Closes #5769
2020-08-03 23:43:24 +02:00
Daniel Stenberg 954cd3eb48
CURL_PUSH_ERROROUT: allow the push callback to fail the parent stream
... by adding support for a new dedicated return code.

Suggested-by: Jonathan Cardoso
Assisted-by: Erik Johansson
URL: https://curl.haxx.se/mail/lib-2020-06/0099.html
Closes #5636
2020-07-16 00:24:29 +02:00
Daniel Stenberg 69bfde4515
http2: only do the *done() cleanups for HTTP
Follow-up to ef86daf4d3

Closes #5650
Fixes #5646
2020-07-04 01:13:50 +02:00
Laramie Leavitt ef86daf4d3
http2: close the http2 connection when no more requests may be sent
Well-behaving HTTP2 servers send two GOAWAY messages. The first
message is a warning that indicates that the server is going to
stop accepting streams. The second one actually closes the stream.

nghttp2 reports this state (and the other state of no more stream
identifiers) via the call nghttp2_session_check_request_allowed().
In this state the client should not create more streams on the
session (tcp connection), and in curl this means that the server
has requested that the connection is closed.

It would be also be possible to put the connclose() call into the
on_http2_frame_recv() function that triggers on the GOAWAY message.

This fixes a bug seen when the client sees the following sequence of
frames:

// advisory GOAWAY
HTTP2 GOAWAY [stream-id = 0, promised-stream-id = -1]
... some additional frames

// final GOAWAY
HTTP2 GOAWAY [stream-id = 0, promised-stream-id = N ]

Before this change, curl will attempt to reuse the connection even
after the last stream, will encounter this error:

* Found bundle for host localhost: 0x5595f0a694e0 [can multiplex]
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (::1) port 10443 (#0)
* Using Stream ID: 9 (easy handle 0x5595f0a72e30)
> GET /index.html?5 HTTP/2
> Host: localhost:10443
> user-agent: curl/7.68.0
> accept: */*
>
* stopped the pause stream!
* Connection #0 to host localhost left intact
curl: (16) Error in the HTTP2 framing layer

This error may posion the connection cache, causing future requests
which resolve to the same curl connection to go through the same error
path.

Closes #5643
2020-07-03 11:16:48 +02:00
Jeremy Maitin-Shepard 31f0e864c7
http2: fix nghttp2_strerror -> nghttp2_http2_strerror in debug messages
Confusingly, nghttp2 has two different error code enums:

- nghttp2_error, to be used with nghttp2_strerror
- nghttp2_error_code, to be used with nghttp2_http2_strerror

Closes #5641
2020-07-02 23:37:28 +02:00
Daniel Stenberg 646cc574f0
http2: set the correct URL in pushed transfers
...previously CURLINFO_EFFECTIVE_URL would report the URL of the
original "mother transfer", not the actually pushed resource.

Reported-by: Jonathan Cardoso Machado
Fixes #5589
Closes #5591
2020-06-25 09:31:21 +02:00
Daniel Stenberg 9c845be279
urldata: let the HTTP method be in the set.* struct
When the method is updated inside libcurl we must still not change the
method as set by the user as then repeated transfers with that same
handle might not execute the same operation anymore!

This fixes the libcurl part of #5462

Test 1633 added to verify.

Closes #5499
2020-06-02 16:30:36 +02:00
Daniel Stenberg c4e6968127
url: alloc the download buffer at transfer start
... 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
2020-05-30 23:14:33 +02:00
Vyron Tsingaras 34a5400de1
http2: keep trying to send pending frames after req.upload_done
Fixes #1410
Closes #5401
2020-05-15 15:49:18 +02:00
Daniel Stenberg 54a2b63c70
http2: simplify and clean up trailer handling
Triggered by a crash detected by OSS-Fuzz after the dynbuf introduction in
ed35d6590e. This should make the trailer handling more straight forward and
hopefully less error-prone.

Deliver the trailer header to the callback already at receive-time. No
longer caches the trailers to get delivered at end of stream.

Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22030
Closes #5348
2020-05-07 09:49:51 +02:00
Daniel Stenberg ed35d6590e
dynbuf: introduce internal generic dynamic buffer functions
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
2020-05-04 10:40:39 +02:00
Jay Satiro 347a374c56 http2: Fix erroneous debug message that h2 connection closed
Prior to this change in libcurl debug builds http2 stream closure was
erroneously referred to as connection closure.

Before:
* nread <= 0, server closed connection, bailing

After:
* nread == 0, stream closed, bailing

Closes https://github.com/curl/curl/pull/5118
2020-03-18 03:39:27 -04:00
Daniel Stenberg e364546fb3
version: make curl_version* thread-safe without using global context
Closes #5010
2020-03-07 12:10:11 +01:00
Daniel Stenberg 15f51474c8
http2: make pausing/unpausing set/clear local stream window
This reduces the HTTP/2 window size to 32 MB since libcurl might have to
buffer up to this amount of data in memory and yet we don't want it set
lower to potentially impact tranfer performance on high speed networks.

Requires nghttp2 commit b3f85e2daa629
(https://github.com/nghttp2/nghttp2/pull/1444) to work properly, to end
up in the next release after 1.40.0.

Fixes #4939
Closes #4940
2020-02-27 10:35:51 +01:00
Daniel Stenberg 5808a0d0f5
http2: now require nghttp2 >= 1.12.0
To simplify our code and since earlier versions lack important function
calls libcurl needs to function correctly.

nghttp2 1.12.0 was relased on June 26, 2016.

Closes #4961
2020-02-21 16:54:47 +01:00
Daniel Stenberg 9607532873
ConnectionExists: respect the max_concurrent_streams limits
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 #4779
Closes #4784
2020-01-13 15:44:58 +01:00
Daniel Stenberg ee263de7a3
conncache: fix multi-thread use of shared connection cache
It could accidentally let the connection get used by more than one
thread, leading to double-free and more.

Reported-by: Christopher Reid
Fixes #4544
Closes #4557
2019-12-09 15:30:09 +01:00
Jay Satiro 9c1806ae46 build: Disable Visual Studio warning "conditional expression is constant"
- Disable warning C4127 "conditional expression is constant" globally
  in curl_setup.h for when building with Microsoft's compiler.

This mainly affects building with the Visual Studio project files found
in the projects dir.

Prior to this change the cmake and winbuild build systems already
disabled 4127 globally for when building with Microsoft's compiler.
Also, 4127 was already disabled for all build systems in the limited
circumstance of the WHILE_FALSE macro which disabled the warning
specifically for while(0). This commit removes the WHILE_FALSE macro and
all other cruft in favor of disabling globally in curl_setup.

Background:

We have various macros that cause 0 or 1 to be evaluated, which would
cause warning C4127 in Visual Studio. For example this causes it:

    #define Curl_resolver_asynch() 1

Full behavior is not clearly defined and inconsistent across versions.
However it is documented that since VS 2015 Update 3 Microsoft has
addressed this somewhat but not entirely, not warning on while(true) for
example.

Prior to this change some C4127 warnings occurred when I built with
Visual Studio using the generated projects in the projects dir.

Closes https://github.com/curl/curl/pull/4658
2019-12-01 19:01:02 -05:00
Daniel Stenberg dcd7e37c3a
url: make Curl_close() NULLify the pointer too
This is the common pattern used in the code and by a unified approach we
avoid mistakes.

Closes #4534
2019-10-30 09:36:21 +01:00
Daniel Stenberg 95a4cfd888
http2_recv: a closed stream trumps pause state
... and thus should return 0, not EAGAIN.

Reported-by: Tom van der Woerdt
Fixes #4496
Closes #4505
2019-10-18 23:23:48 +02:00
Daniel Stenberg 59041f052a
http2: expire a timeout at end of stream
To make sure that transfer is being dealt with. Streams without
Content-Length need a final read to notice the end-of-stream state.

Reported-by: Tom van der Woerdt
Fixes #4496
2019-10-18 23:23:43 +02:00
Daniel Stenberg 8a00560de2
http2: move state-init from creation to pre-transfer
To make sure that the HTTP/2 state is initialized correctly for
duplicated handles. It would otherwise easily generate "spurious"
PRIORITY frames to get sent over HTTP/2 connections when duplicated easy
handles were used.

Reported-by: Daniel Silverstone
Fixes #4303
Closes #4442
2019-10-03 22:55:42 +02:00
Kunal Ekawde c124e6b3c0
CURLMOPT_MAX_CONCURRENT_STREAMS: new setopt
Closes #4410
2019-10-02 07:47:48 +02:00
Daniel Stenberg b259baabfe
http2: Expression 'stream->stream_id != - 1' is always true
PVS-Studio warning
Fixes #4402
2019-09-23 22:44:02 +02:00
Daniel Stenberg f91b82e688
http2: A value is being subtracted from the unsigned variable
PVS-Studio warning
Fixes #4402
2019-09-23 22:44:02 +02:00
Barry Pollard 0023fce38d
http: lowercase headernames for HTTP/2 and HTTP/3
Closes #4401
Fixes #4400
2019-09-23 22:33:31 +02:00
Christoph M. Becker 7c596f5dea
http2: relax verification of :authority in push promise requests
If the :authority pseudo header field doesn't contain an explicit port,
we assume it is valid for the default port, instead of rejecting the
request for all ports.

Ref: https://curl.haxx.se/mail/lib-2019-09/0041.html

Closes #4365
2019-09-16 23:36:22 +02:00
Daniel Stenberg c1b6a384f9
http2: when marked for closure and wanted to close == OK
It could otherwise return an error even when closed correctly if GOAWAY
had been received previously.

Reported-by: Tom van der Woerdt
Fixes #4267
Closes #4268
2019-08-26 22:44:59 +02:00
Daniel Stenberg b1616dad8f
timediff: make it 64 bit (if possible) even with 32 bit time_t
... to make it hold microseconds too.

Fixes #4165
Closes #4168
2019-08-01 07:43:24 +02:00
Daniel Stenberg a55faf33d4
cleanup: remove the 'numsocks' argument used in many places
It was used (intended) to pass in the size of the 'socks' array that is
also passed to these functions, but was rarely actually checked/used and
the array is defined to a fixed size of MAX_SOCKSPEREASYHANDLE entries
that should be used instead.

Closes #4169
2019-07-30 23:16:44 +02:00
Daniel Stenberg fc6045f5d1
http2_recv: trigger another read when the last data is returned
... so that end-of-stream is detected properly.

Reported-by: Tom van der Woerdt
Fixes #4043
Closes #4160
2019-07-29 22:16:27 +02:00
Daniel Gustafsson 11d7fe118d headers: Remove no longer exported functions
There were a leftover few prototypes of Curl_ functions that we used to
export but no longer do, this removes those prototypes and cleans up any
comments still referring to them.

Curl_write32_le(), Curl_strcpy_url(), Curl_strlen_url(), Curl_up_free()
Curl_concat_url(), Curl_detach_connnection(), Curl_http_setup_conn()
were made static in 05b100aee2.
Curl_http_perhapsrewind() made static in 574aecee20.

For the remainder, I didn't trawl the Git logs hard enough to capture
their exact time of deletion, but they were all gone: Curl_splayprint(),
Curl_http2_send_request(), Curl_global_host_cache_dtor(),
Curl_scan_cache_used(), Curl_hostcache_destroy(), Curl_second_connect(),
Curl_http_auth_stage() and Curl_close_connections().

Closes #4096
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
2019-07-10 19:26:40 +02:00
Daniel Stenberg 265f7f42f6
http2: call done_sending on end of upload
To make sure a HTTP/2 stream registers the end of stream.

Bug #4043 made me find this problem but this fix doesn't correct the
reported issue.

Closes #4068
2019-06-24 23:11:37 +02:00
Daniel Stenberg c0c40ab075
http2: don't call stream-close on already closed streams
Closes #4055
2019-06-21 00:07:13 +02:00
Daniel Stenberg ee6945c088
http2: remove CURL_DISABLE_TYPECHECK define
... in http2-less builds as it served no use.
2019-06-11 07:49:24 +02:00