Fix/485 port qualified host matching - #495
Conversation
Previously the port was stripped from the request authority before credential resolution, so a secret's hostPattern like example.internal:3000 could never match. host_matches now compares ports when the pattern specifies one, and falls back to matching any port for bare-hostname patterns, preserving existing behavior. Fixes onecli#485
|
Thanks for taking this on — the approach matches what we found when filing #485, and passing the port-inclusive authority at both A few notes from a close read:
Happy to test the branch against our live setup — seven port-qualified plain-HTTP servers behind one hostname, which is the scenario from the issue. |
|
Thanks for the detailed review - all three are spot-on! Fixed the dropped #[test] on host_exact_match. Got caught by the cargo fmt diff, sorry! Going to push an updated commit soon. Your real-world testing on that seven-ports-one-hostname case would be very helpful! |
- host_exact_match lost its #[test] attribute in the earlier fmt diff; restored it (was silently not running). - mitm.rs's per-request rule re-resolution inside the MITM tunnel was still stripping the port before calling resolve_from_cache, so a port-qualified secret would match at CONNECT time but fail to re-inject on subsequent requests. Fixed with the same approach as the CONNECT/plain-HTTP paths. - Documented two known edge cases in split_host_port per review feedback: implicit default ports aren't normalized against explicit :80/:443 patterns, and unbracketed IPv6 literals aren't handled.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
I have read the CONTRIBUTING.md file.
YES
What kind of change does this PR introduce?
Bug fix.
What is the current behavior?
Fixes #485.
Generic secrets can be created with a port-qualified
hostPattern(e.g.example.internal:3000), but the gateway strips the port from the requestauthority before resolving credentials.
host_matchesonly ever sees thebare hostname, so a port-qualified pattern can never match — the secret is
silently unusable and requests get
credential not found ... status=401.What is the new behavior?
connect::resolveat boththe CONNECT and plain-HTTP proxy paths, instead of the port-stripped
hostname.
host_matchessplits both the request host and the pattern intohost/port. A pattern with a port (
example.internal:3000) now requiresan exact port match. A bare-hostname pattern (
example.internal) stillmatches any port, so existing behavior is unchanged.
Verified locally by creating a generic secret with
hostPattern: example.internal:3000, granting it to an agent, and sending a requestthrough the proxy:
injection_count=0— secret never matched.injection_count=1— secret injected correctly.Added two unit tests covering both cases. Full
connect::suite passes(35/35),
cargo buildandpnpm checkpass clean.Additional context
Didn't change the secret-creation validation to reject malformed
hostname:portpatterns (mentioned as a "nice to have" in the issue) —happy to add that in a follow-up if wanted.