#
c9b95c0b |
| 19-Jun-2024 |
Stefan Eissing |
lib: graceful connection shutdown When libcurl discards a connection there are two phases this may go through: "shutdown" and "closing". If a connection is aborted, the shutdown phas
lib: graceful connection shutdown When libcurl discards a connection there are two phases this may go through: "shutdown" and "closing". If a connection is aborted, the shutdown phase is skipped and it is closed right away. The connection filters attached to the connection implement the phases in their `do_shutdown()` and `do_close()` callbacks. Filters carry now a `shutdown` flags next to `connected` to keep track of the shutdown operation. Filters are shut down from top to bottom. If a filter is not connected, its shutdown is skipped. Notable filters that *do* something during shutdown are HTTP/2 and TLS. HTTP/2 sends the GOAWAY frame. TLS sends its close notify and expects to receive a close notify from the server. As sends and receives may EAGAIN on the network, a shutdown is often not successful right away and needs to poll the connection's socket(s). To facilitate this, such connections are placed on a new shutdown list inside the connection cache. Since managing this list requires the cooperation of a multi handle, only the connection cache belonging to a multi handle is used. If a connection was in another cache when being discarded, it is removed there and added to the multi's cache. If no multi handle is available at that time, the connection is shutdown and closed in a one-time, best-effort attempt. When a multi handle is destroyed, all connection still on the shutdown list are discarded with a final shutdown attempt and close. In curl debug builds, the environment variable `CURL_GRACEFUL_SHUTDOWN` can be set to make this graceful with a timeout in milliseconds given by the variable. The shutdown list is limited to the max number of connections configured for a multi cache. Set via CURLMOPT_MAX_TOTAL_CONNECTIONS. When the limit is reached, the oldest connection on the shutdown list is discarded. - In multi_wait() and multi_waitfds(), collect all connection caches involved (each transfer might carry its own) into a temporary list. Let each connection cache on the list contribute sockets and POLLIN/OUT events it's connections are waiting for. - in multi_perform() collect the connection caches the same way and let them peform their maintenance. This will make another non-blocking attempt to shutdown all connections on its shutdown list. - for event based multis (multi->socket_cb set), add the sockets and their poll events via the callback. When `multi_socket()` is invoked for a socket not known by an active transfer, forward this to the multi's cache for processing. On closing a connection, remove its socket(s) via the callback. TLS connection filters MUST NOT send close nofity messages in their `do_close()` implementation. The reason is that a TLS close notify signals a success. When a connection is aborted and skips its shutdown phase, the server needs to see a missing close notify to detect something has gone wrong. A graceful shutdown of FTP's data connection is performed implicitly before regarding the upload/download as complete and continuing on the control connection. For FTP without TLS, there is just the socket close happening. But with TLS, the sent/received close notify signals that the transfer is complete and healthy. Servers like `vsftpd` verify that and reject uploads without a TLS close notify. - added test_19_* for shutdown related tests - test_19_01 and test_19_02 test for TCP RST packets which happen without a graceful shutdown and should no longer appear otherwise. - add test_19_03 for handling shutdowns by the server - add test_19_04 for handling shutdowns by curl - add test_19_05 for event based shutdowny by server - add test_30_06/07 and test_31_06/07 for shutdown checks on FTP up- and downloads. Closes #13976
show more ...
|
#
c8e0cd1d |
| 25-Apr-2024 |
Stefan Eissing |
http3: quiche+ngtcp2 improvements - quiche: error transfers that try to receive on a closed or draining connection - ngtcp2: use callback for extending max bidi streams. This
http3: quiche+ngtcp2 improvements - quiche: error transfers that try to receive on a closed or draining connection - ngtcp2: use callback for extending max bidi streams. This allows more precise calculation of MAX_CONCURRENT as we only can start a new stream when the server acknowledges the close - not when we locally have closed it. - remove a fprintf() from h2-download client to avoid excess log files on tests timing out. Closes #13475
show more ...
|
#
08d10d2a |
| 15-Apr-2024 |
Stefan Eissing |
http3: extend download abort tests, fixes in ngtcp2 - fix flow handling in ngtcp2 to ACK data on streams we abort ourself. - extend test_02_23* cases to also run for h3 - skip
http3: extend download abort tests, fixes in ngtcp2 - fix flow handling in ngtcp2 to ACK data on streams we abort ourself. - extend test_02_23* cases to also run for h3 - skip test_02_23* for OpenSSL QUIC as it gets stalled on progressing the connection Closes #13374
show more ...
|
#
28c5ddf1 |
| 09-Apr-2024 |
Stefan Eissing |
tests: stabilitze test_02_23* - h2-download now always opens the output file on first write callback invocation, if it will pause the transfer or not. - Checks on output files then
tests: stabilitze test_02_23* - h2-download now always opens the output file on first write callback invocation, if it will pause the transfer or not. - Checks on output files then does not depend on the amount of data curl has collected for the first write. Closes #13323
show more ...
|
#
1302aa6b |
| 05-Apr-2024 |
Stefan Eissing |
http2: emit RST when client write fails - When the writing of response data fails, reset the stream and do not return a callback error to nghttp2. That would be a fatal error for
http2: emit RST when client write fails - When the writing of response data fails, reset the stream and do not return a callback error to nghttp2. That would be a fatal error for the connection and harm other requests. - add test cases for various abort scenarios Reported-by: Konstantin Kuzov Fixes #13292 Closes #13298
show more ...
|
#
95a882d2 |
| 19-Dec-2023 |
Viktor Szakats |
build: fix `-Wconversion`/`-Wsign-conversion` warnings Fix remaining warnings in examples and tests which are not suppressed by the pragma in `lib/curl_setup.h`. Silence a toolc
build: fix `-Wconversion`/`-Wsign-conversion` warnings Fix remaining warnings in examples and tests which are not suppressed by the pragma in `lib/curl_setup.h`. Silence a toolchain issue causing warnings in `FD_SET()` calls with older Cygwin/MSYS2 builds. Likely fixed on 2020-08-03 by: https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=5717262b8ecfed0f7fab63e2c09c78991e36f9dd Follow-up to 2dbe75bd7f3c36837aa06fd87a442bdf3fb7faef #12492 Closes #12557
show more ...
|
#
3829759b |
| 08-Dec-2023 |
Viktor Szakats |
build: enable missing OpenSSF-recommended warnings, with fixes https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html as of 2023-11-29 [1]
build: enable missing OpenSSF-recommended warnings, with fixes https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html as of 2023-11-29 [1]. Enable new recommended warnings (except `-Wsign-conversion`): - enable `-Wformat=2` for clang (in both cmake and autotools). - add `CURL_PRINTF()` internal attribute and mark functions accepting printf arguments with it. This is a copy of existing `CURL_TEMP_PRINTF()` but using `__printf__` to make it compatible with redefinting the `printf` symbol: https://gcc.gnu.org/onlinedocs/gcc-3.0.4/gcc_5.html#SEC94 - fix `CURL_PRINTF()` and existing `CURL_TEMP_PRINTF()` for mingw-w64 and enable it on this platform. - enable `-Wimplicit-fallthrough`. - enable `-Wtrampolines`. - add `-Wsign-conversion` commented with a FIXME. - cmake: enable `-pedantic-errors` the way we do it with autotools. Follow-up to d5c0351055d5709da8f3e16c91348092fdb481aa #2747 - lib/curl_trc.h: use `CURL_FORMAT()`, this also fixes it to enable format checks. Previously it was always disabled due to the internal `printf` macro. Fix them: - fix bug where an `set_ipv6_v6only()` call was missed in builds with `--disable-verbose` / `CURL_DISABLE_VERBOSE_STRINGS=ON`. - add internal `FALLTHROUGH()` macro. - replace obsolete fall-through comments with `FALLTHROUGH()`. - fix fallthrough markups: Delete redundant ones (showing up as warnings in most cases). Add missing ones. Fix indentation. - silence `-Wformat-nonliteral` warnings with llvm/clang. - fix one `-Wformat-nonliteral` warning. - fix new `-Wformat` and `-Wformat-security` warnings. - fix `CURL_FORMAT_SOCKET_T` value for mingw-w64. Also move its definition to `lib/curl_setup.h` allowing use in `tests/server`. - lib: fix two wrongly passed string arguments in log outputs. Co-authored-by: Jay Satiro - fix new `-Wformat` warnings on mingw-w64. [1] https://github.com/ossf/wg-best-practices-os-developers/blob/56c0fde3895bfc55c8a973ef49a2572c507b2ae1/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C%2B%2B.md Closes #12489
show more ...
|
#
84338c4d |
| 15-Nov-2023 |
Viktor Szakats |
build: add more picky warnings and fix them Enable more picky compiler warnings. I've found these options in the nghttp3 project when implementing the CMake quick picky warning funct
build: add more picky warnings and fix them Enable more picky compiler warnings. I've found these options in the nghttp3 project when implementing the CMake quick picky warning functionality for it [1]. `-Wunused-macros` was too noisy to keep around, but fixed a few issues it revealed while testing. - autotools: reflect the more precisely-versioned clang warnings. Follow-up to 033f8e2a08eb1d3102f08c4d8c8e85470f8b460e #12324 - autotools: sync between clang and gcc the way we set `no-multichar`. - autotools: avoid setting `-Wstrict-aliasing=3` twice. - autotools: disable `-Wmissing-noreturn` for MSYS gcc targets [2]. It triggers in libtool-generated stub code. - lib/timeval: delete a redundant `!MSDOS` guard from a `WIN32` branch. - lib/curl_setup.h: delete duplicate declaration for `fileno`. Added in initial commit ae1912cb0d494b48d514d937826c9fe83ec96c4d (1999-12-29). This suggests this may not be needed anymore, but if it does, we may restore this for those specific (non-Windows) systems. - lib: delete unused macro `FTP_BUFFER_ALLOCSIZE` since c1d6fe2aaa5a26e49a69a4f2495b3cc7a24d9394. - lib: delete unused macro `isxdigit_ascii` since f65f750742068f579f4ee6d8539ed9d5f0afcb85. - lib/mqtt: delete unused macro `MQTT_HEADER_LEN`. - lib/multi: delete unused macro `SH_READ`/`SH_WRITE`. - lib/hostip: add `noreturn` function attribute via new `CURL_NORETURN` macro. - lib/mprintf: delete duplicate declaration for `Curl_dyn_vprintf`. - lib/rand: fix `-Wunreachable-code` and related fallouts [3]. - lib/setopt: fix `-Wunreachable-code-break`. - lib/system_win32 and lib/timeval: fix double declarations for `Curl_freq` and `Curl_isVistaOrGreater` in CMake UNITY mode [4]. - lib/warnless: fix double declarations in CMake UNITY mode [5]. This was due to force-disabling the header guard of `warnless.h` to to reapply it to source code coming after `warnless.c` in UNITY builds. This reapplied declarations too, causing the warnings. Solved by adding a header guard for the lines that actually need to be reapplied. - lib/vauth/digest: fix `-Wunreachable-code-break` [6]. - lib/vssh/libssh2: fix `-Wunreachable-code-break` and delete redundant block. - lib/vtls/sectransp: fix `-Wunreachable-code-break` [7]. - lib/vtls/sectransp: suppress `-Wunreachable-code`. Detected in `else` branches of dynamic feature checks, with results known at compile-time, e.g. ```c if(SecCertificateCopySubjectSummary) /* -> true */ ``` Likely fixable as a separate micro-project, but given SecureTransport is deprecated anyway, let's just silence these locally. - src/tool_help: delete duplicate declaration for `helptext`. - src/tool_xattr: fix `-Wunreachable-code`. - tests: delete duplicate declaration for `unitfail` [8]. - tests: delete duplicate declaration for `strncasecompare`. - tests/libtest: delete duplicate declaration for `gethostname`. Originally added in 687df5c8c39c370a59999b9afc0917d808d978b7 (2010-08-02). Got complicated later: c49e9683b85ba9d12cbb6eebc4ab2c8dba68fbdc If there are still systems around with warnings, we may restore the prototype, but limited for those systems. - tests/lib2305: delete duplicate declaration for `libtest_debug_config`. - tests/h2-download: fix `-Wunreachable-code-break`. [1] https://github.com/ngtcp2/nghttp3/blob/a70edb08e954d690e8fb2c1df999b5a056f8bf9f/cmake/PickyWarningsC.cmake [2] https://ci.appveyor.com/project/curlorg/curl/builds/48553586/job/3qkgjauiqla5fj45?fullLog=true#L1675 [3] https://github.com/curl/curl/actions/runs/6880886309/job/18716044703?pr=12331#step:7:72 https://github.com/curl/curl/actions/runs/6883016087/job/18722707368?pr=12331#step:7:109 [4] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L204 [5] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L218 [6] https://github.com/curl/curl/actions/runs/6880886309/job/18716042927?pr=12331#step:7:290 [7] https://github.com/curl/curl/actions/runs/6891484996/job/18746659406?pr=12331#step:9:1193 [8] https://github.com/curl/curl/actions/runs/6882803986/job/18722082562?pr=12331#step:33:1870 Closes #12331
show more ...
|
#
a9b7f72b |
| 28-Apr-2023 |
Stefan Eissing |
http2: do flow window accounting for cancelled streams - nghttp2 does not free connection level window flow for aborted streams - when closing transfers, make sure that any buffere
http2: do flow window accounting for cancelled streams - nghttp2 does not free connection level window flow for aborted streams - when closing transfers, make sure that any buffered response data is "given back" to the flow control window - add tests test_02_22 and test_02_23 to reproduce Closes #11052
show more ...
|
#
da2470de |
| 27-Apr-2023 |
Marcel Raad |
tests/http: make curl_setup.h the first include This is required for the macros there to take effect for system libraries. Specifically, including the system libraries first led to w
tests/http: make curl_setup.h the first include This is required for the macros there to take effect for system libraries. Specifically, including the system libraries first led to warnings about `_FILE_OFFSET_BITS` being redefined in curl_config.h on the Solaris autobuilds for ws-data.c and ws-pingpong.c. Also make the curl includes come first for the other source files here for consistency. Closes https://github.com/curl/curl/pull/11046
show more ...
|
#
acd82c8b |
| 26-Apr-2023 |
Stefan Eissing |
tests/http: more tests with specific clients - Makefile support for building test specific clients in tests/http/clients - auto-make of clients when invoking pytest - added test_09_0
tests/http: more tests with specific clients - Makefile support for building test specific clients in tests/http/clients - auto-make of clients when invoking pytest - added test_09_02 for server PUSH_PROMISEs using clients/h2-serverpush - added test_02_21 for lib based downloads and pausing/unpausing transfers curl url parser: - added internal method `curl_url_set_authority()` for setting the authority part of a url (used for PUSH_PROMISE) http2: - made logging of PUSH_PROMISE handling nicer Placing python test requirements in requirements.txt files - separate files to base test suite and http tests since use and module lists differ - using the files in the gh workflows websocket test cases, fixes for we and bufq - bufq: account for spare chunks in space calculation - bufq: reset chunks that are skipped empty - ws: correctly encode frames with 126 bytes payload - ws: update frame meta information on first call of collect callback that fills user buffer - test client ws-data: some test/reporting improvements Closes #11006
show more ...
|