Commit Graph

772 Commits

Author SHA1 Message Date
Daniel Stenberg
c074ba64a8
code: language cleanup in comments
Based on the standards and guidelines we use for our documentation.

 - expand contractions (they're => they are etc)
 - host name = > hostname
 - file name => filename
 - user name = username
 - man page => manpage
 - run-time => runtime
 - set-up => setup
 - back-end => backend
 - a HTTP => an HTTP
 - Two spaces after a period => one space after period

Closes #14073
2024-07-01 22:58:55 +02:00
Daniel Stenberg
816ac2a866
docs: misc language polish
- CURLINFO_FILETIME*: improve language
- add '32bit' and '64bit' as bad words, use 32-bit and 64-bit
- mksymbolsmanpage.pl: avoid "will"

Closes #14070
2024-07-01 16:45:17 +02:00
Daniel Stenberg
9a49af5fd8
lib: add a few DEBUGASSERT(data) to aid code analyzers
... where 'data' is assumed to always work.

Closes #14033
2024-06-27 10:31:13 +02:00
Stefan Eissing
c9b95c0bb3
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
2024-06-26 08:33:17 +02:00
Stefan Eissing
def99d8507
multi: multi_getsock(), check correct socket
- in phase CONNECTING/TUNNELING/PROTOCONNECT, retrieve
   the socket from the connection filters and do not rely
   on `conn->sockfd` being already set by the transfer.
 - this applies to the default behaviour, a protocol handler
   may override this via its callbacks.
 - add a warning message in multi_getsock() when the transfer
   is expected to have something in its pollset, but instead
   it is empty.

Reported-by: saurabhsingh-dev on github
Fixes #13998
Closes #14011
2024-06-25 13:15:01 +02:00
Stefan Eissing
3841569ec8
transfer: do not use EXPIRE_NOW while blocked
- When a transfer sets `data->state.select_bits`, it is
  scheduled for rerun with EXPIRE_NOW. If such a transfer
  is blocked (due to PAUSE, for example), this will lead to
  a busy loop.
- multi.c: check for transfer block
- sendf.*: add Curl_xfer_is_blocked()
- sendf.*: add client reader `is_paused()` callback
- implement is_paused()` callback where needed

Closes #13908
2024-06-13 15:13:43 +02:00
Stefan Eissing
374d178f14
multi: prepare multi_wait() for future shutdown usage
- new struct curl_pollfds and struct curl_waitfds
- add structs and methods to init/add/cleanup an array of pollfd and
  struct curl_waitfd. Use in multi_wait() and multi_waitfds() to
  populate the sets for polling.
- place USE_WINSOCK WSAEventSelect() setting into a separate loop over
  all collected pfds

Closes #13900
2024-06-10 13:11:05 +02:00
Viktor Szakats
72abf7c13a
lib: tidy up types and casts
Cherry-picked from #13489
Closes #13862
2024-06-05 14:02:39 +02:00
Andy Pan
23fe1a52dc
socketpair: add eventfd and use SOCK_NONBLOCK for socketpair()
Currently, we use `pipe` for `wakeup_create`, which requires ***two***
file descriptors. Furthermore, given its complexity inside, `pipe` is a
bit heavyweight for just a simple event wait/notify mechanism.

`eventfd` would be a more suitable solution for this kind of scenario,
kernel also advocates for developers to use `eventfd` instead of `pipe`
in some simple use cases:

    Applications can use an eventfd file descriptor instead of a pipe
    (see pipe(2) in all cases where a pipe is used simply to signal
    events. The kernel overhead of an eventfd file descriptor is much
    lower than that of a pipe, and only one file descriptor is required
    (versus the two required for a pipe).

This change adds the new backend of `eventfd` for `wakeup_create` and
uses it where available, eliminating the overhead of `pipe`. Also, it
optimizes the `wakeup_create` to eliminate the system calls that make
file descriptors non-blocking by moving the logic of setting
non-blocking flags on file descriptors to `socketpair.c` and using
`SOCK_NONBLOCK` for `socketpair(2)`, `EFD_NONBLOCK` for `eventfd(2)`.

Ref:
https://man7.org/linux/man-pages/man7/pipe.7.html
https://man7.org/linux/man-pages/man2/eventfd.2.html
https://man7.org/linux/man-pages/man2/socketpair.2.html
https://www.gnu.org/software/gnulib/manual/html_node/eventfd.html

Closes #13874
2024-06-04 23:45:36 +02:00
Stefan Eissing
c8096668ae
multi: fix multi_wait() timeout handling
- determine the actual poll timeout *after* all sockets
  have been collected. Protocols and connection filters may
  install new timeouts during collection.
- add debug logging to test1533 where the mistake was noticed

Reported-by: Matt Jolly
Fixes #13782
Closes #13825
2024-05-30 08:29:00 +02:00
Stefan Eissing
e101a7a8b0
multi: add multi->proto_hash, a key-value store for protocol data
- add `Curl_hash_add2()` that passes a destructor function for
  the element added. Call element destructor instead of hash
  destructor if present.
- multi: add `proto_hash` for protocol related information,
  remove `struct multi_ssl_backend_data`.
- openssl: use multi->proto_hash to keep x509 shared store
- schannel: use multi->proto_hash to keep x509 shared store
- vtls: remove Curl_free_multi_ssl_backend_data() and its
  equivalents in the TLS backends

Closes #13345
2024-05-26 00:15:01 +02:00
Viktor Szakats
a35bbe8977
lib: bump hash sizes to size_t
Follow-up to cc907e80a2 #13502
Cherry-picked from #13489
Closes #13601
2024-05-12 18:53:09 +02:00
Jay Satiro
798a37b25e lib: clear the easy handle's saved errno before transfer
- Clear data->state.os_errno before transfer.

- Explain the change in behavior in the CURLINFO_OS_ERRNO doc.

- Add to the CURLINFO_OS_ERRNO doc the list of libcurl network-related
  errors that may cause the errno to be saved.

data->state.os_errno is saved before libcurl returns a network-related
failure such as connection failure. It is accessible to the user via
CURLINFO_OS_ERRNO so they can get more information about the failure.

Prior to this change it wasn't cleared before transfer, so if a user
retrieved the saved errno it could be from a previous transfer. That is
because an errno is not always saved for network-related errors.

Closes https://github.com/curl/curl/pull/13574
2024-05-10 18:26:32 -04:00
Daniel Stenberg
7f7ad97bf1
multi: remove useless assignment
Spotted by CodeSonar

Closes #13500
2024-04-30 09:24:07 +02:00
Stefan Eissing
2d2c27e5a3
multi: multi_wait improvements
- only call `multi_getsock()` once for all transfers
 - realloc pollset array on demand
 - fold repeated sockets

Closes #13150
2024-04-25 23:31:59 +02:00
Daniel Stenberg
3572dd65bb
multi: avoid memory-leak risk
'newurl' is allocated in some conditions and used in a few scenarios,
but there were theoretical combinations in which it would not get freed.
Move the free to happen unconditionally. Never triggered by tests, but
spotted by Coverity.

Closes #13471
2024-04-25 22:32:32 +02:00
Daniel Stenberg
25236c6a80
multi: remove the unused Curl_preconnect function
The implementation has been removed, no point in keeping it around.

Follow-up to 476adfeac0

Closes #13422
2024-04-19 15:06:48 +02:00
Stefan Eissing
270a25c011
cw-out: improved error handling
- remember error encountered in invoking write callback and always fail
  afterwards without further invokes

- check behaviour in test_02_17 with h2-pausing client

Reported-by: Pavel Kropachev
Fixes #13337
Closes #13340
2024-04-16 15:52:10 +02:00
Daniel Stenberg
be659030ba
multi: introduce SETUP state for better timeouts
Since we can go to the CONNECT state from PENDING, potentially multiple
times for a single transfer, this change introdues a SETUP state that
happens before CONNECT when doing a new transfer.

Now, doing a redirect on a handle goes back to SETUP (not CONNECT like
before) and we initilize the connect timeout etc in SETUP. Previously,
we would do it in CONNECT but that would make it unreliable in cases
where a transfer goes in and out between CONNECT and PENDING multiple
times.

SETUP is transient, so the handle never actually stays in that state.

Additionally: take care of timeouts of PENDING transfers in
curl_multi_perform()

Ref: #13227
Closes #13371
2024-04-15 23:42:06 +02:00
Daniel Stenberg
a1ec035afc
multi: timeout handles even without connection
When there is a "change" in a multi handle and pending handles are moved
back to the main list to be retested if they can proceed further (for
example a previous transfer completed or a connection has a confirmed
multiplexed state), the timeout check in multi_runsingle() would not
trigger because it required an established connection.

This could make a pending tranfer go back to pending state even though
it had been "in progress" for a longer time than permitted. By removing
the requirement for an associated connection, the timeout check will be
done proper even for transfers that has not yet been assigned one.

Ref #13227
Reported-by: Rahul Krishna M
Closes #13276
2024-04-15 09:49:14 +02:00
RainRat
1087937992
misc: fix typos
Closes #13344
2024-04-11 15:44:22 +02:00
Stefan Eissing
c296abd42d
llist: add Curl_llist_append()
- use for better readability in all places where the "insert_next"
  actually performs an append to the list
- add some tests in unit1300

Closes #13336
2024-04-11 09:00:51 +02:00
Dmitry Karpov
02beac6bb6
lib: add curl_multi_waitfds
New function call, similar to curl_multi_fdset()

Closes #13135
2024-04-09 16:53:40 +02:00
Jay Satiro
bf567dd9f1 lib: use multi instead of multi_easy for the active multi
- Use data->multi and not data->multi_easy to refer to the active multi.

The easy handle's active multi is always data->multi.

This is a follow up to 757dfdf which changed curl so that an easy handle
used with the easy interface and then multi interface cannot have two
different multi handles associated with it at the same time
(data->multi_easy from the easy interface and data->multi from the multi
interface).

Closes https://github.com/curl/curl/pull/12665
2024-04-05 16:47:36 -04:00
Stefan Eissing
4e4e8af1f6
lib: move 'done' parameter to SingleRequests
A transfer may do several `SingleRequest`s for its success. This happens
regularly for authentication, follows and retries on failed connections.
The "readwrite()" calls and functions connected to those carried a `bool
*done` parameter to indicate that the current `SingleRequest` is over.
This may happen before `upload_done` or `download_done` bits of
`SingleRequest` are set.

The problem with that is now `write_resp()` protocol handlers are
invoked in places where the `bool *done` cannot be passed up to the
caller. Instead of being a bool in the call chain, it needs to become a
member of `SingleRequest`, reflecting its state.

This removes the `bool *done` parameter and adds the `done` bit to
`SingleRequest` instead. It adds `Curl_req_soft_reset()` for using a
`SingleRequest` in a follow up, clearing `done` and other
flags/counters.

Closes #13096
2024-03-11 23:27:02 +01:00
Stefan Eissing
fcef00db1a
lib: keep conn IP information together
new struct ip_quadruple for holding local/remote addr+port

- used in data->info and conn and cf-socket.c
- copy back and forth complete struct
- add 'secondary' to conn
- use secondary in reporting success for ftp 2nd connection

Reported-by: DasKutti on github
Fixes #13084
Closes #13090
2024-03-09 23:24:58 +01:00
Stefan Eissing
14bcea074a
lib: enhance client reader resume + rewind
- update client reader documentation
- client reader, add rewind capabilities
    - tell creader to rewind on next start
    - Curl_client_reset() will keep reader for future rewind if requested
    - add Curl_client_cleanup() for freeing all resources independent of
      rewinds
    - add Curl_client_start() to trigger rewinds
    - move rewind code from multi.c to sendf.c and make part of
      "cr-in"'s implementation
- http, move the "resume_from" handling into the client readers
    - the setup of a HTTP request is reshuffled to follow:
      * determine method, target, auth negotiation
      * install the client reader(s) for the request, including crlf
        conversions and "chunked" encoding
      * apply ranges to client reader
      * concat request headers, upgrades, cookies, etc.
      * complete request by determining Content-Length of installed
        readers in combination with method
      * send
    - add methods for client readers to
      * return the overall length they will generate (or -1 when unknown)
      * return the amount of data on the CLIENT level, so that
        expect-100 can decide if it want to apply itself
      * set a "resume_from" offset or fail if unsupported
    - struct HTTP has become largely empty now
- rename `Client_reader_*` to `Curl_creader_*`

Closes #13026
2024-03-05 13:26:05 +01:00
Stefan Eissing
e3905de819
lib: further send/upload handling polish
- Move all the "upload_done" handling to request.c

  - add possibility to abort sending of a request
  - add `Curl_req_done_sending()` for checks
  - transfer.c: readwrite_upload() now clean

- removing data->state.ulbuf and data->req.upload_fromhere

  - as well as data->req.upload_present
  - set data->req.upload_done on having read all from
    the client and completely flushed the send buffer

- tftp, remove setting of data->req.upload_fromhere

  - serves no purpose as `upload_present` is not set
    and the data itself is directly `sendto()` anyway

- smtp, make upload EOB conversion a client reader
- xfer_ulbuf addition

  - add xfer_ulbuf for borrowing, similar to xfer_buf
  - use in file upload
  - use in c-hyper body sending

- h1-proxy, remove init of data->state.uilbuf that is never used
- smb, add own send_buf instead of using data->state.ulbuf

Closes #13010
2024-03-04 08:42:56 +01:00
Daniel Stenberg
757dfdfb55
multi: make add_handle free any multi_easy
If the easy handle that is being added to a multi handle has previously
been used for curl_easy_perform(), there is a private multi handle here
that we can kill off. While it flushes some caches etc for the easy
handle would it be used for an easy interface transfer again after being
used in the multi stack, this cleanup simplifies behavior and uses less
memory.

Closes #12992
2024-02-27 10:03:24 +01:00
Stefan Eissing
5929822114
lib: send rework
Curl_read/Curl_write clarifications

- replace `Curl_read()`, `Curl_write()` and `Curl_nwrite()` to 1clarify
  when and at what level they operate

- send/recv of transfer related data is now done via
  `Curl_xfer_send()/Curl_xfer_recv()` which no longer has
  socket/socketindex as parameter. It decides on the transfer setup of
  `conn->sockfd` and `conn->writesockfd` on which connection filter
  chain to operate.

- send/recv on a specific connection filter chain is done via
  `Curl_conn_send()/Curl_conn_recv()` which get the socket index as
  parameter.

- rename `Curl_setup_transfer()` to `Curl_xfer_setup()` for naming
  consistency

- clarify that the special CURLE_AGAIN handling to return `CURLE_OK`
  with length 0 only applies to `Curl_xfer_send()` and CURLE_AGAIN is
  returned by all other send() variants.

SingleRequest reshuffling

- move functions into request.[ch]
- differentiate between reset and free
- add Curl_req_done() to perform last actions
- add a send `bufq` to SingleRequest for future use in keeping upload data

Closes #12963
2024-02-27 08:58:10 +01:00
Stefan Eissing
463472a2d6
lib: move client writer into own source
Refactoring of the client writer that passes the data to the
client/application's callback functions.

- split out into own source cw-out.[ch] from sendf.c

- move tempwrite and tempcount from data->state into the context of the
  client writer

- redesign the 3 tempwrite dynbufs as a linked list of dynbufs. On
  paused transfers, this allows to "record" interleaved HEADER/BODY
  chunks to be "played back" in the same order on unpausing.

- keep the overall size limit of all buffered data to DYN_PAUSE_BUFFER.
  On exceeding that, return CURLE_TOO_LARGE instead of
  CURLE_OUT_OF_MEMORY as before.

- add method to be called when a transfer is DONE to allow writing of
  any data still buffered

- when paused, record HEADER writes exactly as they come for later
  playback. HEADERs are documented to be written one-by-one.

Closes #12898
2024-02-26 14:25:41 +01:00
MAntoniak
e26c362544
lib: remove curl_mimepart object when CURL_DISABLE_MIME
Remove curl_mimepart object from UserDefined structure when
CURL_DISABLE_MIME flag is active. Reduce size of UserDefined structure.

Also remove unreachable code: when CURL_DISABLE_MIME is set, httpreq can
never have HTTPREQ_POST_MIME value and the same goes for the
CURL_DISABLE_FORM_API flag and the HTTPREQ_POST_FORM value

Closes #12948
2024-02-26 09:22:34 +01:00
Stefan Eissing
f274fc5c68
multi: fix multi_sock handling of select_bits
- OR the event bitmask to data->state.select_bits instead of overwriting
  them. They are cleared again on use.

Reported-by: 5533asdg on github
Fixes #12971
Closes #12972
2024-02-22 14:15:27 +01:00
Stefan Eissing
476adfeac0
multi: add xfer_buf to multi handle
- can be borrowed by transfer during recv-write operation
- needs to be released before borrowing again
- adjustis size to `data->set.buffer_size`
- used in transfer.c readwrite_data()

Closes #12805
2024-02-09 09:43:50 +01:00
Stefan Eissing
d7b6ce64ce
lib: replace readwrite with write_resp
This clarifies the handling of server responses by folding the code for
the complicated protocols into their protocol handlers. This concerns
mainly HTTP and its bastard sibling RTSP.

The terms "read" and "write" are often used without clear context if
they refer to the connect or the client/application side of a
transfer. This PR uses "read/write" for operations on the client side
and "send/receive" for the connection, e.g. server side. If this is
considered useful, we can revisit renaming of further methods in another
PR.

Curl's protocol handler `readwrite()` method been changed:

```diff
-  CURLcode (*readwrite)(struct Curl_easy *data, struct connectdata *conn,
-                        const char *buf, size_t blen,
-                        size_t *pconsumed, bool *readmore);
+  CURLcode (*write_resp)(struct Curl_easy *data, const char *buf, size_t blen,
+                         bool is_eos, bool *done);
```

The name was changed to clarify that this writes reponse data to the
client side. The parameter changes are:

* `conn` removed as it always operates on `data->conn`
* `pconsumed` removed as the method needs to handle all data on success
* `readmore` removed as no longer necessary
* `is_eos` as indicator that this is the last call for the transfer
  response (end-of-stream).
* `done` TRUE on return iff the transfer response is to be treated as
  finished

This change affects many files only because of updated comments in
handlers that provide no implementation. The real change is that the
HTTP protocol handlers now provide an implementation.

The HTTP protocol handlers `write_resp()` implementation will get passed
**all** raw data of a server response for the transfer. The HTTP/1.x
formatted status and headers, as well as the undecoded response
body. `Curl_http_write_resp_hds()` is used internally to parse the
response headers and pass them on. This method is public as the RTSP
protocol handler also uses it.

HTTP/1.1 "chunked" transport encoding is now part of the general
*content encoding* writer stack, just like other encodings. A new flag
`CLIENTWRITE_EOS` was added for the last client write. This allows
writers to verify that they are in a valid end state. The chunked
decoder will check if it indeed has seen the last chunk.

The general response handling in `transfer.c:466` happens in function
`readwrite_data()`. This mainly operates now like:

```
static CURLcode readwrite_data(data, ...)
{
  do {
    Curl_xfer_recv_resp(data, buf)
    ...
    Curl_xfer_write_resp(data, buf)
    ...
  } while(interested);
  ...
}
```

All the response data handling is implemented in
`Curl_xfer_write_resp()`. It calls the protocol handler's `write_resp()`
implementation if available, or does the default behaviour.

All raw response data needs to pass through this function. Which also
means that anyone in possession of such data may call
`Curl_xfer_write_resp()`.

Closes #12480
2024-01-13 17:23:42 +01:00
Stefan Eissing
48d86999af
multi: pollset adjust, init with FIRSTSOCKET during connect
- `conn->sockfd` is set by `Curl_setup_transfer()`, but that
  is called *after* the connection has been established
- use `conn->sock[FIRSTSOCKET]` instead

Follow-up to a0f94800d5
Closes #12664
2024-01-09 17:41:13 +01:00
Stefan Eissing
a0f94800d5
transfer: adjust_pollset improvements
- let `multi_getsock()` initialize the pollset in what the
  transfer state requires in regards to SEND/RECV
- change connection filters `adjust_pollset()` implementation
  to react on the presence of POLLIN/-OUT in the pollset and
  no longer check CURL_WANT_SEND/CURL_WANT_RECV
- cf-socket will no longer add POLLIN on its own
- http2 and http/3 filters will only do adjustments if the
  passed pollset wants to POLLIN/OUT for the transfer on
  the socket. This is similar to the HTTP/2 proxy filter
  and works in stacked filters.

Closes #12640
2024-01-07 14:36:25 +01:00
Daniel Stenberg
68f96fc9bf
getinfo: CURLINFO_QUEUE_TIME_T
Returns the time, in microseconds, during which this transfer was held
in a waiting queue before it started "for real". A transfer might be put
in a queue if after getting started, it cannot create a new connection
etc due to set conditions and limits imposed by the application.

Ref: #12293
Closes #12368
2023-12-27 09:28:48 +01:00
Viktor Szakats
3829759bd0
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 d5c0351055 #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] 56c0fde389/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C%2B%2B.md

Closes #12489
2023-12-16 13:12:37 +00:00
Daniel Stenberg
907eea0804
Revert "urldata: move async resolver state from easy handle to connectdata"
This reverts commit 56a4db2e4e (#12198)

We want the c-ares channel to be held in the easy handle, not per
connection - for performance.

Closes #12524
2023-12-15 12:57:35 +01:00
Stefan Eissing
8706b68010
lib: eliminate conn->cselect_bits
- use `data->state.dselect_bits` everywhere instead
- remove `bool *comeback` parameter as non-zero
  `data->state.dselect_bits` will indicate that IO is
  incomplete.

Closes #12512
2023-12-14 15:20:05 +01:00
Daniel Stenberg
d1c2bb3d8d
multi: during ratelimit multi_getsock should return no sockets
... as there is nothing to wait for then, it just waits. Otherwise, this
causes much more CPU work and updates than necessary during ratelimit
periods.

Ref: https://curl.se/mail/lib-2023-11/0056.html
Closes #12430
2023-11-30 15:53:22 +01:00
Daniel Stenberg
9a8bb20d48
CURLMOPT_MAX_CONCURRENT_STREAMS: make sure the set value is within range
... or use the default value.

Also clarify the documentation language somewhat.

Closes #12382
2023-11-22 15:01:04 +01:00
Daniel Stenberg
2d06eebf28
urldata: make maxconnects a 32 bit value
"2^32 idle connections ought to be enough for anybody"

Closes #12375
2023-11-22 13:27:38 +01:00
Viktor Szakats
84338c4de2
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 033f8e2a08 #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 ae1912cb0d
  (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
  c1d6fe2aaa.
- lib: delete unused macro `isxdigit_ascii` since
  f65f750742.
- 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 687df5c8c3
  (2010-08-02).
  Got complicated later: c49e9683b8
  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] a70edb08e9/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
2023-11-21 16:35:42 +00:00
Stefan Eissing
56a4db2e4e
urldata: move async resolver state from easy handle to connectdata
- resolving is done for a connection, not for every transfer
- save create/dup/free of a cares channel for each transfer
- check values of setopt calls against a local channel if no
  connection has been attached yet, when needed.

Closes #12198
2023-10-26 17:29:17 +02:00
Stefan Eissing
47f5b1a37f
lib: introduce struct easy_poll_set for poll information
Connection filter had a `get_select_socks()` method, inspired by the
various `getsocks` functions involved during the lifetime of a
transfer. These, depending on transfer state (CONNECT/DO/DONE/ etc.),
return sockets to monitor and flag if this shall be done for POLLIN
and/or POLLOUT.

Due to this design, sockets and flags could only be added, not
removed. This led to problems in filters like HTTP/2 where flow control
prohibits the sending of data until the peer increases the flow
window. The general transfer loop wants to write, adds POLLOUT, the
socket is writeable but no data can be written.

This leads to cpu busy loops. To prevent that, HTTP/2 did set the
`SEND_HOLD` flag of such a blocked transfer, so the transfer loop cedes
further attempts. This works if only one such filter is involved. If a
HTTP/2 transfer goes through a HTTP/2 proxy, two filters are
setting/clearing this flag and may step on each other's toes.

Connection filters `get_select_socks()` is replaced by
`adjust_pollset()`. They get passed a `struct easy_pollset` that keeps
up to `MAX_SOCKSPEREASYHANDLE` sockets and their `POLLIN|POLLOUT`
flags. This struct is initialized in `multi_getsock()` by calling the
various `getsocks()` implementations based on transfer state, as before.

After protocol handlers/transfer loop have set the sockets and flags
they want, the `easy_pollset` is *always* passed to the filters. Filters
"higher" in the chain are called first, starting at the first
not-yet-connection one. Each filter may add sockets and/or change
flags. When all flags are removed, the socket itself is removed from the
pollset.

Example:

 * transfer wants to send, adds POLLOUT
 * http/2 filter has a flow control block, removes POLLOUT and adds
   POLLIN (it is waiting on a WINDOW_UPDATE from the server)
 * TLS filter is connected and changes nothing
 * h2-proxy filter also has a flow control block on its tunnel stream,
   removes POLLOUT and adds POLLIN also.
 * socket filter is connected and changes nothing
 * The resulting pollset is then mixed together with all other transfers
   and their pollsets, just as before.

Use of `SEND_HOLD` is no longer necessary in the filters.

All filters are adapted for the changed method. The handling in
`multi.c` has been adjusted, but its state handling the the protocol
handlers' `getsocks` method are untouched.

The most affected filters are http/2, ngtcp2, quiche and h2-proxy. TLS
filters needed to be adjusted for the connecting handshake read/write
handling.

No noticeable difference in performance was detected in local scorecard
runs.

Closes #11833
2023-10-25 09:34:32 +02:00
Daniel Stenberg
fc077bc786
urldata: move the 'internal' boolean to the state struct
... where all the other state bits for the easy handles live.

Closes #12165
2023-10-20 16:59:52 +02:00
Daniel Stenberg
43eb798da0
asyn-thread: use pipe instead of socketpair for IPC when available
If pipe() is present. Less overhead.

Helped-by: Viktor Szakats
Closes #12146
2023-10-18 08:23:53 +02:00
Daniel Stenberg
03a70edc50
multi: use pipe instead of socketpair to *wakeup()
If pipe() is present. Less overhead.

Closes #12142
2023-10-17 15:29:31 +02:00