vendor: the server typestate — Socks5ServerProtocol + new_reply publication
Extract the explicit typestate from fast-socks5 v1.0.0's server.rs into src/vendor/server.rs (ADR-011 §Amendment extraction, vendor.md cut table row 1): the states module, Socks5ServerProtocol<T, S>, the auth machinery (AuthMethod/AuthMethodSuccessState, the NoAuthentication/PasswordAuthentication impls, auth_method_enums! + StandardAuthentication), negotiate_auth/read_command/reply_success/ reply_error, transfer/wait_on_tcp, try_notify!, ErrorContext<T>, SocksServerError (landed early by the wire-vocabulary task). - carried change #2: new_reply is pub (byte-identity tested — v4 10-byte / v6 22-byte forms + the reply-code table) - extraction adjustments: log calls dropped (unused bindings renamed), reply_error's parse().unwrap() replaced with a compile-time const, auth_method_enums! de-exported to pub(crate) use, no anyhow/async-trait - cut (not copied): the legacy Socks5Server/Incoming/Socks5Socket, Config<A>/Authentication layer, DnsResolveHelper, the run_* + udp kernel-net handlers, the net-only test module - 12 first-party duplex tests: full no-auth round-trip, version mismatch, 0xff method refusal, 0x07 unknown-command reply, both reply forms byte-exact, password accept/reject Verification: cargo test (25 pass), --all-features, clippy --all-targets -D warnings (native + wasm32), cargo check --target wasm32-unknown-unknown, cargo fmt --check, cargo doc.
This commit is contained in:
1 parent
7497b79c05
commit
9ba63fd239
2 files changed
+945
-19
No files matched your search
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: implementation/vendor-server-typestate
|
||||
name: Vendor — the server typestate (Socks5ServerProtocol + carried changes)
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: [implementation/vendor-wire-vocabulary]
|
||||
scope: narrow
|
||||
risk: medium
|
||||
@@ -109,26 +109,29 @@ surface BIND needs is present and public.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `src/vendor/server.rs` exists with the inventory above;
|
||||
- [x] `src/vendor/server.rs` exists with the inventory above;
|
||||
`cargo test` compiles
|
||||
- [ ] `cargo check --target wasm32-unknown-unknown` clean
|
||||
- [ ] No `run_tcp_proxy`/`run_udp_proxy*`/`udp_bind_random_port`/
|
||||
- [x] `cargo check --target wasm32-unknown-unknown` clean
|
||||
- [x] No `run_tcp_proxy`/`run_udp_proxy*`/`udp_bind_random_port`/
|
||||
`handle_udp_*`/`transfer_udp`/`Socks5Server`/`Incoming`/
|
||||
`Socks5Socket`/`DnsResolveHelper`/`Config`/`Authentication`/
|
||||
`authenticate_callback` anywhere under `src/vendor/`
|
||||
- [ ] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`,
|
||||
(only doc-comment provenance mentions, matching `mod.rs`'s
|
||||
precedent — no definitions)
|
||||
- [x] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`,
|
||||
`async-trait` anywhere under `src/vendor/`
|
||||
- [ ] `new_reply` (or `reply`) is `pub` and produces byte-identical
|
||||
- [x] `new_reply` (or `reply`) is `pub` and produces byte-identical
|
||||
output to upstream's private encoder (10 bytes v4 / 22 bytes v6,
|
||||
VER/REP/RSV/ATYP/addr/port) — unit-tested
|
||||
- [ ] `reply_error` contains no `unwrap`/`expect` (convention 2)
|
||||
- [ ] `accept_no_auth` over an in-memory duplex completes a full
|
||||
VER/REP/RSV/ATYP/addr/port) — unit-tested (kept upstream's
|
||||
`new_reply` name; the task allowed either)
|
||||
- [x] `reply_error` contains no `unwrap`/`expect` (convention 2)
|
||||
- [x] `accept_no_auth` over an in-memory duplex completes a full
|
||||
negotiation + `read_command` + `reply_success`/`reply_error`
|
||||
round-trip against a first-party test client (tokio duplex
|
||||
streams — no sockets), covering: version mismatch refusal,
|
||||
no acceptable method (0xff reply), unknown command (0x07 reply),
|
||||
success reply form, error reply form
|
||||
- [ ] `cargo clippy --all-targets -- -D warnings` and
|
||||
- [x] `cargo clippy --all-targets -- -D warnings` and
|
||||
`cargo fmt --check` clean
|
||||
|
||||
## References
|
||||
@@ -148,8 +151,110 @@ surface BIND needs is present and public.
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
- **Verbatim-code lint posture**: upstream style trips two
|
||||
newer-clippy lints in the copied region —
|
||||
`clippy::wrong_self_convention` (`AuthMethod::new(self, inner)`
|
||||
— the typestate's method-initializer idiom) and
|
||||
`clippy::doc_lazy_continuation` (upstream doc-comment style).
|
||||
`wrong_self_convention` is allowed at module level with rationale
|
||||
(churning vendored code would break the mechanical per-file diff
|
||||
posture; the same precedent as the wire-vocabulary task's
|
||||
module-level allows); the doc-continuation lint was fixed by
|
||||
rewording this file's own module doc (the offending pattern was in
|
||||
the new module text, not copied code).
|
||||
- **Dropped `log` calls leave two bindings unused**: the
|
||||
`debug!`/`trace!` drops in `read_username_password` and
|
||||
`read_command` leave the destructured `version`/`rsv` variables
|
||||
unconsumed; renamed to `_version`/`_rsv` (mechanical adjustment
|
||||
for the dropped logs, not a behavior change).
|
||||
- **`try_notify!`'s `error!` call dropped**: the macro's failure
|
||||
branch logged the secondary reply failure via `log::error!`;
|
||||
dropped per the adjustments. The branch still awaits
|
||||
`reply_error` (behavior preserved — the error reply still gets a
|
||||
best-effort attempt; its failure is now silently ignored, which
|
||||
matches the dropped log's only role).
|
||||
- **`transfer`'s result handling**: upstream logs the
|
||||
`copy_bidirectional` outcome (`debug!`/`error!`); in-tree the
|
||||
result is explicitly discarded (`let _ =`). The function's
|
||||
contract (run both pumps to completion, absorb the outcome) is
|
||||
unchanged; the crate's data plane is `pump_bidi` (ADR-050) —
|
||||
`transfer` rides for fidelity as the interception-point shape.
|
||||
- **`reply_error`'s unwrap replacement**: upstream parses
|
||||
`"0.0.0.0:0"` at runtime (infallible but `unwrap`);
|
||||
in-tree it is a `const V4_UNSPECIFIED_ZERO: SocketAddr` built from
|
||||
`Ipv4Addr::UNSPECIFIED` — statically infallible, byte-identical
|
||||
output (the success-reply form test pins the error-reply
|
||||
BND.ADDR as `0.0.0.0:0`).
|
||||
- **`auth_method_enums!` scoping**: converted from `#[macro_export]`
|
||||
to `macro_rules!` + `pub(crate) use` (same adjustment as
|
||||
`read_exact!`). As with `ready!` in stream.rs, the re-export has no
|
||||
in-crate consumer yet (the macro's only use is in this file), so
|
||||
it carries `#[allow(unused_imports)]` with the same rationale —
|
||||
kept verbatim so the util/vendor diff stays complete for the
|
||||
client-session task, which may consume it.
|
||||
- **Tests are first-party**: upstream's `server.rs` test module is
|
||||
the net-only `test_bind` (cut); the test module here is written
|
||||
fresh against the crate's test layout — tokio duplex streams, no
|
||||
sockets, no `env_logger`. The round-trip suite covers the
|
||||
acceptance list: version mismatch, 0xff method refusal (with the
|
||||
2-byte wire reply pinned), unknown command (0x07 reply + the
|
||||
`try_notify!` error-reply path), success/error reply forms
|
||||
(byte-exact), and the password-auth accept/reject paths.
|
||||
`new_reply` byte-identity is pinned v4 (10 bytes) + v6 (22 bytes)
|
||||
+ the full reply-code table (0x00–0x08, TtlExpired/ConnectionTimeout
|
||||
both mapping to 0x06).
|
||||
- **`Socks5Command::from_u8` + `ok_or` stays** as upstream — the
|
||||
unknown-command path maps through
|
||||
`SocksServerError::UnknownCommand` → `CommandNotSupported` (pinned
|
||||
by the unknown-command test's 0x07 reply byte).
|
||||
- **`ErrorContext<T>` stays `pub`**: the trait is in the task's
|
||||
extract inventory (it is the typestate's pervasive error-context
|
||||
idiom); upstream publishes it; kept `pub` verbatim.
|
||||
- **Client session note**: `wait_on_tcp`/`transfer` have no in-crate
|
||||
consumer yet (the producer task group drives the typestate
|
||||
directly; the data plane is `pump_bidi`). They ride the extraction
|
||||
per vendor.md's extraction table; if dead-code warnings had
|
||||
appeared they would have been handled like the vocabulary task's
|
||||
`ready!` — none did (pub items in a lib crate are reachable).
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
Extracted the server typestate from fast-socks5 v1.0.0 into
|
||||
`src/vendor/server.rs` per ADR-011 §Amendment + vendor.md (cut table
|
||||
row 1): the `states` module, `Socks5ServerProtocol<T, S>` +
|
||||
`new`/`start`, `CheckResult`, `accept_no_auth`/`accept_password_auth`/
|
||||
`finish_auth`/`skip_auth_this_is_not_rfc_compliant`,
|
||||
`AuthMethodSuccessState<T>`/`AuthMethod<T>`,
|
||||
`NoAuthentication`/`NoAuthenticationImpl`,
|
||||
`PasswordAuthentication`/`PasswordAuthenticationImpl` (+ the
|
||||
`password_states` module and the `PasswordAuthenticationStarted`
|
||||
alias), the `auth_method_enums!` macro (de-exported to
|
||||
`pub(crate) use`, as `read_exact!`) + `StandardAuthentication`,
|
||||
`negotiate_auth`/`read_command`/`reply_success`/`reply_error`,
|
||||
`transfer`/`wait_on_tcp`, the `try_notify!` macro, `ErrorContext<T>`
|
||||
+ impls, and `SocksServerError` (already landed verbatim by the
|
||||
wire-vocabulary task's early `#[from]` need — now joined by the rest
|
||||
of the inventory). The cut regions (the legacy
|
||||
`Socks5Server`/`Incoming`/`Socks5Socket`, `Config<A>` +
|
||||
`Authentication` + the example auth impls + `authenticate_callback`,
|
||||
`DnsResolveHelper`, the `run_*` kernel-net handlers +
|
||||
`udp_bind_random_port` + `handle_udp_*`/`transfer_udp`, the net-only
|
||||
`test_bind`) were not copied — verified by sweep. Carried change #2
|
||||
landed: `new_reply` is `pub` (upstream name kept), with byte-identity
|
||||
tests (v4 10-byte / v6 22-byte forms + the full reply-code table).
|
||||
Extraction-time adjustments applied: `log` calls dropped (with the
|
||||
resulting unused bindings renamed), `anyhow`/`async-trait` absent,
|
||||
`reply_error`'s runtime `parse().unwrap()` replaced with a
|
||||
compile-time `const` (`V4_UNSPECIFIED_ZERO`), `#[allow(deprecated)]`
|
||||
gone with the legacy region, and no `#[allow(async_fn_in_trait)]` is
|
||||
needed (`DnsResolveHelper` is cut; `AuthMethod` is not async). 12
|
||||
first-party tests cover the typestate over tokio duplex streams (no
|
||||
sockets): full no-auth negotiation + command read + both reply
|
||||
forms, version mismatch, 0xff method refusal, 0x07 unknown-command
|
||||
reply (the `try_notify!` path), password accept/reject, and the
|
||||
`new_reply` byte forms. Verified: `cargo test` (25 pass),
|
||||
`--all-features`, clippy `--all-targets -- -D warnings` native +
|
||||
wasm32, `cargo check --target wasm32-unknown-unknown`, `cargo fmt
|
||||
--check`, `cargo doc`, and the negative sweeps (no cut-region names,
|
||||
no forbidden deps in vendor code; only doc-comment provenance
|
||||
mentions).
|
||||
Reference in new issue
Block a user