Skip to content

WTF::URLParser should accept compressed IPv4-in-IPv6 addresses - #648

Open
steipete wants to merge 1 commit into
oven-sh:mainfrom
steipete:claude/url-ipv4-embedded
Open

steipete wants to merge 1 commit into
oven-sh:mainfrom
steipete:claude/url-ipv4-embedded

Conversation

@steipete

@steipete steipete commented Sep 13, 2026 •

Copy link
Copy Markdown

URLParser should parse IPv4 addresses as the last two pieces of an IPv6 address
https://bugs.webkit.org/show_bug.cgi?id=162991

Reviewed by NOBODY (OOPS!).

Follow-up to bug 162991. The IPv6 parser rejects a valid compressed address when the compression marker leaves two zero pieces before a dotted IPv4 tail. The compressPointer && piecePointer == 5 check is stricter than the WHATWG IPv6 parser, which rejects an embedded IPv4 address only when pieceIndex is greater than 6.

Remove the extra rejection. The existing piece bounds and compression logic continue to reject addresses that do not fit. Add regression coverage for loopback, private, and public dotted tails and an overfull invalid address. Ada and Node accept the valid forms and canonicalize the dotted tail into two hexadecimal pieces.

AI-assisted: implementation and tests were developed with Codex and reviewed by a separate Codex pass; I inspected and validated the result.

Validation:

  • Node 26/Ada accepts and canonicalizes the three valid cases and rejects the invalid boundary.
  • Stock Bun 1.4.2, using the current pinned parser, rejects the three valid cases before this change.
  • git diff --check and a focused P0-P2 review pass.
  • TestWTF was not run locally because it requires a local WebKit build; the added WebKitAPI cases are included for upstream CI.
  • Source/WTF/wtf/URLParser.cpp:
    (WTF::URLParser::parseIPv6Host): Remove the nonstandard compressed IPv4-in-IPv6 rejection.
  • Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp:
    (TestWebKitAPI::TEST_F): Cover valid canonicalization and the overfull invalid boundary in WTF_URLParser.ParserDifferences.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 07f286c6-50dc-4ed8-a136-8ea53bc5b554

📥 Commits

Reviewing files that changed from the base of the PR and between 28f58fb and e1daa80.

📒 Files selected for processing (2)
  • Source/WTF/wtf/URLParser.cpp
  • Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp
💤 Files with no reviewable changes (1)
  • Source/WTF/wtf/URLParser.cpp

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

The IPv6 parser now accepts compressed literals with an embedded IPv4 tail after five preceding pieces. Tests cover three normalized valid addresses and one overfull invalid address.

Changes

IPv6 embedded IPv4 parsing

Layer / File(s) Summary
Parser acceptance and validation coverage
Source/WTF/wtf/URLParser.cpp, Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp
parseIPv6Host no longer rejects the targeted compressed form. ParserDifferences checks IPv4-tail normalization for three valid literals and rejects one overfull literal.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e1daa

The change adds targeted compressed IPv6 dotted-tail support with regression coverage; no confirmed production or test-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description identifies Bugzilla bug 162991, states the review status, explains the fix, lists changed files and functions, and documents validation and test limitations. It substantially follows t…
Title check ✅ Passed The title clearly identifies the primary change: accepting compressed IPv4-in-IPv6 addresses in WTF::URLParser.

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Comment @coderabbitai help to get the list of available commands.

@steipete steipete changed the title fix(url): accept compressed IPv4-in-IPv6 addresses WTF::URLParser should accept compressed IPv4-in-IPv6 addresses Sep 13, 2026
@robobun

robobun commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

#909 carries this commit unchanged, with you as its author. It adds the fix for the other difference between parseIPv6Host and the URL Standard: an address that ends with a single : parses today.

#909 is on a branch of this repository, so CI builds it and runs the WTF_URLParser rows of this pull request. With both commits, Bun and Node.js v26.3.0 agree on a corpus of 334,619 bracketed hosts. If you or a maintainer prefer to land this pull request by itself, #909 can drop the commit.

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.

2 participants