Fix getRootDomain incorrectly truncating IP address literals#63
Fix getRootDomain incorrectly truncating IP address literals#63
Conversation
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes getRootDomain behavior for IP-literal hostnames so redirect-domain comparisons don’t incorrectly truncate IP addresses (e.g., 192.168.1.100 → 1.100).
Changes:
- Added IPv4/IPv6 literal detection to
utils.jsand early-return ingetRootDomainfor IPs. - Moved
IPV4_REGEX/IPV6_REGEXdefinitions frombackground.jsintoutils.jsto avoid duplication.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| utils.js | Adds IP-literal handling to getRootDomain and hosts shared IP regexes. |
| background.js | Removes duplicate IP regex definitions (now relying on utils.js being loaded first). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if (IPV4_REGEX.test(hostname) || IPV6_REGEX.test(hostname)) { | ||
| return hostname; |
There was a problem hiding this comment.
getRootDomain is documented as returning a normalized root domain, but for IP literals it now returns hostname as-is. This can lead to comparison mismatches (e.g., uppercase hex in IPv6, or bracketed IPv6 like [::1]) versus the lowercased/normalized values used elsewhere. Consider normalizing IP inputs too (lowercasing and stripping optional [/]) before returning.
| return hostname; | |
| const normalized = hostname.toLowerCase().replace(/^\[/, "").replace(/\]$/, ""); | |
| return normalized; |
| ["uk", "en-GB,en;q=0.9"], | ||
| ["za", "en-ZA,en;q=0.9"] | ||
| ]); | ||
| const IPV4_REGEX = /^(25[0-5]|2[0-4]\d|1?\d?\d)(\.(25[0-5]|2[0-4]\d|1?\d?\d)){3}$/; | ||
| const IPV6_REGEX = | ||
| /^\[?(?:(?:[a-f0-9]{1,4}:){7}[a-f0-9]{1,4}|(?:[a-f0-9]{1,4}:){1,7}:|(?:[a-f0-9]{1,4}:){1,6}:[a-f0-9]{1,4}|(?:[a-f0-9]{1,4}:){1,5}(?::[a-f0-9]{1,4}){1,2}|(?:[a-f0-9]{1,4}:){1,4}(?::[a-f0-9]{1,4}){1,3}|(?:[a-f0-9]{1,4}:){1,3}(?::[a-f0-9]{1,4}){1,4}|(?:[a-f0-9]{1,4}:){1,2}(?::[a-f0-9]{1,4}){1,5}|[a-f0-9]{1,4}:(?:(?::[a-f0-9]{1,4}){1,6})|:(?:(?::[a-f0-9]{1,4}){1,7}|:))\]?$/i; | ||
|
|
||
| const readLocationHeader = (headers = []) => { |
There was a problem hiding this comment.
background.js still uses IPV4_REGEX/IPV6_REGEX (e.g. in buildAcceptLanguage) but the definitions are now in utils.js, creating an implicit cross-file global dependency on manifest script order. To prevent accidental breakage if the load order changes, consider adding a short comment here noting the dependency, or centralizing the shared constants under an explicit shared namespace (e.g., globalThis.TRUEPATH_IPV4_REGEX).
getRootDomainsplits hostnames on.to extract the root domain, causing IP addresses like192.168.1.100to return1.100instead of the full address — breaking redirect domain comparison for IP-hosted servers.Changes
utils.jsIPV4_REGEXandIPV6_REGEXhere frombackground.js(loads first per manifest script order, so both files share the same global scope)getRootDomainfor IP literals — returns hostname as-isbackground.jsIPV4_REGEX/IPV6_REGEXdefinitionsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.