aio-libs / aio-libs/aioftp

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

Abierto
#205 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Python
Estrellas
211
Forks
63
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.