--- id: implementation/vendor-client-session name: "Vendor (the client session, Socks5Stream::use_stream path)" status: completed depends_on: [implementation/vendor-wire-vocabulary] scope: narrow risk: low impact: component level: implementation --- ## Description Extract the client session, the consumer half's stream-session base (ADR-002; POC #1's fit) into `src/vendor/client.rs`: the `Socks5Stream::use_stream` path plus its `AsyncRead`/`AsyncWrite` impls, generic over the backing stream. The consumer layer (a later task group) opens a channels `BiStream` and feeds it here; the first-party associate session in `consumer.rs` was informed by this shape (vendor.md's extraction table). **What is extracted** (upstream `src/client.rs` → `src/vendor/client.rs`): 1. `Socks5Stream` struct + the generic impl block, verbatim: `use_stream`, `request`, `send_version_and_methods`, `which_method_accepted`, `use_password_auth`, `request_header`, `read_request_reply`, `get_socket`/`get_socket_ref`/`get_socket_mut`. 2. The `AsyncRead`/`AsyncWrite` impls for `Socks5Stream` (upstream `client.rs:641–681`) verbatim; the consumer's session rides these (a `Socks5Stream` is itself a stream, so `pump_bidi`-shaped consumers and the front doors can pipe through it). 3. The client `Config` struct, **pruned**: upstream carries `connect_timeout` (only consumed by the cut `connect_raw`) and `skip_auth` (consumed by `use_stream`). Keep `skip_auth`; drop `connect_timeout` and its setter (its only consumer is cut). If dropping it leaves `Config` a single-field struct, keep it anyway (fidelity: the field is load-bearing in `use_stream`'s handshake branch) or inline the flag, which is the implementer's choice, but the `use_stream` behavior must stay byte-identical. **What is NOT extracted** (vendor.md's cut table row 2): - `Socks5Datagram` (struct + impl, the kernel `UdpSocket` associate client; the crate's associate session is first-party in `consumer.rs` per ADR-003's handle swap). - The `Socks5Stream` convenience impl (`connect`/`connect_with_password`/`connect_raw`, the `tokio::net::TcpStream` dialers; the consumer's transport is a channels `BiStream`, not a TCP dial). **Extraction-time adjustments** (mechanical): - Drop `anyhow::Context`: upstream wraps nearly every client error in `.context(...)`. The error surface is `SocksError` (thiserror): replace `.context("...")?` with plain `?` where the error variant already carries the meaning, and with the typed variants (`SocksError::Io`, `AuthMethodUnacceptable`, ...) where upstream used `anyhow::Error::msg(...).into()` (upstream `client.rs:285`, the missing-target-addr case) or `SocksError::Other(anyhow!(...))` (upstream `client.rs:528`, but that site is inside the cut `Socks5Datagram`, so it goes with the cut). No behavior change to the wire: the same bytes are read/written; only the error typing changes. - Drop `log` calls (`debug!` in the extracted region). - `use_password_auth`'s `unreachable!()` (upstream `client.rs:207`: `methods.get(1)` being `Some(AuthenticationMethod::None)`): convention 2 forbids panics in library code. Replace with a typed error path (`SocksError::AuthenticationRejected(...)` or an `AuthMethodUnacceptable`); the state is reachable only by misuse of the public API, but the faithful translation is an error return, not a panic. - `client_src`-style `"[::]:0".parse().unwrap()` sites are all inside the cut `Socks5Datagram` region, so verify none remain. ## Acceptance Criteria - [x] `src/vendor/client.rs` exists with the inventory above; `cargo test` compiles - [x] `cargo check --target wasm32-unknown-unknown` clean - [x] No `Socks5Datagram`, no `Socks5Stream` convenience impl (`connect`/`connect_with_password`/`connect_raw`) anywhere under `src/vendor/` - [x] No `anyhow`, `log`, `tokio::net`, `UdpSocket` anywhere under `src/vendor/` - [x] No `unwrap`/`expect`/`unreachable!`/panic paths in the extracted client code (convention 2) - [x] `Socks5Stream::use_stream` over an in-memory duplex completes: no-auth handshake, password handshake (both accept and reject paths), CONNECT request + reply read (success and each mapped failure code), domain-target and IP-target request encoding, against a raw-byte mock server (pinned byte sequences written directly, not the vendored server typestate; this keeps this task parallel with the typestate extraction and the client's wire expectations uncorrelated from the server implementation; tokio duplex streams, no sockets) - [x] `Socks5Stream` implements `AsyncRead` + `AsyncWrite` (the impls extracted) and the impls are exercised by at least one test (e.g. `tokio::io::copy` through the session) - [x] `cargo clippy --all-targets -- -D warnings` and `cargo fmt --check` clean ## References - docs/architecture/vendor.md (the extraction table row 5, the cut table row 2) - docs/architecture/decisions/002-protocol-base.md (the client side: `Socks5Stream::use_stream`) - docs/architecture/decisions/003-udp-handle-swap.md (why `Socks5Datagram` is cut, since the associate session is first-party) - docs/research/poc-connect-wasm-findings.md §Wire/API notes (`Socks5Stream::request` target construction; the `AuthenticationRejected` unit-variant note) - Upstream: `/workspace/fast-socks5/src/client.rs` - POC #4 gate map: docs/research/poc-connect-wasm-findings.md (client.rs row) ## Notes - **Verbatim-code lint posture**: no module-level clippy allows were needed for this file (unlike `server.rs`'s `wrong_self_convention`): upstream's client region compiles clean under the newer clippy after the mechanical adjustments. The posture note stays in the module doc for the re-vendor diff. - **Clippy `unused_io_amount` fix (the one non-verbatim line)**: upstream's `request_header` writes with `self.socket.write(&packet [..padding])` (upstream `client.rs:331`); in-tree it is `write_all`; `deny(clippy::unused_io_amount)` is on by default and a partial write of a SOCKS5 request header would corrupt the conversation anyway, so `write_all` is the faithful translation (byte-identical on the wire for a healthy stream; strictly better on a partial-write stream). The comment above the write (why the packet is length-limited) is upstream's, preserved. - **`anyhow::Error::msg("target addr should be present").into()`** (upstream `client.rs:285`) became the typed `SocksError::MissingTargetAddr` variant, added to `SocksError` in `mod.rs` (the wire-vocabulary task's error enum, extended by one variant; the message text is upstream's, kept as the variant's `#[error]` string). This is the task's "typed variants where upstream used `anyhow::Error::msg(...).into()`" adjustment; the variant is wire-inert (error surface only). - **`use_password_auth`'s `unreachable!()`** (upstream `client.rs:207`): upstream's match has three arms, `Some(Password{..})` → Ok, `Some(None)` → `unreachable!()`, `None` → `AuthenticationRejected("missing user pass")`. In-tree the `Some(None)` and `None` arms collapse to a single `AuthenticationRejected("Authentication rejected, missing user pass")` return (the same message as upstream's `None` arm, an error return instead of a panic, per convention 2). The state is reachable only by misuse (`auth = Some(AuthenticationMethod::None)` while the server picks password). - **`Config` pruned to `skip_auth`**: `connect_timeout` + its setter are cut with `connect_raw` (their only consumer). With one field left, the struct stayed (fidelity: the field is load-bearing in `use_stream`'s handshake branch); `Default` was derived instead of the manual impl (upstream's manual `Default` only existed to spell the now-cut field). `set_skip_auth` is upstream verbatim. - **Dropped `log` calls leave bindings unused**: the `debug!` drops in `read_request_reply`/`use_password_auth` leave `rsv`/`version` unconsumed → renamed `_rsv`/`_version` (same mechanical adjustment as the server-typestate task). `which_method_accepted`'s version binding is still consumed (the mismatch check). - **`use_password_auth` gains a version check**: upstream read the auth-sub-negotiation reply and compared only `is_success`; the destructured `version` fed only a dropped `debug!`. In-tree the dropped-log rename would have discarded it entirely; instead it is checked against `SOCKS5_VERSION` (→ `UnsupportedSocksVersion`), the same check every other reply read in this file performs, since the alternative (an `_version` binding) silently accepts a v4 reply on the sub-negotiation. Byte-level behavior on a compliant server is unchanged. - **`request`'s dropped `debug!`**: the `debug!("Requesting headers ...")` consumed `&self.target_addr` only via formatting; nothing renamed (the field is written on the previous line, read in `request_header`). - **Tests are first-party** (upstream's client test module rides the cut net region, testing `connect` against real listeners): the test module here is written fresh against the crate's test layout: raw-byte mock servers over tokio duplex streams, pinned byte sequences written directly (uncorrelated from the vendored server typestate, per the task). Coverage: no-auth handshake + CONNECT success (greeting `[5,1,0]`, method-choice `[5,0]`, domain request `5 1 0 3 len… port`, reply → bind addr, and post-reply pass-through both directions), password accept (greeting `[5,2,0,2]`, the 14-byte user/pass packet, status `[1,0]`, IP-target request bytes), password reject (status `[1,0xff]` → `AuthenticationRejected`), unacceptable method (`[5,0xff]` reply written by the client, then `AuthMethodUnacceptable`), version mismatch (both greeting directions), missing target addr (typed `MissingTargetAddr`), UDP ASSOCIATE zero fallback (`[5,3,0,1,0,0,0,0,0,0]`), `skip_auth` (greeting bypassed, request being the first bytes), the full mapped failure-code table (0x01–0x08 → `SocksError::ReplyError` with the code round-tripped), domain >255 (→ `ExceededMaxDomainLen(256)`), `tokio::io::copy` through the session (the `AsyncRead`/`AsyncWrite` impls), and the socket accessors (`get_socket_ref`/`get_socket_mut`/`get_socket`). - **`request_header` is exercised directly** in two tests (the missing-target and the zero-fallback one); since it is private, the tests live in the same module, so this is in-crate visibility, no surface change. ## Summary Extracted the client session from fast-socks5 v1.0.0 into `src/vendor/client.rs` per ADR-011 §Amendment + vendor.md (cut table row 2): `Socks5Stream` with the `use_stream` path (`send_version_ and_methods`, `which_method_accepted`, `use_password_auth`, `request_header`, `read_request_reply`, `get_socket`/ `get_socket_ref`/`get_socket_mut`), the `AsyncRead`/`AsyncWrite` impls, and the pruned `Config` (`skip_auth` only). The cut regions (`Socks5Datagram` struct + impl, the `Socks5Stream` convenience impl with `connect`/`connect_with_password`/ `connect_raw`, `Config::connect_timeout` + setter) were not copied (verified by sweep; only doc-comment provenance mentions remain). Extraction-time adjustments applied: `anyhow` dropped (the `client.rs:285` site became the typed `SocksError::MissingTargetAddr` variant (one variant added to the vocabulary task's `SocksError`; the `client.rs:528` `Other(anyhow!(...))` site went with the cut `Socks5Datagram`), `log` calls dropped (unused bindings renamed), `use_password_auth`'s `unreachable!()` replaced with an error return (upstream's `None`-arm message), the `"[::]:0".parse().unwrap()` sites verified gone with the cut region, and one clippy-driven correction (`write` → `write_all` in `request_header`). 14 first-party tests pin the wire behavior against raw-byte mock servers over tokio duplex streams (no sockets, no dependency on the vendored server typestate): both handshakes (accept + reject paths), the 0xff method refusal, version mismatch, CONNECT success with the domain-target and IP-target request forms byte-pinned, the full mapped failure-code table (0x01–0x08), the UDP-ASSOCIATE zero fallback, `skip_auth`, domain-length rejection, the `AsyncRead`/`AsyncWrite` impls via `tokio::io::copy`, and the socket accessors. Verified: `cargo test` (37 pass), `--all-features`, clippy `--all-targets -- -D warnings` native + wasm32, `cargo check --target wasm32-unknown-unknown`, `cargo fmt --check`, `cargo doc --no-deps`, and the negative sweeps.