Repository navigation
feat(go-native): let callers accept "--" host labels - #72
Merged
Merged
Conversation
The URI validator rejects any host label containing "--" outside the xn-- punycode prefix. That blocks legitimate hostnames such as S3 Express directory buckets (name--use1-az4--x-s3.s3express-...), and the Go native port had no way to relax it, unlike the Rust core's UriValidator::reject_double_dash. Add WithDoubleDashHostsAllowed for NewPolicy and NewAllowListPolicy, backed by SetRejectDoubleDashHosts on the engine validator. The default is unchanged and the shared conformance vector still passes. Every other check (denied suffixes, bare-IP and CIDR rules, control characters, schemes) is unaffected by the opt-out. Closes #71
|
|
Unpinned stable clippy gained unused_async_trait_impl, which flags the test-only DnsBackend impl that answers without awaiting. The trait method is async, so allow the lint on that impl.
The Build WASM job rebuilt core.wasm on every run and pushed the result as a github-actions[bot] commit. With an unpinned stable toolchain, that put a new binary on PRs that touched no Rust at all and made the CLA check flag the bot as a contributor. Diff the PR base (or the previous push) against HEAD for the Rust workspace, the build script, and this workflow, and skip the toolchain install, build, and commit steps when none of them changed. The job still runs so the Go and Node jobs that depend on it are unaffected.
mostafa
reviewed
Sep 21, 2026
mostafa
left a comment
mostafa
left a comment
Member
There was a problem hiding this comment.
The Go wiring is default-safe and the existing tests cover the main paths. A few notes on the public knob, coverage, and the wasm skip.
Review on #72: the opt-out is global, not S3 Express specific, and the godoc did not say so. Spell out that every "--" hostname passes the heuristic, point customer-URL callers at an allow-list pairing, and note that the Rust PolicyBuilder does not expose the setter yet. Tests: pin that the opt-out cannot skip allow-list mode, and check control characters in the host rather than only in the path. CI: also watch rust-toolchain files as core.wasm inputs.
… event Review on #72: operators could not see from the audit stream that hostname hardening was relaxed. Add double_dash_hosts_allowed to the policy_created fields. Go native only, since the Rust core does not expose the setter on its builder yet.
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.
Closes #71
The URI validator rejects any host label containing
--outside thexn--punycode prefix. That blocks legitimate hostnames such as S3 Express directory buckets, and the Go native port had no way to relax it, unlike the Rust core'sUriValidator::reject_double_dash.This adds
WithDoubleDashHostsAllowed()forNewPolicyandNewAllowListPolicy. The default is unchanged and the shared conformance vector still passes. The opt-out does not touch denied suffixes, bare-IP and CIDR rules, control-character rejection, or the scheme check.No new shared vector: the URL-validation vectors carry no per-case policy config, so an opt-in knob cannot be expressed there without changing every language's runner. The other bindings build the validator per request without config, so exposing the knob there is a separate change.