Skip to content

ABOR during the data-connection wait sends no 426/226 acknowledgement and orphans the transfer #205

Description

@mandipadk

Summary

If a client sends ABOR after PASV/RETR|LIST|STOR but before opening the data socket, the transfer worker is cancelled while parked in the ConnectionConditions data-connection wait. The cancellation bypasses @worker's handler, so the server sends neither the 426 nor the 226 acknowledgement the @worker contract promises, and the asyncio.shield-wrapped transfer future is left orphaned.

What happens

For transfer commands the decorators stack @ConnectionConditions(..., data_connection_made, wait=True) outside @worker inside the real coroutine (e.g. server.py:1238-1244 mlsd, 1297-1303 list). So ConnectionConditions.wrapper runs first:

# server.py  ConnectionConditions.wrapper
try:
    await asyncio.wait_for(asyncio.shield(aggregate), timeout)   # data-connection wait
except asyncio.TimeoutError:
    ...
    return True
return await f(cls, connection, *args, **kwargs)                  # -> @worker

abor (server.py:1592-1599) cancels each worker and sends no response itself, relying on @worker to emit 426/226. But the CancelledError raised at await asyncio.wait_for(asyncio.shield(aggregate), ...) is not a TimeoutError, so it skips the except and never reaches return await f(...) — meaning @worker's except asyncio.CancelledError (the only emitter of 426 transfer aborted / 226 abort successful, server.py:586-588) is never entered. The client gets zero acknowledgements, and because aggregate is asyncio.shield-wrapped, the inner gather/data_connection future is not cancelled and is left pending.

Reproduction

Self-contained — pip install aioftp, then python repro.py. Applies the real decorators (ConnectionConditions, worker) in the exact order the library uses for transfer commands (verbatim from Server.list/Server.mlsd), drives the real Server.abor and a real Connection; the data socket is the literal unresolved data_connection future. No FTP server/socket needed.

import asyncio
from aioftp.server import ConnectionConditions, worker, Server
from aioftp.common import Connection


def make_connection():
    conn = Connection()
    responses = []
    conn.response = lambda *args: responses.append(args)   # real response sink shape
    conn.wait_future_timeout = 1
    conn.extra_workers = set()
    conn["logged"].set_result(True)                         # satisfy abor's @ConnectionConditions
    assert not conn["data_connection"].done()              # the precondition: unresolved
    return conn, responses


def transfer_decorators(fn):
    # exactly as Server.list / Server.mlsd stack them (server.py:1238-1244, 1297-1303)
    return ConnectionConditions(
        ConnectionConditions.data_connection_made, wait=True,
        fail_code="425", fail_info="Can't open data connection",
    )(worker(fn))


async def main():
    server = Server.__new__(Server)        # only abor + fields are used
    server.wait_future_timeout = 1

    # ---- BUG: ABOR while the worker is parked in the data-connection wait ----
    conn, responses = make_connection()

    @transfer_decorators
    async def list_worker(self, connection, rest):
        connection.response("226", "list transfer done")   # never reached in the bug case
        return True

    task = asyncio.create_task(list_worker(server, conn, "."))
    conn.extra_workers.add(task)                            # as server.py:1321
    for _ in range(4):
        await asyncio.sleep(0)                              # let it park in ConnectionConditions.wrapper
    assert not task.done()

    await server.abor(conn, "")                             # REAL abor -> worker.cancel()
    try:
        await task
    except asyncio.CancelledError:
        pass

    dc = conn["data_connection"]
    print(f"responses sent: {responses}")
    print(f"data_connection future: done={dc.done()} cancelled={dc.cancelled()} (orphaned if pending)")
    assert responses == [], "expected ZERO acks (no 426/226)"
    assert not dc.done(), "expected the shielded transfer future to be orphaned"
    print("BUG CONFIRMED: ABOR during the data-connection wait sent no ack and orphaned the transfer")


asyncio.run(main())

Output:

responses sent: []
data_connection future: done=False cancelled=False (orphaned if pending)
BUG CONFIRMED: ABOR during the data-connection wait sent no ack and orphaned the transfer

Positive control: if data_connection is pre-resolved (so the cancel lands inside @worker's body instead), the normal path emits 426 then 226 — confirming the missing acks are specific to the pre-data-connection window. (Also reproduces deterministically under simloom, the scheduler this was found with.)

Suggested fix

Ensure a worker cancelled during the ConnectionConditions data-connection wait still emits the abort acknowledgements and tears down the shielded transfer — e.g. catch CancelledError in ConnectionConditions.wrapper (emit the 426/226 and cancel aggregate), or restructure so the @worker cancellation handler covers the pre-data-connection window.

Caveat

The harness applies the real decorators/classes but does not run the full Server.list end-to-end (which would require a started PASV server, path_io, and get_paths); the worker body is a no-op placeholder that is never reached in the bug case. A maintainer confirmation against a live ABOR-during-PASV handshake would strengthen the report.

Relationship to existing issues

Distinct from the known client-side abort issues (#163, #201, #202, #198), which all cancel after the data connection is established. This is the server-side, pre-data-connection window. #29 ("ABOR(T) races", closed 2016) is the opposite timing (worker already finished). No CHANGELOG entry or test (tests/test_abort.py aborts only during active transfers) covers this window.

Environment

  • aioftp 0.27.2 (commit 226ad64), CPython 3.11+

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions