Repository navigation
routing: PR4 — rule/config/dispatcher correctness pass - #12
Merged
Merged
Conversation
Fourth wave of fixes from the full-project review, covering routing,
rule semantics, config parsing, and ACL conversion correctness.
Dispatcher / Router
* `rule_target_to_action` no longer lower-cases the forwarded outbound
name. Reject keywords (`reject`/`block`/`deny`) still match
case-insensitively; everything else is forwarded verbatim so
outbounds registered as `"Proxy_Out"` actually resolve instead of
silently falling through to `"default"`.
* `Dispatcher::dispatch_udp` now waits for the first packet and routes
by `packet.target` instead of routing against a `0.0.0.0:0`
sentinel. UDP ACLs (`IP-CIDR`, `DOMAIN-SUFFIX`, `DST-PORT`, …) are
no longer no-ops on UDP traffic. A small proxy channel replays the
first packet plus everything that follows to the chosen handler.
Rule parser / matcher (`wind-core/rule.rs`)
* Empty `AND`/`OR` are rejected at parse time. Empty AND silently
degenerates into MATCH; empty OR is vacuous never-match.
* Reversed `DST-PORT`/`SRC-PORT` ranges (lo > hi) are rejected at
parse time. Previously every comparison was false so the rule
matched nothing without warning.
* `IP-SUFFIX`/`SRC-IP-SUFFIX` are formally aliased to `IP-CIDR` —
their old `matches` body was byte-for-byte identical, but the two
variants existed in parallel and silently let either keyword be
used with confusion. True low-bit suffix semantics is a TODO.
* `DOMAIN-SUFFIX` / `DOMAIN-KEYWORD` values are lower-cased at parse
time so the per-match hot path can compare without
allocating. Previously each evaluation built TWO `String`s.
* `DOMAIN-WILDCARD` compiles its regex once at parse time and stores
it on a new `DomainWildcardPattern { pattern, re }` struct; the
matcher is now a single `re.is_match(host)` instead of recompiling
per packet. Failures surface as a parse error.
* `SUB-RULE` keeps all parsed sub-rules (was dropping everything past
the first) and matches them with AND semantics.
* `Display` for `SUB-RULE` round-trips through the parser
(previously emitted a mismatched-paren form with three closing `)`).
`TargetAddr` deserialisation
* Empty hosts (`":80"`) are now rejected with a clear error instead of
parsing to `Domain("", _)` and failing later at DNS time.
* The `host:port` split uses `rsplit_once(':')` and rejects domains
containing whitespace / `[` / `]` / `:` so malformed inputs surface
cleanly.
I/O
* `wind-core::io::copy_io` delegates to `tokio::io::copy_bidirectional`
which correctly handles half-close: when one direction sees EOF it
calls `shutdown()` on the opposite writer and continues pumping the
remaining direction. The hand-rolled select-loop broke out on the
FIRST EOF and dropped in-flight bytes flowing the other way.
`tuic-server` config
* The `__send_window` / `__receive_window` deprecated fields had their
`#[serde(rename)]` annotations swapped, so a legacy top-level
`send_window` deserialised into `__receive_window` and was then
migrated into `quic.receive_window`. QUIC flow-control parameters
were silently transposed on every load of a deprecated layout.
* `infer_config_format` no longer misclassifies YAML files containing
`=` in a VALUE (e.g. a base64 secret `aGVsbG8=`) as TOML. The bare
`... || trimmed.contains('=')` is replaced by a strict
`^[A-Za-z_][\w-]*\s*=` identifier-assignment check, and the rest of
the expression is properly parenthesised.
`wind` config loader
* Unknown extensions on `--config <path>` no longer fall through to
the TOML parser. `.toml`, `.yaml`, `.yml` are recognised; anything
else (including `.json`) is rejected with an explicit error.
`wind-dns` config
* `attempts` got a `#[serde(default)]` so omitting it in a config that
also uses `deny_unknown_fields` (some formats) no longer errors.
ACL pest grammar + `address_to_rule_types`
* The `ipv6` and `ipv6_cidr` productions now require at least one `:`
so tokens like `proxy 10` or `cafe` no longer match as IPv6 and
silently get filed under a never-matching host route.
* `AclAddress::Ip` is parsed to a real `IpAddr` first, then turned
into a `/32` IPv4 host route OR a `/128` IPv6 host route. Previously
the code unconditionally tried `format!("{ip}/32").parse::<IpNet>()`
first; `ipnet` accepts `/32` for IPv6 and returned the entire
surrounding `/32` network (~96 bits wider than intended). Malformed
literals are now dropped with a warn log instead of silently
becoming `0.0.0.0/32`.
Tests
* +12 wind-core unit tests covering each rule/types/dispatcher change.
* +7 tuic-server tests covering the config-format inference fix and
the ACL IPv6/IPv4/malformed-IP code paths.
* +1 wind test covering the unknown-extension rejection path.
* Existing tests untouched and unchanged.
Local verification (Windows): `cargo test -p wind-core --lib pr4`,
`cargo test -p tuic-server --lib pr4`, and `cargo test -p wind --lib
pr4` all green. Full-workspace `cargo test --all-targets` deferred to
CI for speed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Same `mis` -> `miss`/`mist` false positive as before; `mis-detected` becomes `wrongly classified`. Same meaning, no false alarm. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment line rewrap on the heuristic-fix block in `tuic-server/config.rs::infer_config_format`. No code change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`dispatcher_routes_udp_to_correct_handler` hung under PR4-B because `Dispatcher::dispatch_udp` now `await`s the first packet (to route by its real target). The existing test constructed an empty `UdpStream` whose upstream sender was bound to `_tx2` — leading-underscore is a silent-lint hint, NOT an early drop, so the sender stayed alive for the whole `#[tokio::test]` function and `rx.recv().await` blocked forever. Push one packet through and drop the sender so the dispatcher can make its routing decision, hand the stream to the chosen handler, and return cleanly. Test now finishes in <1 ms locally; CI reported it stuck for 60s+ before this fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
Fourth wave of fixes from the full-project review — closes the routing/rule/config correctness findings.
Closed findings
rule_target_to_actionlower-cased outbound names, causingProxy_Outetc. to silently fall through todefaultNotable approach notes
Deferred / out-of-scope
Test plan
🤖 Generated with Claude Code