tasks: decompose the vendor group — six dependency-ordered tasks
The inline vendor (vendor.md, ADR-011 §Amendment) is the root of the crate's dependency graph; this decomposes it before any downstream group (params/wire/producer/consumer/doors). - vendor-crate-scaffolding: Cargo.toml (wasm-clean tokio subset, no alkcall yet), lib.rs, vendor/ skeleton, bundled MIT notice - vendor-wire-vocabulary: consts, codecs, errors, macros — with the extraction-boundary pins (error-variant pruning, pub command codecs, macro de-exporting) - vendor-server-typestate: Socks5ServerProtocol + the new_reply publication (carried change #2); BIND's driving is producer-group work, the vendor carries only the surface - vendor-client-session: Socks5Stream::use_stream path + the AsyncRead/AsyncWrite impls; Config pruned (connect_timeout cut); tests against a raw-byte mock server so it runs parallel with the typestate task - vendor-differential-tests: RFC edge-case suite over duplex streams, pinning upstream behavior per vendor.md's differential posture - review-vendor-extraction: the gate before downstream groups — file-by-file diff vs /workspace/fast-socks5 must contain only cut/carried-change/adjustment hunks Graph validated: all dependency ids resolve, no cycles; topological order scaffolding → wire → (typestate ∥ client) → tests → review.
This commit is contained in:
1 parent
b0cfc26065
commit
2cc010cf2b
6 files changed
+705
No files matched your search
@@ -0,0 +1,85 @@
|
||||
---
|
||||
id: review-vendor-extraction
|
||||
name: Review — the vendor extraction before dependent layers
|
||||
status: pending
|
||||
depends_on: [implementation/vendor-differential-tests]
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: project
|
||||
level: review
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Review checkpoint before any layer builds on the vendor. Everything
|
||||
else in the crate (params, wire, producer, consumer, doors, backends)
|
||||
consumes the vendored typestate/codecs, so extraction errors — a
|
||||
behavior drift vs upstream, a missed cut, a wasm regression — would
|
||||
propagate into every downstream task. This review is the gate between
|
||||
the vendor group and the rest of the decomposition.
|
||||
|
||||
Review against the architecture, in this order:
|
||||
|
||||
1. **Extraction completeness vs the cut table** (vendor.md): every
|
||||
"What is Extracted" row is present; every "What is NOT Extracted"
|
||||
row is absent. Diff `src/vendor/` against
|
||||
`/workspace/fast-socks5/src/` file-by-file — the diff must contain
|
||||
exactly: the extraction cut (deletions per the cut table), the two
|
||||
carried changes (`new_reply` publication; the BIND surface being
|
||||
present-and-public), the extraction-time adjustments (anyhow/log
|
||||
removal, error-variant pruning, macro de-exporting, the
|
||||
no-unwrap translations), and the provenance/license notes. Any diff
|
||||
hunk outside those categories is a finding.
|
||||
2. **Wasm-clean invariant** (ADR-009 §Amendment, ADR-011 §Amendment):
|
||||
`cargo check --target wasm32-unknown-unknown` and native
|
||||
`--no-default-features`-equivalent cleanliness; no `net`-shaped
|
||||
feature exists; `tokio` stays on the wasm-clean subset in
|
||||
`[dependencies]`.
|
||||
3. **Convention compliance** (AGENTS.md): no `unwrap`/`expect`/panics
|
||||
outside tests; `thiserror` error surface; no comments in code
|
||||
except doc comments and the pinned non-obvious-correctness notes;
|
||||
naming; module structure (one module per file, `lib.rs`
|
||||
re-exports).
|
||||
4. **RFC fidelity spot-check**: run the differential test suite; for
|
||||
two or three of the trickiest pinned expectations (e.g. the
|
||||
reply-form byte counts, the method-negotiation preference order),
|
||||
independently verify the pinned bytes against the reference
|
||||
checkout and RFC 1928.
|
||||
5. **License/provenance** (ADR-011): the vendor point (v1.0.0) noted
|
||||
in-tree; the MIT notice present and referenced.
|
||||
6. **Docs sync**: `vendor.md`'s extraction table and cut table match
|
||||
what landed (if the implementer discovered a boundary correction —
|
||||
e.g. a transitive dependency the cut table missed — the doc was
|
||||
updated in the same change, not silently diverged).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] The file-by-file diff vs upstream contains only
|
||||
cut-table/carried-change/adjustment/provenance changes — no
|
||||
unexplained divergence
|
||||
- [ ] Both wasm checks pass (`cargo check` and
|
||||
`cargo clippy --target wasm32-unknown-unknown -- -D warnings`)
|
||||
- [ ] Full verification suite green: `cargo test`,
|
||||
`cargo clippy --all-targets -- -D warnings`, `cargo fmt --check`
|
||||
- [ ] No convention violations (unwrap/panics/error surface/naming)
|
||||
- [ ] License + provenance notes verified present
|
||||
- [ ] `vendor.md` matches the implemented tree (or the mismatch is
|
||||
filed as a blocker before proceeding)
|
||||
- [ ] Findings recorded; critical findings block the downstream task
|
||||
groups until resolved
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/vendor.md (the spec under review)
|
||||
- docs/architecture/decisions/011-inline-vendor.md
|
||||
- docs/architecture/decisions/009-feature-gates.md
|
||||
- AGENTS.md (conventions 1–17; the verification commands)
|
||||
- /workspace/fast-socks5 (the reference checkout for the diff)
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
@@ -0,0 +1,123 @@
|
||||
---
|
||||
id: implementation/vendor-client-session
|
||||
name: Vendor — the client session (Socks5Stream::use_stream path)
|
||||
status: pending
|
||||
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<S>::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<S>` 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<S>` (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 — 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<TcpStream>` 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 — verify none remain.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `src/vendor/client.rs` exists with the inventory above;
|
||||
`cargo test` compiles
|
||||
- [ ] `cargo check --target wasm32-unknown-unknown` clean
|
||||
- [ ] No `Socks5Datagram`, no `Socks5Stream<TcpStream>` convenience
|
||||
impl (`connect`/`connect_with_password`/`connect_raw`) anywhere
|
||||
under `src/vendor/`
|
||||
- [ ] No `anyhow`, `log`, `tokio::net`, `UdpSocket` anywhere under
|
||||
`src/vendor/`
|
||||
- [ ] No `unwrap`/`expect`/`unreachable!`/panic paths in the extracted
|
||||
client code (convention 2)
|
||||
- [ ] `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 — keeping this
|
||||
task parallel with the typestate extraction and the client's
|
||||
wire expectations uncorrelated from the server implementation;
|
||||
tokio duplex streams, no sockets)
|
||||
- [ ] `Socks5Stream<S>` implements `AsyncRead` + `AsyncWrite` (the
|
||||
impls extracted) and the impls are exercised by at least one
|
||||
test (e.g. `tokio::io::copy` through the session)
|
||||
- [ ] `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<S>::use_stream`)
|
||||
- docs/architecture/decisions/003-udp-handle-swap.md (why
|
||||
`Socks5Datagram` is cut — 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
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
@@ -0,0 +1,93 @@
|
||||
---
|
||||
id: implementation/vendor-crate-scaffolding
|
||||
name: Vendor — crate scaffolding (Cargo.toml, lib.rs, vendor module skeleton, license notice)
|
||||
status: pending
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: project
|
||||
level: implementation
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Create the `alksocks` crate skeleton so the vendor extraction tasks have
|
||||
their targets. This is the first implementation work in the repo — `src/`
|
||||
does not exist yet. The crate is born wasm-clean: the default build is
|
||||
protocol-only with zero kernel-net code (ADR-009 §Amendment, ADR-011
|
||||
§Amendment), so the wasm check must be green from the first commit.
|
||||
|
||||
Deliverables:
|
||||
|
||||
1. **`Cargo.toml`** mirroring the sibling crates' posture
|
||||
(`/workspace/@alkdev/alktunnels`, `/workspace/@alkdev/alktty`):
|
||||
- `name = "alksocks"`, `version = "0.1.0"`, `edition = "2021"`,
|
||||
`rust-version = "1.88"`, `license = "MIT OR Apache-2.0"`,
|
||||
description/keywords/categories/repository per the siblings.
|
||||
- `[features]`: `default = []`, `local = []`, `composed = []` —
|
||||
declared empty now (ADR-009 pins the two-feature layout; the
|
||||
features are populated by their modules' later tasks; `composed`
|
||||
will add the alktunnels dep, `local` will add `tokio/net` +
|
||||
`socket2`).
|
||||
- `[dependencies]`: `tokio = { version = "1", default-features = false,
|
||||
features = ["rt", "sync", "io-util", "macros", "time"] }` (the
|
||||
wasm-clean subset per AGENTS.md convention 4 — **never
|
||||
`features = ["full"]`**), `thiserror = "2"`, `tracing = "0.1"`.
|
||||
**No `alkcall` yet** — the vendor does not need it; the
|
||||
producer/consumer tasks pin `alkcall = "0.8.0"` when they land.
|
||||
- `[dev-dependencies]`: `tokio = { version = "1", features = ["full",
|
||||
"test-util", "macros"] }` (tests are native-only; they are not
|
||||
part of the wasm-clean surface).
|
||||
2. **`src/lib.rs`**: `#![forbid(unsafe_code)]`, brief crate docs
|
||||
(what the crate is, pointing at `docs/architecture/overview.md` —
|
||||
doc comments are fine and expected), `pub mod vendor;`.
|
||||
3. **`src/vendor/` skeleton** preserving upstream file names so per-file
|
||||
diffs against the reference checkout stay mechanical (the
|
||||
maintenance posture, vendor.md §Maintenance Posture):
|
||||
- `src/vendor/mod.rs` — module doc carrying the provenance note:
|
||||
extracted from fast-socks5 **v1.0.0** (MIT, third-party upstream),
|
||||
per ADR-011; the extraction cut + carried changes are recorded in
|
||||
`docs/architecture/vendor.md`; submodule declarations for `server`,
|
||||
`client`, `util` (stub files, filled by the extraction tasks).
|
||||
- `src/vendor/server.rs`, `src/vendor/client.rs`,
|
||||
`src/vendor/util/mod.rs`, `src/vendor/util/stream.rs`,
|
||||
`src/vendor/util/target_addr.rs` — empty stubs.
|
||||
4. **Bundled license notice** at `src/vendor/LICENSE`: the upstream MIT
|
||||
license text plus attribution (fast-socks5 v1.0.0,
|
||||
github.com/dizda/fast-socks5), referenced from the vendor module doc
|
||||
(ADR-011's license obligation — upstream has no per-file headers, so
|
||||
the bundled notice is the obligation's carrier).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `cargo test` passes (empty crate compiles and runs)
|
||||
- [ ] `cargo clippy --all-targets -- -D warnings` clean
|
||||
- [ ] `cargo fmt --check` clean
|
||||
- [ ] `cargo check --target wasm32-unknown-unknown` clean (the wasm gate
|
||||
is green from birth — no `mio`/`socket2`/`tokio::net` in the graph)
|
||||
- [ ] `cargo clippy --target wasm32-unknown-unknown -- -D warnings` clean
|
||||
- [ ] `Cargo.toml` has no `alkcall`, no `anyhow`, no `log`, no
|
||||
`async-trait`, no `tokio-stream`, no `socket2` in `[dependencies]`
|
||||
- [ ] `src/vendor/LICENSE` exists with the MIT text + v1.0.0 attribution,
|
||||
referenced from the vendor module doc
|
||||
- [ ] `#![forbid(unsafe_code)]` present in `lib.rs`
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/vendor.md (the vendor spec)
|
||||
- docs/architecture/decisions/011-inline-vendor.md (§Amendment — the
|
||||
extraction posture, license obligation, wasm gate)
|
||||
- docs/architecture/decisions/009-feature-gates.md (the feature layout)
|
||||
- docs/architecture/overview.md §Dependencies (the tokio subset)
|
||||
- Sibling Cargo.toml posture: `/workspace/@alkdev/alktunnels/Cargo.toml`,
|
||||
`/workspace/@alkdev/alktty/Cargo.toml`
|
||||
- Upstream license: `/workspace/fast-socks5/Cargo.toml` (v1.0.0, MIT) +
|
||||
`/workspace/fast-socks5/LICENSE`
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
@@ -0,0 +1,115 @@
|
||||
---
|
||||
id: implementation/vendor-differential-tests
|
||||
name: Vendor — the differential RFC edge-case test suite
|
||||
status: pending
|
||||
depends_on: [implementation/vendor-server-typestate, implementation/vendor-client-session]
|
||||
scope: moderate
|
||||
risk: medium
|
||||
impact: component
|
||||
level: implementation
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Build the crate's RFC edge-case test suite for the extracted vendor —
|
||||
the differential-testing posture vendor.md pins: "the crate's test
|
||||
suite covers the edge cases the extracted code must keep passing; when
|
||||
in doubt, diff behavior against the reference checkout." The per-task
|
||||
unit tests (wire vocabulary, typestate, client session) cover the
|
||||
happy paths; this task covers the RFC edge cases and the
|
||||
extraction-fidelity checks, using `/workspace/fast-socks5` as the
|
||||
behavioral reference (read-only — tests never depend on it at build
|
||||
time; the reference is consulted while writing the tests, and the
|
||||
assertions pin the expected bytes).
|
||||
|
||||
Test infrastructure: tokio duplex streams (`tokio::io::duplex`) —
|
||||
no sockets, no `local` feature, runs in the default wasm-clean crate's
|
||||
test suite (tests are native-only via dev-deps; the crate itself stays
|
||||
wasm-clean). A first-party test harness drives the vendored typestate
|
||||
server against the vendored client session over duplex pairs.
|
||||
|
||||
**Edge-case coverage** (each pinned with explicit expected bytes where
|
||||
the wire form is the point):
|
||||
|
||||
1. **Negotiation edge cases**: method-list parsing (empty method list,
|
||||
zero methods offered → 0xff), method-order preference (server picks
|
||||
its first match, not the client's first offer), version mismatch
|
||||
(0x04 greeting → refusal), truncated greeting (EOF mid-handshake →
|
||||
typed error, no hang), password-auth sub-protocol (empty username →
|
||||
`EmptyUsername`, empty password → `EmptyPassword`, reject path →
|
||||
0x01/0xff reply then error).
|
||||
2. **Command-read edge cases**: unknown command byte (0x04+ →
|
||||
`UnknownCommand` → 0x07 reply), reserved-byte tolerance (RSV ≠ 0 —
|
||||
upstream reads it without checking; pin the upstream behavior),
|
||||
address-type edge cases (ATYP 0x02/0x05+ → `IncorrectAddressType`
|
||||
→ 0x08 reply), domain-length boundary (255-byte domain OK,
|
||||
256+ rejected at encode time by `to_be_bytes`), truncated address
|
||||
fields (EOF mid-address → typed error).
|
||||
3. **Reply-form edge cases** (the `new_reply` carried change's
|
||||
regression guard): v4 reply = 10 bytes, v6 reply = 22 bytes,
|
||||
VER=0x05/RSV=0x00 in every reply, error replies carry
|
||||
`0.0.0.0:0`-equivalent (ATYP=1, zero addr+port), success replies
|
||||
carry the caller's `sock_addr` verbatim.
|
||||
4. **Client-session edge cases**: reply#1 with a non-zero reply code →
|
||||
the mapped `ReplyError` (the `from_u8` table), domain-target
|
||||
request encoding (ATYP=3, len-prefixed), the UDP-ASSOCIATE
|
||||
zero-target fallback (upstream `request_header`'s
|
||||
`target_addr: None` branch — pin the 10-byte zero form),
|
||||
`skip_auth` behavior (no method negotiation bytes on the wire).
|
||||
5. **UDP codec edge cases**: `parse_udp_request` with garbage RSV →
|
||||
`GarbageInReserved`, FRAG ≠ 0 passthrough (the parser returns frag;
|
||||
the *caller* refuses — pin the parser's contract),
|
||||
`new_udp_header`/`parse_udp_request` round-trip for v4/v6/domain.
|
||||
6. **`transfer`/`wait_on_tcp` semantics**: `transfer` pumps both
|
||||
directions to completion over duplexes (including half-close
|
||||
propagation); `wait_on_tcp` returns `EOF` on clean close,
|
||||
`UnexpectedUdpControlGarbage` on a control byte.
|
||||
|
||||
**Differential method**: for each behavior above, consult the
|
||||
reference checkout's behavior (its own tests + code reading) and pin
|
||||
the *same* expectation in-tree, noting the upstream source line in the
|
||||
test's doc comment where the expectation is non-obvious. Where the
|
||||
extraction deliberately diverges (the pruned error variants, the
|
||||
published `new_reply`), the test pins the *in-tree* contract and the
|
||||
divergence is already recorded in vendor.md — no new divergence may be
|
||||
introduced by this task.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] The six coverage groups above each have at least one test;
|
||||
all pass in `cargo test`
|
||||
- [ ] Tests use only duplex/in-memory streams — no `TcpListener`/
|
||||
`UdpSocket`/`tokio::net` in test code (the suite runs in the
|
||||
default-feature build)
|
||||
- [ ] Every non-obvious expectation carries a doc-comment note naming
|
||||
the upstream behavior it pins (file + line in
|
||||
`/workspace/fast-socks5`)
|
||||
- [ ] `cargo test` (default features) green;
|
||||
`cargo test --all-features` green (no feature-gated vendor code
|
||||
exists, so both must pass identically)
|
||||
- [ ] `cargo check --target wasm32-unknown-unknown` still clean
|
||||
(tests are dev-only; the crate surface unchanged)
|
||||
- [ ] `cargo clippy --all-targets -- -D warnings` and
|
||||
`cargo fmt --check` clean
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/vendor.md §Maintenance Posture (the
|
||||
differential-testing contract)
|
||||
- docs/architecture/decisions/011-inline-vendor.md (differential
|
||||
testing guards the RFC edge cases)
|
||||
- docs/architecture/decisions/002-protocol-base.md (faithful
|
||||
reply-code mapping)
|
||||
- Reference checkout: `/workspace/fast-socks5` (v1.0.0 — behavior
|
||||
source for the pinned expectations; its `src/lib.rs` test module and
|
||||
`src/server.rs` test module are the upstream coverage to mirror
|
||||
where extractable)
|
||||
- RFC 1928 (the wire form's authority)
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
@@ -0,0 +1,155 @@
|
||||
---
|
||||
id: implementation/vendor-server-typestate
|
||||
name: Vendor — the server typestate (Socks5ServerProtocol + carried changes)
|
||||
status: pending
|
||||
depends_on: [implementation/vendor-wire-vocabulary]
|
||||
scope: narrow
|
||||
risk: medium
|
||||
impact: component
|
||||
level: implementation
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Extract the server typestate — fast-socks5's explicit API, the
|
||||
producer's conversation driver (ADR-002) — into
|
||||
`src/vendor/server.rs`, and apply the two carried changes that touch
|
||||
this file (BIND's reply surface + the `new_reply` publication). This
|
||||
is the highest-fidelity-sensitive extraction: the typestate is the
|
||||
RFC conversation's state machine, and the producer layer (a later
|
||||
task group) drives it directly over a channels `BiStream`.
|
||||
|
||||
**What is extracted** (upstream `src/server.rs` → `src/vendor/server.rs`):
|
||||
|
||||
1. `SocksServerError` enum + `to_reply_error` verbatim (the
|
||||
server-side wire error vocabulary; keep `UnexpectedUdpControlGarbage`
|
||||
— the ASSOCIATE quiet-wait semantics consume it).
|
||||
2. `ErrorContext<T>` trait + its two impls (`err_when`) verbatim —
|
||||
the typestate's internal error-context idiom, used pervasively in
|
||||
the extracted region.
|
||||
3. The typestate core, verbatim:
|
||||
- `states` module (`Opened`/`Authenticated`/`CommandRead`),
|
||||
`Socks5ServerProtocol<T, S>` + `new`/`start`.
|
||||
- `CheckResult` trait + impls.
|
||||
- `accept_no_auth` / `accept_password_auth` / `finish_auth` /
|
||||
`skip_auth_this_is_not_rfc_compliant`.
|
||||
- `AuthMethodSuccessState<T>`, `AuthMethod<T>` traits.
|
||||
- `NoAuthentication`/`NoAuthenticationImpl`,
|
||||
`PasswordAuthentication`/`PasswordAuthenticationImpl` (+ the
|
||||
`password_states` module, `PasswordAuthenticationStarted` alias).
|
||||
- `auth_method_enums!` macro + `StandardAuthentication` (keep the
|
||||
macro `#[macro_export]`-free per the same adjustment as
|
||||
`read_exact!` — `pub(crate) use` scoping; upstream exports it at
|
||||
the crate root).
|
||||
- `negotiate_auth`, `read_command`, `reply_success`, `reply_error`.
|
||||
- `transfer` + `wait_on_tcp` (the typestate's own two-pump seam —
|
||||
vendor.md's extraction table; `transfer` is the interception-point
|
||||
shape, `wait_on_tcp` is the ASSOCIATE control-stream reader).
|
||||
- `try_notify!` macro (internal to the extracted region; same
|
||||
scoping adjustment).
|
||||
4. **Carried change #2 — `new_reply` publication** (vendor.md's
|
||||
carried changes; POC #5 ledger ask #3): the reply encoder becomes
|
||||
public as a `reply(code, addr)`-shaped constructor. Upstream:
|
||||
`fn new_reply(error: &ReplyError, sock_addr: SocketAddr) -> Vec<u8>`
|
||||
(private, `server.rs:1380`). In-tree: `pub fn new_reply(...)`
|
||||
(or `pub fn reply(code: &ReplyError, addr: SocketAddr) -> Vec<u8>`
|
||||
— either name; pin one and use it consistently) so the BIND
|
||||
reply#2 writer and the consumer's reply readers don't re-encode
|
||||
the 10/22-byte reply form. The vendored `reply_success`/`reply_error`
|
||||
call it; nothing else changes.
|
||||
|
||||
**What is NOT extracted** (the cut — POC #4's gate map + the recipe's
|
||||
two additions; vendor.md's cut table row 1):
|
||||
|
||||
- `run_tcp_proxy`, `run_udp_proxy`/`run_udp_proxy_custom`,
|
||||
`udp_bind_random_port`, `handle_udp_*`/`transfer_udp` (all
|
||||
kernel-net; the producer drives the typestate directly — POC #2's
|
||||
shape — and the local backends are first-party tokio code, ADR-008).
|
||||
- The legacy `Socks5Server`/`Incoming`/`Socks5Socket` (struct + impls,
|
||||
including the `AsyncRead`/`AsyncWrite` impls and the `Unpin` impl —
|
||||
they exist only for the legacy API).
|
||||
- `DnsResolveHelper` trait + impl (net-gated upstream; the wrapper
|
||||
resolves nothing — domain targets pass through unresolved, ADR-012).
|
||||
- The legacy `Config<A>` + `Authentication` trait + impls
|
||||
(`SimpleUserPassword`/`DenyAuthentication`/`AcceptAuthentication`,
|
||||
`authenticate_callback`) — the recipe's addition: ungated upstream
|
||||
but consumed only by the cut legacy region. With it go the
|
||||
`#[allow(deprecated)]` attributes on the cut impls.
|
||||
- The net-only test module (`test_bind` — binds a real listener).
|
||||
|
||||
**Extraction-time adjustments** (mechanical):
|
||||
|
||||
- Drop `anyhow::Context` imports and `log` calls (`trace!`/`debug!`/
|
||||
`error!`/`warn!` in the extracted region — drop or re-point at
|
||||
`tracing`; the typestate's traces are per-field debug logs, drop
|
||||
them; keep behavior identical).
|
||||
- `reply_error`'s `"0.0.0.0:0".parse().unwrap()` (upstream
|
||||
`server.rs:945`): replace the `unwrap` with a `const` parsed at
|
||||
compile time or a documented-infallible construction (AGENTS.md
|
||||
convention 2 — no `unwrap` outside tests; the parse is statically
|
||||
infallible, so a `const`/`SocketAddr::new` form is the faithful
|
||||
translation).
|
||||
- `Socks5Command::from_u8` returning `Option` + `ok_or` stays (the
|
||||
unknown-command path maps to `SocksServerError::UnknownCommand` →
|
||||
`CommandNotSupported`, RFC-faithful).
|
||||
- Keep `#[allow(async_fn_in_trait)]` only where upstream had it and
|
||||
the extracted region still needs it (the `DnsResolveHelper` trait is
|
||||
cut; check whether any remaining trait needs the attribute —
|
||||
`AuthMethod` is not async, so likely none).
|
||||
|
||||
**The BIND note** (why carried change #1 does not appear here): BIND's
|
||||
driving is wrapper-side code (POC #5 — "zero upstream ask for BIND
|
||||
itself"): the typestate's `read_command()` already returns
|
||||
`TCPBind`, `reply_success(sock_addr)` is reply#1's carrier, and the
|
||||
reply#2 writer uses the published `new_reply`. The vendor carries the
|
||||
*shape* (the published encoder + the typestate's existing surface);
|
||||
the BIND phase-model driving itself is the producer task group's work
|
||||
(ADR-007), not the vendor's. This task's obligation is only that the
|
||||
surface BIND needs is present and public.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `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`/
|
||||
`handle_udp_*`/`transfer_udp`/`Socks5Server`/`Incoming`/
|
||||
`Socks5Socket`/`DnsResolveHelper`/`Config`/`Authentication`/
|
||||
`authenticate_callback` anywhere under `src/vendor/`
|
||||
- [ ] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`,
|
||||
`async-trait` anywhere under `src/vendor/`
|
||||
- [ ] `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
|
||||
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
|
||||
`cargo fmt --check` clean
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/vendor.md (the extraction table row 1, the cut
|
||||
table row 1, the carried changes)
|
||||
- docs/architecture/decisions/011-inline-vendor.md §Amendment
|
||||
- docs/architecture/decisions/002-protocol-base.md (the explicit
|
||||
typestate API only)
|
||||
- docs/architecture/decisions/007-bind-in-base.md (the BIND surface
|
||||
the vendor must expose)
|
||||
- docs/research/poc-bind-findings.md §Wire/API notes (`reply_success`
|
||||
as reply#1 carrier; the `new_reply` finding)
|
||||
- Upstream: `/workspace/fast-socks5/src/server.rs`
|
||||
- POC #4 gate map: docs/research/poc-connect-wasm-findings.md
|
||||
(server.rs row)
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
@@ -0,0 +1,134 @@
|
||||
---
|
||||
id: implementation/vendor-wire-vocabulary
|
||||
name: Vendor — the wire vocabulary (consts, commands, codecs, errors, macros)
|
||||
status: pending
|
||||
depends_on: [implementation/vendor-crate-scaffolding]
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: component
|
||||
level: implementation
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Extract the pure wire vocabulary from fast-socks5 v1.0.0 into
|
||||
`src/vendor/` — the RFC 1928 codecs and error enums that everything
|
||||
else (typestate, client sessions, producer/consumer layers) consumes.
|
||||
This is the "codecs" row of vendor.md's extraction table plus the
|
||||
`lib.rs` non-net region. Copy from `/workspace/fast-socks5` (read-only
|
||||
reference checkout) and apply the extraction-time adjustments; the
|
||||
result must be wasm-clean by construction (no `tokio::net`, no
|
||||
`anyhow`, no `log`).
|
||||
|
||||
**What is extracted** (upstream file → in-tree target):
|
||||
|
||||
1. **`src/lib.rs` → `src/vendor/lib.rs`** (new file — upstream's
|
||||
`lib.rs` content minus the cut regions):
|
||||
- `consts` module verbatim (all `SOCKS5_*` constants).
|
||||
- `Socks5Command` enum + `as_u8`/`from_u8` (make both `pub` — the
|
||||
producer's command dispatch and the consumer's request writer both
|
||||
need them; upstream keeps them private with `#[allow(dead_code)]`).
|
||||
- `AuthenticationMethod` enum + impls (consumed by the client
|
||||
session's `use_stream` path; keep as-is).
|
||||
- `ReplyError` enum + `as_u8`/`from_u8` verbatim.
|
||||
- `UdpHeaderError` enum, `new_udp_header`, `parse_udp_request`
|
||||
verbatim (the ASSOCIATE datagram codec — vendor.md's codecs row).
|
||||
- `Result<T, E = SocksError>` type alias.
|
||||
- **NOT extracted**: the net-heavy test module (upstream `lib.rs`
|
||||
lines 345–520 — it binds real sockets; the crate's own test task
|
||||
covers these edge cases), the `socks4` module declaration.
|
||||
2. **`src/util/target_addr.rs` → `src/vendor/util/target_addr.rs`**:
|
||||
- `AddrError`, `TargetAddr` (+ `is_ip`/`is_domain`/`to_be_bytes`/
|
||||
`into_string_and_port`), `ToSocketAddrs` impl, `Display` impl,
|
||||
`ToTargetAddr` trait + all impls, `Addr` enum, `read_address` —
|
||||
verbatim.
|
||||
- **NOT extracted**: the `resolve_dns` method (POC #4's gate map —
|
||||
it is the one net-touching member; `tokio::net::lookup_host` is
|
||||
not in the wasm-clean graph). Delete the method; `AddrError::
|
||||
DNSResolutionFailed`/`NoDNSRecords` go with it (see adjustments).
|
||||
- Drop the `debug!` log calls (upstream `log` → dropped or re-pointed
|
||||
at `tracing`; these are per-octet debug traces — drop them).
|
||||
3. **`src/util/stream.rs` → `src/vendor/util/stream.rs`**:
|
||||
- `read_exact!` and `ready!` macros verbatim. **Adjustment**: they
|
||||
are `#[macro_export]` upstream (crate-root exported, which would
|
||||
pollute the alksocks public API); convert to plain `macro_rules!`
|
||||
with `pub(crate) use` scoping inside the vendor module so they
|
||||
stay internal (the wrapper's own code gets its own macros; the
|
||||
vendor's are implementation detail).
|
||||
- `ConnectError` enum + `to_reply_error` verbatim (the reply-code
|
||||
mapping vocabulary — consumed by the dial seam's error mapping).
|
||||
- **NOT extracted**: `tcp_connect`/`tcp_connect_with_timeout` (the
|
||||
`tokio::net::TcpStream` dialers — POC #4's gate map; the crate's
|
||||
local backend is first-party tokio code, ADR-008).
|
||||
4. **`SocksError` (in `src/vendor/lib.rs`)** — the error vocabulary,
|
||||
pruned to what the extracted region + the crate's error surface
|
||||
actually consumes (vendor.md: "the vendored `SocksError`/
|
||||
`SocksServerError` stay the wire-level vocabulary"):
|
||||
- Keep: `Io`, `AuthMethodUnacceptable`, `UnsupportedSocksVersion`,
|
||||
`ExceededMaxDomainLen`, `AuthenticationRejected(String)`,
|
||||
`ServerError(#[from] SocksServerError)`,
|
||||
`UdpHeaderError(#[from] UdpHeaderError)`,
|
||||
`AddrError(#[from] AddrError)`, `ReplyError(#[from] ReplyError)`.
|
||||
- Drop: `Redaction`, `InvalidHeader` (dead upstream — no consumer in
|
||||
the extracted region), `ConnectError(#[from] ConnectError)`
|
||||
(its only producer was the cut `tcp_connect*`; `ConnectError`
|
||||
itself stays — the dial seam maps it), `ReplySocks4Error` (socks4
|
||||
is cut), `ArgumentInputError` (no consumer), `Other(#[from]
|
||||
anyhow::Error)` (anyhow is dropped — vendor.md's adjustments).
|
||||
- Note: `AuthenticationRejected(String)` on `SocksError` is the
|
||||
client-side variant (client.rs uses it with a message);
|
||||
`SocksServerError::AuthenticationRejected` (unit) is the
|
||||
server-side one — both stay, each in its own enum.
|
||||
|
||||
**Extraction-time adjustments** (mechanical, per ADR-011 §Amendment +
|
||||
vendor.md): drop `anyhow` wrapping, drop `log` calls, prune
|
||||
error variants whose only producers/consumers were cut, keep
|
||||
`thiserror` derives. No behavior change: every kept codec's wire
|
||||
behavior is byte-identical to upstream v1.0.0.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `src/vendor/lib.rs`, `src/vendor/util/{mod,stream,target_addr}.rs`
|
||||
exist with the inventory above; `cargo test` compiles
|
||||
- [ ] `cargo check --target wasm32-unknown-unknown` clean
|
||||
- [ ] `cargo check` (native, default features) clean
|
||||
- [ ] No `anyhow`, `log`, `tokio::net`, `socket2`, `tokio-stream`,
|
||||
`async-trait` anywhere under `src/vendor/`
|
||||
- [ ] `Socks5Command::as_u8`/`from_u8` are `pub` (the wrapper needs
|
||||
them; upstream had them private)
|
||||
- [ ] `read_exact!`/`ready!` are not exported at the crate root (no
|
||||
`#[macro_export]`); they resolve inside the vendor module
|
||||
- [ ] `TargetAddr` has no `resolve_dns` method; `AddrError` has no
|
||||
`DNSResolutionFailed`/`NoDNSRecords` variants
|
||||
- [ ] `SocksError` has no `Other(anyhow::Error)`, no `Redaction`, no
|
||||
`InvalidHeader`, no `ReplySocks4Error`, no `ArgumentInputError`
|
||||
variants
|
||||
- [ ] `cargo clippy --all-targets -- -D warnings` and
|
||||
`cargo fmt --check` clean
|
||||
- [ ] Unit tests for the codecs pass: `to_be_bytes`/`read_address`
|
||||
round-trip (v4/v6/domain), `new_udp_header`/`parse_udp_request`
|
||||
round-trip, `ReplyError::as_u8`/`from_u8` mapping table — ported
|
||||
from upstream's non-net test regions where extractable
|
||||
(vendor.md's "non-net test modules" row), adjusted to the
|
||||
crate's test layout (no `env_logger`, no real sockets)
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/vendor.md (the extraction table + the cut table)
|
||||
- docs/architecture/decisions/011-inline-vendor.md §Amendment (the
|
||||
mechanical adjustments)
|
||||
- docs/architecture/decisions/002-protocol-base.md (the codecs are the
|
||||
wire vocabulary; "no second codec, ever")
|
||||
- Upstream: `/workspace/fast-socks5/src/lib.rs`,
|
||||
`/workspace/fast-socks5/src/util/target_addr.rs`,
|
||||
`/workspace/fast-socks5/src/util/stream.rs`
|
||||
- POC #4 gate map: docs/research/poc-connect-wasm-findings.md
|
||||
(lib.rs/util rows)
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
Reference in new issue
Block a user