fix: bind Unix socket listener in Rust for --listen-socket - #27
Merged
Merged
Conversation
The Rust implementation parsed --listen-socket but never bound a UnixListener, silently running TCP-only despite advertising Unix-socket support. This bypassed the socket-ownership security boundary the flag exists for, and made fd://3 systemd socket activation unimplementable. - Bind a real tokio::net::UnixListener for --listen-socket, served concurrently with the TCP listener via independent listener tasks coordinated by a broadcast shutdown channel. - Remove a stale socket file left by a prior run before binding, matching the Go implementation. - Support fd://3 systemd socket activation by adopting the fd (split into a small helper so the fd-adoption logic is unit-testable without touching the process's real fd 3). - Extract serve_connection<S> so both listeners share the hyper HTTP/1 serving path; each listener now fails independently (log + return) instead of crashing the whole process on bad config, matching Go's startListener behavior. - Add unit tests for stale-file cleanup, fresh bind, and raw-fd wrapping. Fixes #25
- Create both broadcast receivers before spawning the signal-handler tasks: a broadcast send with zero receivers is dropped, so a signal arriving before the listeners subscribed was silently lost. - Back off 100ms after a failed accept so persistent errors (EMFILE, non-listening fd under socket activation) don't busy-loop at 100% CPU. - Reword the from_raw_fd SAFETY comment: the fd comes from user input and may be invalid; misuse surfaces as io::Error, not UB.
Collaborator
Author
|
Self-review (dispatched independent code review against
Also reworded the Noted for follow-up issues (pre-existing, not regressions from this PR):
Verification after fixes: |
This was referenced Sep 11, 2026
Open
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #25 — the Rust implementation parsed
--listen-socketbut never bound aUnixListener, so it silently ran TCP-only. This bypassed the socket-ownership security boundary that flag exists for (SocketMode=0660/SocketGroupdeployments), and madefd://3systemd socket activation unimplementable.Changes (
rs/src/main.rsonly)tokio::net::UnixListenerfor--listen-socket, served concurrently with the TCP listener via two independent listener tasks coordinated by abroadcastshutdown channel (replacing the single-listenermpscshutdown).os.Removebehavior.fd://3systemd socket activation by adopting the fd (unix_listener_from_raw_fd, split out so the fd-adoption mechanics are unit-testable without touching the process's real fd 3).serve_connection<S>so both listeners share the hyper HTTP/1 serving path.startListenerbehavior.Out of scope (noted, not fixed here)
# surfaces 7 pre-existing, unrelated dead-code warnings inpolicy.rs/proxy.rs/middleware.rs(fields kept for YAML deserialization completeness, test helpers). Restored the attribute with a comment explaining why; left as a separate cleanup.--listen-socketas "Go/Rust only" — no doc changes needed.Verification
cargo build --release: clean, no warningscargo test(make test-rs): 115/115 passing (112 existing + 3 new)make lint-rs: clean