Commit Graph

11 Commits

Author SHA1 Message Date
Stefan Eissing
df5db8afaf
conn: fix connection reuse when SSL is optional
In curl 8.12 I tried to improve the logic on how we handle connections
that "upgrade" to TLS later, e.g. with a STARTTLS. I found the existing
code hard to read in this regard. But of course, the "improvements" blew
up in my face.

We fixed issues with imap, opo3, smtp in 8.12.1, but ftp was no longer
reusing existing, upgraded control connections as before. This PR adds
checks in our pytest FTP tests that verify reuse is happening as
intended.

I rewrote the logic in url.c again, so that the new test checks now pass.

Reported-by: Zenju on github
Fixes #16384
Closes #16392
2025-02-20 16:23:35 +01:00
Stefan Eissing
ef3d7877d5
pytest: remove 'repeat' parameter
With the use of the pytest-repeat plugin, this parameter is no
longer necessary.

Closes #16033
2025-01-17 14:03:30 +01:00
Dan Fandrich
0f7ba5c5bf tests: change Python code style to pass ruff checks
Most of the changes consisted of removing unused imports and unnecessary
f-strings.
2024-10-04 14:02:05 -07:00
Stefan Eissing
72d2090fc2
ftp: fix 0-length last write on upload from stdin
When uploading FTP with unknown length, we write a last 0-length chunk
with the EOS flag set. OpenSSL's SSL_write() errors on such a write.
Skip writing 0-length data to TLS backends instead.

Add test in FTPS for such uploads to verify.

Fixes #15101
Reported-by: Denis Goleshchikhin
Closes #15102
2024-10-01 13:57:12 +02:00
Stefan Eissing
db5eae1127
cf-socket: fix listen pollset for FTP active mode
Follow-up to a07ba37b5e which did not
solve the issue of corrent polling for FTP active data connections.

Added test cases for active up-/download.

Closes #14786
2024-09-05 11:00:55 +02:00
Stefan Eissing
44d1b6c271
pytest: add ftp upload ascii test
Add a test the uploads a text file in ascii mode and checks
that lengths match expectations.

Closes #14721
2024-08-29 16:45:59 +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
Viktor Szakats
ff92e3b13c
tidy-up: whitespace [ci skip] 2024-06-24 20:26:34 +02:00
Stefan Eissing
c31041b17e
connection: shutdown TLS (for FTP) better
This adds connection shutdown infrastructure and first use for FTP. FTP
data connections, when not encountering an error, are now shut down in a
blocking way with a 2sec timeout.

    - add cfilter `Curl_cft_shutdown` callback
    - keep a shutdown start timestamp and timeout at connectdata
    - provide shutdown timeout default and member in
      `data->set.shutdowntimeout`.
    - provide methods for starting, interrogating and clearing
      shutdown timers
    - provide `Curl_conn_shutdown_blocking()` to shutdown the
      `sockindex` filter chain in a blocking way. Use that in FTP.
    - add `Curl_conn_cf_poll()` to wait for socket events during
      shutdown of a connection filter chain.
      This gets the monitoring sockets and events via the filters
      "adjust_pollset()" methods. This gives correct behaviour when
      shutting down a TLS connection through a HTTP/2 proxy.
    - Implement shutdown for all socket filters
      - for HTTP/2 and h2 proxying to send GOAWAY
      - for TLS backends to the best of their capabilities
      - for tcp socket filter to make a final, nonblocking
        receive to avoid unwanted RST states
    - add shutdown forwarding to happy eyeballers and
      https connect ballers when applicable.

Closes #13904
2024-06-10 13:08:12 +02:00
Stefan Eissing
5a913d8dc3
pytest: add ftp upload tests
- refs #13556
- allow anon uploads on vsftpd test server
- add test_30_05 for plain upload of 1k, 100k, 1m
- add test_31_05 for SSL upload of 1k, 100k, 1m
- verify file size and contents

Closes #13734
2024-05-22 09:12:43 +02:00
Stefan Eissing
345557248e
pytest: fixes for recent python, add FTP tests
Fixes:
- in uds tests, abort also silently on os errors
- be conservative on the h3 goaway duration
- detect curl debug build and use in checks
- fix caddy version check for slight difference under linux
- set caddy default path fitting for linux
- fix deprecation warnings in valid time checks

FTP tests:
- add '--with-test-vsftpd=path' to configure
- use vsftpd default path suitable for linux
- add test_30 with plain FTP tests
- add test_31 with --ssl-reqd FTP tests
- add vsftpd to linux GHA for pytest workflows

Closes #13661
2024-05-17 16:53:17 +02:00