Skip to content

feat: WHATWG URL component setters and expanded conformance test coverage#54

Open
OmarAlJarrah wants to merge 19 commits into
mainfrom
feat/whatwg-url-setters-conformance
Open

feat: WHATWG URL component setters and expanded conformance test coverage#54
OmarAlJarrah wants to merge 19 commits into
mainfrom
feat/whatwg-url-setters-conformance

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Adds a WHATWG-conformant URL component setter API to Url, and closes the two
remaining WPT conformance gaps versus reference implementations (URL setters and
percent-encoding). Also adds three test forms that go beyond replaying fixed
corpora: round-trip invariants, a java.net differential, and a seeded fuzzer.

Setter API (Url)

Nine immutable setters, each implementing the corresponding WHATWG URL setter
algorithm and returning a new Url (or the receiver unchanged when the setter is
a no-op):

  • withProtocol, withUsername, withPassword
  • withHost, withHostname, withPort(String)
  • withPathname, withSearch, withHash

These follow the WHATWG contract of being no-ops on invalid input rather than
throwing — the deliberate difference from Url.Builder, which validates and throws.
withPort(String) is a new overload alongside the existing withPort(Int?)
convenience.

Internally they reuse the existing parser state machine through a new state-override
entry point (UrlParser.parseWithOverride), so the setters share the same
conformance-tested engine as full parsing rather than reimplementing component
parsing. The public getter surface is unchanged; new methods are captured in the JVM
and Android API snapshots.

Conformance suites

  • URL setters — runs the WPT setters_tests.json corpus (278 cases) against the
    new setters via a getter adapter, ratcheted against a known-failures baseline like
    the existing URL parsing suite. All cases pass except two, which are tracked: both
    are the invalid ACE label xn-- being accepted by host parsing — a pre-existing
    IDNA host-validity gap that also affects full parsing and is out of scope here.
  • Percent-encoding — runs the WPT percent-encoding.json corpus against the
    percent codec (empty known-failures).

Both fixtures are generated from the vendored WPT corpora by new Go generators under
tools/internal/codegen/ (generateSetterTestData, generatePercentEncodingTestData),
consistent with the existing fixture generation.

Beyond fixed corpora

  • Round-trip invariants — over the WPT parsing corpus, asserts serialization is
    idempotent and newBuilder().build() reproduces the original href. This caught a
    real defect: making the pathname getter WHATWG-correct (excluding the /. path
    prefix) had left the builder's recompose path without that prefix, so an
    authority-less URL whose path begins with // failed to rebuild. Fixed by having the
    builder pre-fill from the guarded full-URL path serialization; the getter stays
    spec-correct.
  • java.net differential — compares parsing against java.net.URI on aligned
    inputs and documents the intentional WHATWG-vs-RFC-3986 divergences (host
    case-folding, backslash-to-slash for special schemes) with a rationale per row.
  • Seeded fuzzer — deterministic generative inputs asserting the parser never throws
    and that any successful parse serializes idempotently.

Follow-ups (out of scope)

  • Host parsing accepts the empty ACE label xn--; teaching domain-to-ASCII to reject
    it belongs with the IDNA engine and its reference lock-step.
  • The Uri profile may share the same path-getter/serialization conflation that was
    fixed here for Url; worth a look separately.

Testing

./gradlew build passes across all targets (JVM, JS, Wasm, native); the full JVM
suite is green (URL parsing, IDNA, NFC, href round-trip, the two new conformance
suites, and the three invariant/differential/fuzz suites). API snapshots regenerated
for the new setters.

Also fix the port state to end the digit run on any trailing character
when a state override is in effect (WHATWG URL port state), not just an
authority terminator, so the port setter accepts trailing junk (e.g.
"8080stuff" -> port 8080) per spec.
Add a Go generator (setters.go) that turns the vendored WPT
setters_tests.json corpus into SetterTestData.kt, following the same
chunked-builder shape as the existing url/nfc-test generators. Wires up
a generateSetterTestData Gradle task alongside the other codegen tasks.

This is fixture generation only; the test that consumes
SETTER_TEST_CASES comes next.
Adds UrlSetterConformanceTest, driving all 278 WPT setters_tests.json
cases against the Url.with* setter family with a ratcheted
known-failures baseline (kuri architecture: URL setters live behind
UrlParser.parseWithOverride, a shared engine with the full parser).

Getting this corpus green surfaced a dozen real bugs in the setter
override machinery, all fixed here:

- withPort/withProtocol pre-validated the raw value before the engine's
  own tab/newline stripping, rejecting inputs that are valid once
  stripped (e.g. "\t8080", "h\r\ntt\tps").
- schemeStartState/schemeState didn't fail cleanly under a setter
  override, so an invalid protocol value could restart parsing instead
  of resolving to a no-op.
- Port-state override handling only recognized StateOverride.PORT, so
  a host/hostname setter whose value carried a port (e.g.
  "example.com:8080/stuff") kept parsing past the port into the path,
  corrupting it; and an empty digit run wrongly cleared an existing
  port to null.
- Host-state was missing two WHATWG no-op guards: a colon under a
  hostname override must return before ever touching the host, and
  clearing a host to empty must be refused when the URL already has
  credentials or a port.
- FILE_HOST always fell through into path-start regardless of
  override, and the override loop's Reconsume handling terminated
  before ever invoking the target state at EOF, together breaking
  file: URL host clearing and root-path synthesis under override.
- Path state treated '?' as always opening a query, even under a
  pathname-setter override where it must stay literal path text.
- withHash bypassed the shared engine and so missed the mandatory
  tab/newline strip.
- Serializer's NORM-18 "/." anti-authority guard (needed only to keep
  a full href re-parseable) was leaking into the standalone pathname
  getter.

One known, out-of-scope residual remains and is recorded in
KNOWN_FAILURES with justification: HostParser accepts the ACE label
"xn--" (empty Punycode suffix) instead of rejecting it, verified as a
pre-existing IDNA-validity gap independent of the setter path.

Also corrects a pre-existing StateOverrideParseTest expectation that
predates this corpus and turns out to be wrong per the WPT ground
truth: a hostname setter value containing a colon is a total no-op,
not a partial host change.
Add a Go generator (percentencoding.go) that turns the vendored WPT
percent-encoding.json corpus into PercentEncodingTestData.kt, following
the same chunked-builder shape as the url/setters generators. Only the
utf-8 expectation is emitted per case, since UTF-8 is kuri's sole
charset; other charsets carried by the corpus (big5, shift_jis, ...)
are out of scope. Wires up a generatePercentEncodingTestData Gradle
task alongside the other codegen tasks.

This is fixture generation only; the test that consumes
PERCENT_ENCODING_TEST_CASES comes next.
Assert PercentCodec.encode(input, PercentEncodeSets.C0_CONTROL)
reproduces every WPT percent-encoding.json utf-8 expectation, mirroring
UrlConformanceTest's ratchet shape (untracked-regressions test plus a
known-failures-equals-live-failures test). All 7 in-scope cases pass
against the existing codec with no known failures.
…ty-less // paths

Url.newBuilder().build() threw IllegalArgumentException ("an authority-less
path cannot begin with '//'") for authority-less non-special URLs whose path
begins with an empty segment, e.g. non-spec:/.//path.

The builder's pre-fill constructor sourced its path from the encodedPath
getter, which correctly omits the NORM-18 /. anti-authority guard per the
WHATWG standalone-pathname algorithm. But the builder's recompose concatenates
that path directly onto scheme: with no authority, so the unguarded // re-parses
as a spurious authority and the parser rejects it. Pre-fill instead from the
guarded full-URL path serialization (serializeUrlPath's default), matching the
canonical href and leaving the pathname getter unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant