Allow ssl= on Server.start and fix the README server example

[ekanshul]

Summary

Server.start / Server.run accept **kwargs typed as AsyncIOStartServerKwargs, which includes ssl. Those kwargs were forwarded to asyncio.start_server together with ssl=self.ssl, so:

await server.run(host="127.0.0.1", port=8025, ssl=ctx)

raised TypeError: got multiple values for keyword argument 'ssl'.

ssl is now taken from kwargs when provided (and still used for PASV listeners via self.ssl).

The getting-started server snippet also used undefined user and path_io_factory names. It now matches the tutorial default (aioftp.Server()).

Fixes #204
Fixes #203

Test plan

• New unit test: test_start_ssl_kwarg_does_not_conflict
• python -m pytest tests/test_connection.py tests/test_tls.py — 57 passed

[greptile-apps]

Confidence Score: 2/5

[Medium risk] Allows SSL context to be passed to the FTP server startup.

Not safe to merge until the persistent SSL override and the example’s anonymous file access are addressed. The test-coverage gap is non-blocking.

_{Reviews (1) · Last reviewed commit: "Allow ssl= on Server.start and fix the R..."}

[greptile-apps]

SSL override survives restart

If a server is reused after start(ssl=...), this assignment leaves the override in self.ssl. Neither close() nor a failed start restores the constructor setting. A later start() without ssl= therefore gives control and passive listeners an unexpected TLS configuration, disrupting clients that rely on the original setting. Restore the intended setting before merging.

Knowledge Base Used: Asynchronous FTP server

Artifacts

FTP TLS lifecycle test script

• This authored script runs the pre-PR or PR server source through real control, passive, close, failure, and restart checks; it defines the shared test.

Lifecycle output capture command

• This authored command wrapper records each executed command, working directory, exit code, and process output; it produces the two comparable logs.

Pre-PR FTP TLS lifecycle output

• The executed pre-PR source rejected `start(ssl=...)` and served plaintext control and passive connections on later starts; no override persisted.

PR-head FTP TLS lifecycle output

• The executed PR source served TLS control and passive connections after restart without `ssl`, including after a failed bind; the override persisted.

_{Ran code and verified through T-Rex}

[greptile-apps]

Example exposes writable files

If someone runs this now-working example on a network-reachable machine, it serves the process’s current directory with anonymous read and write access; run() does not restrict the bind to loopback. Clients could read or change those files. The same example appears in docs/index.rst. Restrict the directory and bind, or clearly warn readers about these defaults, before merging.

How this was verified: An anonymous connection read and uploaded files, and the example requested the unrestricted default host.

Knowledge Base Used: Server users and permissions

Artifacts

Isolated README server example probe

• The authored harness extracts and executes each README example in a temporary directory, intercepts wildcard binds, and uses a loopback FTP client to test access; it avoids exposing the repository or a network listener.

Original README example fails before starting

• The command executed the parent-commit example and captured its exit code and `NameError`; the original example never started a service.

Changed README example accepts anonymous reads and writes

• The command executed the changed example and captured its default bind request plus successful anonymous RETR and STOR over a loopback-only connection; the unsafe default permissions are reproducible.

_{Ran code and verified through T-Rex}

[greptile-apps]

Passive TLS remains untested

This test checks only the control listener’s ssl argument. It still passes when passive listeners receive no TLS context, although a client then cannot complete a passive data-connection TLS handshake. Cover a passive connection started with ssl= so a future regression does not break TLS transfers unnoticed. This test gap does not itself block merging.

Knowledge Base Used: Server data connections and transfers

Artifacts

Source for the targeted test and passive TLS probe

• This isolated script runs the PR test and a real FTP exchange, optionally suppressing TLS only during passive-listener creation; it shows exactly what was executed.

Unmodified server: targeted test passes and passive TLS works

• The captured baseline command exits 0, reports the full FTP responses, and completes a passive TLS handshake; the unmodified path works.

Passive TLS suppressed: targeted test still passes

• The captured perturbed command exits 0 and reports the FTP responses, but the passive TLS handshake times out; the targeted test misses the failure.

_{Ran code and verified through T-Rex}

[greptile-apps]

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

•  Runnable getting-started example exposes anonymous read/write access ▶

  • Bug
    • The newly runnable example uses the current working directory as its file root and permits an anonymous FTP client to read and write files. Its default bind request uses host=None, which Python treats as an all-interface bind. Someone running the example from a sensitive directory on a reachable machine could expose or allow changes to its files.
  • Cause
    • Changed README.rst:176 and the duplicate docs/index.rst:147 replace undefined user and path_io_factory references with aioftp.Server(). Its default user has no login requirement, uses Path('.'), and has readable and writable permissions; server.run() supplies no host restriction.
  • Fix
    • Make the getting-started example explicitly use a loopback host and a dedicated, restricted directory with appropriate permissions, or prominently warn readers before presenting the unrestricted example.

•  Start-time TLS override persists into subsequent starts ▶

  • Bug
    • After start(ssl=context), closing and restarting the same Server without ssl still enables TLS on control and EPSV passive sockets. A failed start(ssl=context) likewise leaves the override in place for the next successful start.
  • Cause
    • src/aioftp/server.py lines 796–797 assign the override to self.ssl before the bind succeeds. Line 803 uses that stored value for control connections; lines 1498 and 1514 use it for passive connections. close() does not restore the prior value.
  • Fix
    • Keep the constructor TLS setting separate from a start-time override, and use a per-start effective context for both control and passive listeners; do not commit an override to persistent state before a successful start.

•  SSL kwarg test does not detect missing passive-listener TLS ▶

  • Bug
    • tests/test_connection.py:345-356 mocks asyncio.start_server and checks only the call made by Server.start. Suppressing TLS during passive-listener creation leaves this test passing while a client cannot complete a TLS handshake on the PASV data port.
  • Cause
    • The test never enters the PASV path at src/aioftp/server.py:1519-1562 or checks the passive asyncio.start_server calls at lines 1494-1499 and 1510-1515.
  • Fix
    • Add a test that starts with ssl=, requests PASV, and verifies the passive listener receives that context or completes a TLS data-connection handshake.