Skip to content

fix(libScePosixForWebKit): use a numeric host as its canonical name when the host leaves it empty - #11859

Merged
oneandonlydean merged 4 commits into
boykopovar:mainfrom
jotitadev:fix/macos-numeric-canonical-name
Nov 2, 2026
Merged

oneandonlydean merged 4 commits into
boykopovar:mainfrom
jotitadev:fix/macos-numeric-canonical-name

Conversation

@jotitadev

Copy link
Copy Markdown
Contributor

What

guest_raw_resolver aborts on macOS with raw resolver: numeric canonical name differs: getaddrinfo with AI_CANONNAME|AI_NUMERICHOST on 127.0.0.1 or ::1 returns no canonical name.

FreeBSD 11.4's explore_numeric sets ai_canonname to the numeric host itself when AI_CANONNAME is asked ("based on a clarification in RFC3493"), and copy_ai carries it to every record. glibc sets it on the first record. macOS's getaddrinfo leaves it null for a numeric host, with or without AI_NUMERICHOST (a name such as localhost does get one). Measured with a plain C probe:

getaddrinfo with AI_CANONNAME macOS 27.2 Linux (glibc)
127.0.0.1, ::1, datagram null 127.0.0.1, ::1
127.0.0.1, any socket type null, null 127.0.0.1, null, null

When the guest asks for AI_CANONNAME and the node is a numeric IPv4 (the same inet_aton parse gethostbyname uses) or IPv6 address, or AI_NUMERICHOST was given, a record the host left without a canonical name now gets the node's text. On macOS that fixes every record; on Linux it only fills the later records of a multi-record numeric answer, as FreeBSD does. A host-provided name is never replaced.

Tested

Overlap: #5021 touches Resolver.cpp only in gethostbyname, which main already has; git merge-tree merges this branch with #5021 without a conflict in that file.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none for the change. Building the test on macOS needs the open macOS series listed above.
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

…hen the host leaves it empty

FreeBSD's getaddrinfo sets ai_canonname to the numeric host itself when AI_CANONNAME is asked for a numeric address (explore_numeric, after RFC 3493), on every record it returns. glibc does it on the first record, and macOS leaves it null, so on macOS a guest got no canonical name for 127.0.0.1 or ::1.

When AI_CANONNAME is asked and the node is a numeric IPv4 or IPv6 address, a record the host left without a canonical name gets the node's text.
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5021 core/libs/prx/libScePosixForWebKit/Resolver.cpp

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found one edge case in the numeric-name detection. The fallback recognizes IPv6 only when inet_pton(AF_INET6, node, ...) succeeds, which excludes scoped literals such as fe80::1%en0; I verified inet_pton rejects the scoped form while getaddrinfo accepts it. FreeBSD 11.4 handles the %scope suffix separately and passes the original hostname as the canonical name to its numeric-address path (see explore_numeric_scope in getaddrinfo.c). Could you cover scoped IPv6 with AI_CANONNAME too, or narrow the stated behavior? I have not reproduced the missing canonical name on macOS for this scoped form, so this is a coverage concern rather than a confirmed target failure. The focused Linux/macOS reports and existing checks otherwise look coherent; no independent resolver test was run locally.

…ic host for its canonical name

A scoped literal such as fe80::1%1 is numeric for getaddrinfo, but inet_pton rejects the %scope suffix on glibc. Check the address before the suffix and keep the whole node as the canonical name, as FreeBSD's explore_numeric_scope passes the original hostname to explore_numeric.
@jotitadev

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks, covered in b97c5b6 (new commit, no force-push).

  • The numeric check now runs inet_pton(AF_INET6) on the part before % and keeps the whole node as the canonical name, as explore_numeric_scope passes the original hostname on.
  • guest_raw_resolver gets a scoped case: fe80::1%1, raw hints, with AI_CANONNAME and with AI_CANONNAME|AI_NUMERICHOST, expecting fe80::1%1 as the canonical name.

What I measured with a C probe:

  • macOS 27.2: getaddrinfo accepts fe80::1%1 and leaves ai_canonname null. Its inet_pton also accepts the scoped form, so the previous head already covered this case on macOS.
  • Linux (glibc): inet_pton rejects the scoped form, as you saw, but glibc fills ai_canonname (fe80::1%1) itself.

So the change mainly stops the check from depending on how the host's inet_pton treats the suffix.

Results:

  • macOS (x86-64 under Rosetta, main 801d5b9 + the macOS series): with only the scoped check moved first and without this PR, it fails with scoped numeric canonical name differs 3 of 3. At b97c5b6 guest_raw_resolver passes 3 of 3.
  • Linux (Ubuntu 26.04, GCC 15.2, clean clone of b97c5b6): guest_raw_resolver passes 3 of 3.
  • tools/check_conventions.py passes.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The scoped IPv6 follow-up addresses the reported case and adds the requested regression. I spotted one allocation-failure edge in the new fallback: after getaddrinfo succeeds, constructing std::string address can throw std::bad_alloc before freeaddrinfo(native) runs. This function has no surrounding catch/RAII cleanup, so the exception can cross the C ABI and leak the host addrinfo list. Please consider using a bounded stack buffer for the IPv6 portion or an RAII owner for native (and map allocation failure to the function’s normal error result). I did not reproduce OOM; this is a source-level exception-safety finding. The focused resolver test passed under QEMU TCG on the exact head, but this local host required emulation because its CPU lacks the project’s AVX2/FMA requirement.

…ting in the canonical name fallback

Copy the address part before %scope into a bounded stack buffer instead of a std::string, so the check after a successful host getaddrinfo cannot throw before freeaddrinfo and the exception cannot cross the C ABI. An address longer than the buffer is not a numeric IPv6 literal.
@jotitadev

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks, done in c2a143da80 (new commit, no force-push). The address part before %scope is now copied into a 64-byte stack buffer and passed to inet_pton, so nothing between the successful host getaddrinfo and freeaddrinfo allocates or throws. A node whose address part does not fit is not a numeric IPv6 literal (the longest textual form is 45 characters), so it falls through to the existing path.

Linux (Ubuntu 26.04, GCC 15.2, Release): guest_raw_resolver and guest_resolver pass 3/3 on this head, including the scoped fe80::1%1 case from the previous commit, and git diff --check is clean. I did not rerun macOS for this commit. The change only replaces the std::string with the stack buffer and does not change the result: the same bytes reach inet_pton on both hosts.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@maskym

maskym commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Windows result for head c2a143da80, merged into main f759e165d9 (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #11945, #11899, #11772, #11087, #10789, #11627, #11354, all merged cleanly, so the run is a joint one; the tests of this PR are guest_raw_resolver (0.04 s), all passed (the batch's 14 tests: 14/14). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed — approved

The stack-buffer follow-up resolves the allocation-failure path I flagged, and the scoped numeric IPv6 case is now covered.

What I checked

  • Reviewed exact head c2a143da8050d1253f83abcf709072287919dd0b against live main 10bec5c322eb581b3e959e6c7e18bcf09e068e96.
  • The new fallback strips %scope only for the inet_pton(AF_INET6) classification, keeps the original node as the canonical name, and uses a fixed 64-byte stack buffer. It introduces no allocation between successful getaddrinfo and freeaddrinfo.
  • Host-provided canonical names retain precedence. The existing numeric IPv4 fallback and the AI_CANONNAME / AI_NUMERICHOST mappings are consistent with the new branch.
  • Current-main merge preview is clean. The touched resolver and test files have not changed on main since the PR base.

Results

Check Result
Current-main merge preview Clean (92237d620cf093f8529753146a38b1b9cdf8c33e)
Diff check Pass; no output
Conventions Pass (All conventions checks passed.)
Focused Linux tests Author reports guest_raw_resolver and guest_resolver passed 3/3 on the latest head
Focused Windows tests Independent report says the PR test passed in a 14/14 test batch on the exact head, merged locally with its stated main and prerequisite PRs
Focused macOS tests Author reports 3/3 on the earlier functional head; the final change only replaces the temporary string with a stack buffer and was not rerun on macOS
Hosted checks Windows, conventions, labels, eligibility, and report pass; Linux and specialized product jobs skipped
Raw local output
merge-base: 801d5b9bc11049e6274daca09d70690bd5b8d0a9
merge-tree: 92237d620cf093f8529753146a38b1b9cdf8c33e
All conventions checks passed.
git diff --check: no output (pass)

Negative control

The author reports the original macOS failure 3/3 and the fixed resolver test passing 3/3. Linux passes on the latest head. No separate mutation or forced-failure run is reported.

Findings

Blocking

None found in the scoped resolver change.

Non-blocking

I did not independently build or run the resolver tests. The runtime results above are author and independent-review reports. The final stack-buffer-only commit was not rerun on macOS.

Sources

Not verified

I did not run platform builds, CTest, title behavior, or console behavior locally. The macOS result applies to the preceding functional head, not the final stack-buffer-only commit.

To get this to a good state

No further change is required for the resolver correctness and exception-safety findings reviewed here.

@imellag

imellag commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Confirmed on a second Mac with today's main. Apple M4 Pro, macOS 27.2, Apple clang 21.0.0, x86-64 Release under Rosetta 2. The tree is support/macos-boot-1030c (05bd71a66b, main 21a556f567 + the open macOS PRs), without and with head c2a143da80 merged (clean merge):

  • Base: guest_raw_resolver aborts at numeric canonical name differs in 3 of 3 runs. It was 1 of the 19 failures in a full CPU-only ctest run on that tree; guest_resolver and call_site_resolver pass.
  • This PR: guest_raw_resolver passes 5 of 5 (--repeat until-fail:5), and guest_resolver and call_site_resolver still pass.

AI-assisted: yes

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 1, 2026
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 2, 2026
@oneandonlydean
oneandonlydean merged commit 4f8d512 into boykopovar:main Nov 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants