Skip to content

Fix Get through IPv4 aliases into compressed paths - #288

Open
oschwald wants to merge 1 commit into
mainfrom
greg/fix-get-through-alias-compressed-path
Open

oschwald wants to merge 1 commit into
mainfrom
greg/fix-get-through-alias-compressed-path

Conversation

@oschwald

@oschwald oschwald commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Tree.Get missed data stored in a compressed path when the lookup went through the Teredo (2001::/32) or 6to4 (2002::/16) IPv4 alias. The lookup kept its depth relative to the alias, but a compressed path stores its bits and end depth at IPv4-subtree depth (96 and deeper), so the walk compared the wrong bits.

When getRecord follows an alias record, it now copies the aliased IPv4 bytes to bytes 12 to 15 of a new address and walks the IPv4 root from depth 96. It converts the returned prefix length back to the caller's address. All three alias networks are byte-aligned, which a comment notes.

The new test inserts 1.2.3.0/29 and checks IPv4, Teredo, and 6to4 lookups for hits and for addresses that leave the path, before and after WriteTo. Without the fix, the alias lookups return nil before WriteTo.

No CHANGELOG entry: compressed paths are not in v1.2.0, so no released version had this bug.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • IPv4 network lookups through IPv4-alias, 6to4, and Teredo addresses now correctly find matching records when the network tree contains compressed paths.
    • Lookups that diverge from a matching path now return the appropriate broader network without an unrelated value.
    • These results remain consistent after the tree is written and loaded.

Tree.Get for an address in 2001::/32 or 2002::/16 returned no value
and a wrong prefix when the aliased IPv4 data was in a compressed
path. WriteTo expands the paths, so Get was correct only after a
write.

A compressed path stores its bits and end depth at their position
below ::/96. Through an alias, the lookup depth is relative to the
alias network, so the walk compared the wrong bits. Get now moves the
aliased IPv4 bits to ::/96 when it follows an alias and converts the
returned prefix length back to the alias network.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 19:51

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f23add70-be28-4771-8faa-8df6f35b7214
Commits

Reviewing files that changed from the base of the PR and between 34b590c and dba0ff8.

Files selected for processing: 2
  • node.go
  • tree_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

The change updates getRecord to handle alias records using the IPv4 root depth and translate the resulting prefix length. A new test checks lookups through IPv4, Teredo, and 6to4 addresses before and after WriteTo.

Changes: IPv4 Alias Lookup

Layer / File(s) Summary
Alias lookup and validation
node.go, tree_test.go
Alias lookups use the IPv4 root depth of 96 and adjust the returned prefix length to the original depth. The test checks a compressed /29 path through IPv4, Teredo, and 6to4 addresses before and after WriteTo, and checks broader-network results for addresses outside the path.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to dba0f

No actionable merge-blocking risk is established. The change addresses compressed-path alias lookups and adds regression tests; merge after normal checks pass.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Fix Get through IPv4 aliases into compressed paths" matches the main change. The raw summary and the objectives both describe a fix in Tree.getRecord for lookups through the IPv4 aliases (T…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Full details: Title check

Explanation

The title "Fix Get through IPv4 aliases into compressed paths" matches the main change. The raw summary and the objectives both describe a fix in Tree.getRecord for lookups through the IPv4 aliases (Teredo 2001::/32 and 6to4 2002::/16) when IPv4 data sits in a compressed path. The added test covers the same scenario, so the title is specific and accurate.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the IPv4 trail
Through Teredo hops and 6to4 mail
A compressed path is found just right
Then written out and checked in flight
The rabbit thumps: the prefixes align!

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

@github-actions

Copy link
Copy Markdown

Modver result

This report was generated by Modver,
a Go package and command that helps you obey semantic versioning rules in your Go module.

This PR does not require a change in your module’s version number.
(You might still consider bumping the patchlevel anyway.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants