aio-libs / aio-libs/aioftp

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

Aperta
#205 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
211
Fork
63
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### 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:

```python
# 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.

```python
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](https://pypi.org/project/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+

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.