gh-103443: Make the TLS shutdown of the FTP data connection best-effort

[AliReza7222]

gh-103443: ftplib: make the TLS shutdown of the data connection best-effort

Fixes: gh-77303 ·
gh-78738 ·
gh-103443 ·
gh-124850

All four are open, labelled type-bug, and have zero linked PRs.

Suggested branch name: gh-103443-ftplib-data-shutdown
Suggested NEWS file: Misc/NEWS.d/next/Library/2026-10-06-12-00-00.gh-issue-103443.<6-char>.rst

---

Summary

retrbinary(), retrlines(), storbinary() and storlines() end their
with self.transfercmd(...) as conn: block with

# shutdown ssl layer
if _SSLSocket is not None and isinstance(conn, _SSLSocket):
    conn.unwrap()

That runs after the payload has been fully exchanged, on a socket which
exists for exactly one transfer and is closed a few lines later anyway.
Servers are under no obligation to complete a TLS shutdown handshake there,
and a large number of them do not. The result is that a transfer which
actually succeeded is reported as a failure:

Symptom	Issue
ConnectionResetError: [Errno 104] Connection reset by peer out of unwrap()	gh-103443
ssl.SSLEOFError: EOF occurred in violation of protocol	gh-124850
Transfer succeeds against servers that shut the channel down themselves, fails against others	gh-77303
Hangs forever — unwrap() blocks waiting for a close_notify that never comes	gh-78738

In every case the error or the hang happens after sendall()/recv() has
finished. The bytes are already on the wire; there is nothing left to
report.

Why not simply delete unwrap()?

That is the obvious fix and it is wrong. unwrap() does two things: it
flushes the record layer and it sends close_notify, which is how the peer
learns that the transfer is over rather than merely stalled.

Measured with CPython's own Lib/test/test_ftplib.py dummy server
(DummyTLS_FTPServer), TestTLS_FTPClassMixin, on CPython 3.13.3, three
runs each:

Lib/ftplib.py	Result
as-is (with unwrap())	3/3 OK — Ran 96 tests
unwrap() deleted everywhere	3/3 FAILED — test_storlines: AssertionError: 14739 != 17000

14739 of 17000 bytes — a silently truncated upload. Without
close_notify the peer stops draining the socket while data is still in
flight. Any fix that keeps unwrap() (this PR) or that changes how the
record layer is flushed needs to be re-checked against that measurement;
simply removing the call is a data-loss regression.

Proposed fix

Keep the handshake, bound it, and never let it escape:

    _DATA_SHUTDOWN_TIMEOUT = 5.0

    def _shutdown_data_channel(conn, timeout=None):
        """Perform a best-effort TLS shutdown of a data connection.

        The data connection of an FTP transfer is ephemeral: it is created
        by transfercmd() for one transfer and closed right afterwards.
        Servers are not required to complete a TLS shutdown handshake on it
        and many do not send close_notify at all (gh-77303, gh-78738,
        gh-103443, gh-124850), which makes an unconditional unwrap() either
        raise or block forever *after* the whole payload has already been
        sent, turning a completed transfer into an apparent failure.  The
        handshake is still attempted (well-behaved peers get a clean
        close_notify), but it is time-limited and never propagates an
        error.
        """
        if timeout is None:
            timeout = _DATA_SHUTDOWN_TIMEOUT
        try:
            conn.settimeout(timeout)
            conn.unwrap()
        except (OSError, ValueError):
            # ConnectionResetError, SSLEOFError, TimeoutError and
            # ssl.SSLError are all OSError subclasses; ValueError covers
            # "No SSL wrapper around ...".
            pass

and each of the four methods calls it in place of conn.unwrap():

             if _SSLSocket is not None and isinstance(conn, _SSLSocket):
-                conn.unwrap()
+                _shutdown_data_channel(conn)

_shutdown_data_channel lives in the else: branch of the
try: import ssl block, exactly like _SSLSocket and FTP_TLS do, so
import ftplib still works when ssl is unavailable. The four call sites
are inside class FTP, which is defined before that block — the name is
resolved at call time, which is how _SSLSocket already works today.

Full patch: patch.diff — 1 file changed, 31 insertions(+), 8 deletions(-).

Design notes

• settimeout() before unwrap() is what fixes the hang. Verified
  empirically against a server that accepts the handshake and then never
  sends close_notify:

  socket timeout	unwrap() outcome
  None (blocking)	never returns (killed by an outer 45 s watchdog)
  2.0	TimeoutError after 2.00 s
  1.0	TimeoutError after 1.00 s
  0.5	TimeoutError after 0.50 s

  TimeoutError is an OSError subclass, so the except swallows it. The
  timeout is deliberately not restored: the socket is closed unconditionally
  a few lines later by the with statement.

• The except is (OSError, ValueError), not a bare except. It
  covers ConnectionResetError, SSLEOFError, TimeoutError,
  ssl.SSLError (all OSError) plus the ValueError that unwrap()
  raises if called twice. Programming errors still propagate.

• unwrap() is still attempted. See the table above — removing it
  truncates data.

• FTP_TLS.ccc() keeps its unconditional self.sock = self.sock.unwrap().
  That is the control connection, which is long-lived and whose shutdown
  the caller explicitly asked for. Only the per-transfer data channel is
  made best-effort.

• _DATA_SHUTDOWN_TIMEOUT is read at call time (the timeout=None
  sentinel), not baked in as a default argument, so it can be adjusted —
  and monkeypatched by tests — after import.

• 5 seconds only ever costs time against a peer that is already broken;
  against a healthy peer unwrap() completes in the time it takes to send
  one TLS record.

Alternatives considered

1. Delete the four unwrap() blocks. — Rejected; truncates data
   (measured above).
2. try: conn.unwrap() except ssl.SSLError: pass without a timeout. —
   Fixes ftplib: FTP_TLS seems to have problems with sites that close the encrypted channel themselfes #77303 / ftplib: FTP_TLS storbinary is hanging/never completing #103443 / ftplib storbinary  #124850 but not When sending binary file to a Microsoft FTP server over FTP TLS, the SSL unwind method hangs #78738: a silent peer
   still hangs the caller forever.
3. conn.shutdown(socket.SHUT_WR) instead of unwrap(). — Emits a TCP
   FIN without close_notify, i.e. exactly the ragged EOF that causes the
   truncation above. Rejected for the same reason as (1).
4. Only fixing storbinary/retrbinary. — retrlines() and
   storlines() have the same four lines, and nlst(), dir(), mlsd()
   all funnel through retrlines().

Tests

Two new methods in TestTLS_FTPClass
(tests.py), plus two existing tests that act as guards.

New — fail without the patch, pass with it:

$ python -m test test_ftplib            # patched
Ran 98 tests in 7.5s
OK (skipped=1)

$ python -m test test_ftplib            # unpatched
ERROR: test_data_shutdown_error_is_ignored (exc=ConnectionResetError(...))
ERROR: test_data_shutdown_error_is_ignored (exc=SSLEOFError(...))
ERROR: test_data_shutdown_error_is_ignored (exc=SSLError(...))
ERROR: test_data_shutdown_error_is_ignored (exc=ConnectionResetError(104, ...))
ERROR: test_data_shutdown_error_is_ignored (exc=ValueError(...))
FAIL:   test_data_shutdown_timeout_is_ignored
Ran 98 tests in 36.7s
FAILED (failures=1, errors=5, skipped=1)

test_data_shutdown_error_is_ignored drives all five failure modes from the
four issues through both retrbinary() and storbinary().
test_data_shutdown_timeout_is_ignored fails the moment unwrap() is
called without the socket timeout being lowered first, so it catches any
regression that drops the settimeout() — the half of the fix that is easy
to lose.

Existing — guard against "just delete it":

test_storbinary and test_storlines (inherited by
TestTLS_FTPClassMixin) fail with a truncated payload if unwrap() is
removed. No new test was written for that case on purpose; the existing
ones already cover it and were used to measure the truncation above.

Full matrix (CPython 3.13.3 system interpreter, Lib/ from
main @ 82c62ab, no C build required because ftplib and
test_ftplib are pure Python):

Lib/ftplib.py	test_ftplib with the 2 new tests
patched	98 tests OK (3/3 runs)
unpatched	6 failures (5 errors + 1 failure)

Test certificate/data: Lib/test/certdata/keycert3.pem — required, without
it every TLS test fails with a handshake timeout rather than a clean skip.

PR checklist

• Title is gh-103443: ftplib: make the TLS shutdown of the data connection best-effort — the gh-NNNNNN: prefix is mandatory.
• Description links all four issues. No new issue is needed: all four
  already exist and are open. Parent-issue discussion belongs on GitHub,
  not in the PR.
• NEWS entry: blurb add -i 103443 -s Library (or the manual file below)
  — Misc/NEWS.d/next/Library/...gh-issue-103443....rst (news.rst).
  Keep it to one paragraph, ≤80 columns, no leading -, no issue number
  in the body (it is in the file name), and no extra gh- numbers —
  sampled Misc/NEWS.d/next/Library/ entries never cite them. Optional
  trailing Patch by <your name>. (used by ~1 in 6 entries; never invent
  an attribution).
• Docs: Doc/library/ftplib.rst updated (docs.rst), and
  make -C Doc builds without warnings.
• Tests: tests.py inserted into TestTLS_FTPClass;
  ./python -m test test_ftplib green, ideally ./python -m test -j0.
• ./python Tools/patchcheck/patchcheck.py clean (NEWS, whitespace,
  docs presence).
• pre-commit run --all-files clean.
• CLA: after the PR is opened, cpython-cla-bot leaves a comment
  with a "click to sign" button — log in with GitHub, Authorize Python
  CLA Bot, Sign. One-time only, but it must cover every email
  address used in the commits, so set git config user.email
  beforehand.
• No force-pushes; reviewers rely on the commit history. CPython
  squash-merges in the end anyway.

Out of scope

• Implicit FTPS (RFC 4227). ftplib has no implicit-mode support and this
  is not the place to add it. A standalone reference implementation that
  also works around this bug on unpatched ftplib is kept next to this PR
  as ftplib_implicit.py — it is not meant to be
  merged into Lib/.
• FTP_TLS.ccc() and the control-connection shutdown.
• Changing FTP_TLS.transfercmd() or the PROT P handling.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #34990558 | 📁 Comparing 704990c against main (2639fd6)

  🔍 Preview build  

2 files changed

± library/ftplib.html
± whatsnew/changelog.html