# PLAN-acceptor.md — Let a caller supply accepted connections
> Scope note: this plan does **not** add TLS to `mini-static`. TLS stays a non-goal
> (README, *Non-goals*). The problem is narrower and entirely structural: the crate owns
> its listener, so the only way to serve anything other than plain TCP is to abandon the
> hardened accept loop. This plan opens that seam.
## Where We Are
`run_on(addr, header_timeout)` calls `TcpListener::bind(addr)` itself
(`src/server.rs:1089`) and returns `(u16, ServerHandle)`. `run`, `run_all`, and
`run_ephemeral` all delegate to it. The accept loop it spawns provides, in one place:
- a connection-count ceiling via an `OwnedSemaphorePermit` per connection
(`DEFAULT_MAX_CONNECTIONS = 1024`, `Server::with_max_connections`);
- exponential accept-error backoff, 10 ms doubling to a 1 s cap, so a sustained
`accept()` failure degrades instead of ending the loop or busy-spinning;
- `header_read_timeout` and `max_buf_size` per request, via hyper's `http1::Builder`
with a `TokioTimer` installed;
- one log line per request and per connection/accept error, when configured;
- graceful shutdown: `ServerHandle::shutdown()` stops accepting and drains in-flight
connections for up to 5 s before aborting the `JoinSet`.
The loop is already generic over *where connections come from*: a private `TcpAccept`
trait (`src/server.rs:45`) exists so accept-error backoff can be tested against a
listener that fails on demand. But `TcpAccept::accept` returns a concrete
`(TcpStream, SocketAddr)`, and `serve_connection` takes a concrete `TcpStream`, so the
seam admits only plain TCP.
Consequently an embedder who needs TLS (or a Unix socket, or an in-process duplex
stream) must call `Server::handle_request` — which is public — from an accept loop of
their own, and reimplement or forgo every bullet above. The crate's headline feature is
connection-lifecycle safety, and the only supported path to TLS discards it.
## Where We Need To Be
- A caller can supply accepted connections of their own stream type and get **the same**
accept loop: same connection ceiling, same backoff, same per-request header bounds,
same logging, same `ServerHandle` drain semantics. Nothing in the list above is
reimplemented by the caller or silently skipped.
- Doing so requires no TLS dependency in `mini-static`, and no change to its dependency
tree.
- A per-connection setup step (a TLS handshake, most obviously) runs **inside the
spawned connection task**, under its own timeout — never on the accept loop, where a
single slow or hostile peer would stall every other pending connection.
- `run`, `run_on`, `run_all`, and `run_ephemeral` keep their exact current signatures and
behavior; existing callers are untouched.
- The seam is demonstrated against a real TLS acceptor, not merely asserted to fit.
## Commits
### 1. Make the connection path generic over its stream type
- Does: Change `serve_connection` to take
`impl AsyncRead + AsyncWrite + Unpin + Send + 'static` instead of `TcpStream`. The
`'static` is load-bearing and not optional: the connection future is handed to
`JoinSet::spawn`, so the stream it owns must outlive the caller's frame. No public API
change, no behavior change.
- Verifies: the full existing suite passes unchanged — `tests/server.rs`,
`tests/keep_alive_bounds.rs`, and `tests/live_reload.rs` in particular, since they
cover the timeout, ceiling, drain, and SSE paths. A unit test serves one request over
a `tokio::io::duplex` pair, proving a non-`TcpStream` stream reaches `handle_request`.
- Touches: `src/server.rs` (~15 LOC), one unit test (~40 LOC).
- Reverts cleanly: yes — self-contained signature change.
### 2. Public `ConnectionSource` trait, replacing the private `TcpAccept`
- Does: Promote `TcpAccept` to a public `ConnectionSource`, in its **final** shape —
see the note below on why the shape cannot be grown incrementally:
```rust
pub trait ConnectionSource: Send + Sync + 'static {
type Raw: Send + 'static;
type Stream: AsyncRead + AsyncWrite + Unpin + Send + 'static;
fn accept(&self) -> impl Future<Output = io::Result<Self::Raw>> + Send;
fn ready(&self, raw: Self::Raw) -> impl Future<Output = io::Result<Self::Stream>> + Send;
}
```
Three constraints forced this exact shape, each verified against the compiler rather
than assumed:
- **RPITIT with an explicit `+ Send`, not `async fn`.** A public trait with `async fn`
trips the `async_fn_in_trait` lint, which under this repo's `-D warnings` gate is a
build failure, not a warning. More importantly the bound is *needed*: the accept loop
spawns per-connection tasks, so a future without a guaranteed `Send` would not
compile at the spawn site. Implementors may still write `async fn` in their `impl`.
- **Two associated types, not one with a default.** `type Ready = Self::Stream;` does
not compile on stable — associated type defaults are unstable (rust#29661). `Raw`
and `Stream` are therefore both explicit; identity sources set them to the same type.
- The `SocketAddr` the current trait returns is discarded at its only call site, so it
is dropped rather than carried (see Open Questions if logging ever wants it).
Implement for `TcpListener` (`Raw = Stream = TcpStream`, `ready` returns its input).
`accept_and_permit` becomes generic. The internal loop calls `ready` **inside the
spawned connection task** from the outset — commit 4 adds its timeout, but the correct
call site is established here rather than moved later.
- Verifies: existing `tests/unit/server/accept.rs` backoff tests pass against the
reshaped trait; `TcpListener`'s impl is exercised by every existing socket test;
`cargo clippy --all-targets -- -D warnings` passes, which is the check that would fail
on the `async fn` form.
- Touches: `src/server.rs` (~50 LOC), `tests/unit/server/accept.rs` (~15 LOC), export in
`src/lib.rs`.
- Reverts cleanly: yes, together with commit 1 if both revert; alone it restores a
private trait with no callers outside the module.
- **Why the shape is final here, not grown in commit 4:** this commit publishes the
trait. Adding `ready` or splitting `Raw` out afterwards would be a breaking change to
an already-published public API, in the middle of the plan's own sequence. Commit 4 is
therefore about *bounding* `ready`, never about introducing it.
### 3. `run_with_source` entry point
- Does: Add
`pub async fn run_with_source<S: ConnectionSource>(&self, source: S, header_timeout: Duration) -> Result<ServerHandle, StaticError>`,
running the identical accept loop over `source`. Returns only a `ServerHandle` — the
caller owns the listener and already knows its address.
`run_on` does considerably more than bind, and all of it must move with the loop, not
stay behind: the required-tool-binary check that currently fails *before* binding, the
live-reload `Broadcaster` construction, `start_watching`, the `SourcePipeline` startup
build, and the `server.broadcaster = Some(..)` mutation that the accept loop's `Server`
clone depends on (`src/server.rs:1092-1150`). The refactor extracts everything except
the bind into one private function; `run_with_source` and `run_on` are then both thin
wrappers over it. Getting this wrong is silent: a TLS-served site would start with no
live-reload, no watcher, and no startup build, and nothing would report it.
- Verifies: a test drives `run_with_source` with a plain `TcpListener` and asserts a
served `200`; a second asserts `with_max_connections(1)` still bounds concurrency
through this path; a third asserts `handle.shutdown()` still drains; a fourth asserts a
configured-but-missing CSS/JS tool still fails with `PipelineSetup` through this path,
proving the pre-bind check moved rather than vanished. Each mirrors an existing
`run`-based test, so divergence between the two paths shows up as a failure.
- Touches: `src/server.rs` (~60 LOC, mostly extraction), new `tests/connection_source.rs`
(~150 LOC), README.
- Reverts cleanly: yes — additive entry point, plus an extraction that is inert on its
own.
- Note: `run_ephemeral`'s loopback-only guarantee (`127.0.0.1:0`, a documented security
property) does **not** extend here. A caller supplying their own source chooses their
own bind address; document that on the method.
### 4. Bound the per-connection setup step
- Does: Wrap the `ready` call (already sited in the spawned connection task by commit 2)
in `header_timeout`, so a peer that opens a connection and then stalls mid-handshake is
dropped on the same schedule as one that stalls mid-header. A `ready` failure —
handshake rejected, client vanished — drops the connection, releases its permit, and
logs `connection setup failed: <cause>` through the existing sink; before this, a
failure would be indistinguishable from an idle connection. Document on the trait that
`accept` must not perform negotiation, and why: accepting is serialized, so a handshake
there would queue every pending connection behind the slowest peer.
- Verifies: a fake source whose `ready` sleeps past `header_timeout` — assert that
connection is dropped, that a fast connection offered concurrently is served *while*
the slow one is still stalling (this is the assertion that actually proves the accept
loop stayed free, and the test is worthless without it), that the slow connection's
permit is released so `with_max_connections(1)` recovers, and that the failure is
logged. A second fake whose `ready` returns `Err` asserts the same drop-and-release
path without the timeout.
- Touches: `src/server.rs` (~30 LOC), `tests/connection_source.rs` (~100 LOC).
- Reverts cleanly: yes — reverting restores an unbounded `ready`, which is commit 2's
behavior, without touching the trait's public shape.
### 5. Prove the seam against real TLS
- Does: Add `tokio-rustls` and `rcgen` as **dev-dependencies only**, plus
`examples/tls.rs` and an integration test serving HTTPS through `run_with_source` with
a self-signed certificate. No change to `[dependencies]`.
- Verifies: the test — **not** `#[ignore]`d, unlike `tests/real_tools.rs`, because
`rcgen` mints the certificate in-process and nothing external has to be installed; an
ignored test proves the seam fits rustls only on the days someone remembers to run it,
which defeats the commit's entire purpose — completes a real TLS handshake and receives
a `200`, and asserts the connection ceiling, header timeout, and graceful shutdown
still apply on that same TLS server.
- Touches: `Cargo.toml` (dev-deps + example), `examples/tls.rs` (~80 LOC), one test
(~60 LOC), README.
- Reverts cleanly: yes — dev-only, nothing in the library depends on it.
## Open Questions
- **Naming.** `ConnectionSource` over `Accept` because the latter reads as a verb and
collides conceptually with hyper/tower's own `Accept` notions; a reader seeing
`impl ConnectionSource for TlsListener` should not have to check which `Accept` is
meant. Worth one more look before commit 2 makes it public API.
- **Trait shape — resolved, by compiling it.** The originally-proposed defaulted
associated type (`type Ready = Self::Stream;`) does not build on stable, and a public
`async fn` in a trait fails this repo's `-D warnings` gate. Both were checked against
`rustc`/`clippy` rather than reasoned about; the shape now in commit 2 compiles clean
under `-D warnings`, including a spawn site that forces the `Send` bounds to be real.
What remains genuinely open is only *ergonomics*: whether requiring implementors to
name both `Raw` and `Stream` is annoying enough to justify a `Connecting`-future
variant. Judge that with commit 5's rustls impl in hand — it is the only implementor
whose ergonomics can be judged rather than imagined.
- **No dynamic dispatch.** RPITIT is not dyn-compatible, so `Box<dyn ConnectionSource>`
is impossible by construction. Fine for a generic parameter; confirm no backstack
consumer wants dynamic dispatch before commit 2 publishes the shape, because adding it
later means a different trait, not a compatible extension.
- **MSRV.** RPITIT requires Rust 1.75 and the crate declares no `rust-version`. Commit 2
should add one rather than leave the requirement implicit and discovered by a
consumer's failing build — but check what the rest of the workspace pins first, since
an MSRV that contradicts its siblings is worse than none.
- **Peer address.** The current trait returns a `SocketAddr` that the accept loop throws
away. Dropping it is right for now, but if request logging later wants a client
address, it has to come back — as an `Option` on the source, since a Unix socket or a
duplex pair has no meaningful one. Flagged, not built.
- **Does anything else need this?** The Unix-socket and in-process-duplex cases are
hypothetical; TLS is the concrete driver. If TLS turns out not to be a real
requirement for any backstack consumer, this plan is speculative work and should not
be executed — the README correction alone (0.28.2) is then the whole fix. **This is
the gating question, and it is not a formality:** every commit here is well-shaped, and
none of them is worth executing against a need nobody has. Answer it before commit 1.
## Grill Verdict
Round: 1
Status: PASS
Findings resolved:
- [Commit 4, G1] `type Ready = Self::Stream;` does not compile — associated type
defaults are unstable (rust#29661). Verified against `rustc`, not assumed. The trait
now carries two explicit associated types, `Raw` and `Stream`.
- [Commit 2, G1] A public trait with `async fn` trips `async_fn_in_trait`, which under
this repo's standing `-D warnings` gate is a *build failure*. Verified with `clippy`.
Switched to RPITIT with an explicit `+ Send`. The private `TcpAccept` never hit this
because the lint exempts private traits — so the existing code gave no warning that
publishing it would break the build.
- [Commit 2, G1] The `+ Send` on the returned futures is required, not cosmetic: the
loop spawns per-connection tasks, and a non-`Send` future would fail at the spawn
site. Proved by compiling a spawn against the trait.
- [Commits 2/4, G3+P3] The original ordering published a one-type trait in commit 2 and
reshaped it in commit 4 — a breaking change to already-published public API inside the
plan's own sequence. Commit 2 now defines the final shape and sites `ready` correctly;
commit 4 only bounds it.
- [Commit 1, G3] The stream bound omitted `'static`, which the `JoinSet::spawn` site
requires. Stated, with the reason.
- [Commit 3, G1] "Refactored to bind and delegate" hid the load-bearing part: `run_on`
also does the tool-binary pre-check, broadcaster construction, `start_watching`,
the pipeline startup build, and the `broadcaster` mutation the loop's `Server` clone
depends on. Left behind, a TLS-served site would silently start with no live-reload,
no watcher, and no startup build. Now specified as a single extraction, with a test
asserting the pre-bind tool check survived the move.
- [Commit 3, G3] `run_ephemeral`'s loopback-only guarantee does not extend to a
caller-supplied source; now documented on the method.
- [Commit 4, G1] Unstated failure path: what happens when `ready` errors. Now drops the
connection, releases the permit, and logs — with a test for the `Err` path distinct
from the timeout path.
- [Commit 4, G1] The stall test needed the assertion that actually proves the point — a
fast connection served *while* the slow one stalls. Without it the test passes on an
implementation that blocks the accept loop.
- [Commit 5, G2] `#[ignore]` was copied from `tests/real_tools.rs`, whose reason
(external binaries on `PATH`) does not apply — `rcgen` mints certs in-process. An
ignored test cannot discharge this commit's stated purpose of proving the seam fits
rustls. Now runs by default.
Findings accepted as tradeoffs:
- [Commit 5, G2] Two dev-dependencies (`tokio-rustls`, `rcgen`) with sizeable trees,
slowing every test build, to exercise a seam no `[dependencies]` entry touches.
Reason: the alternative is asserting the design fits rustls without ever compiling it
against rustls, which is the specific failure this plan exists to avoid repeating.
- [Whole plan, G2] The plan may be speculative in its entirety. Reason: it is gated on
the final open question above rather than resolved here, because whether a backstack
consumer needs TLS is not a fact discoverable from this repo.