Skip to content

Update isLocalURL to include LAN addresses, .local domains, and hostn…#5973

Open
cathaysia wants to merge 4 commits into
firefox-devtools:mainfrom
cathaysia:main
Open

Update isLocalURL to include LAN addresses, .local domains, and hostn…#5973
cathaysia wants to merge 4 commits into
firefox-devtools:mainfrom
cathaysia:main

Conversation

@cathaysia
Copy link
Copy Markdown

@cathaysia cathaysia commented Apr 28, 2026

Treat non-public addresses—such as loopback addresses, LAN addresses, and CGNAT addresses—as local addresses; this allows for direct access to computers within the local network, Tailscale and etc..

@cathaysia cathaysia force-pushed the main branch 2 times, most recently from 38f05a1 to c5cad4d Compare April 28, 2026 07:34
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.35%. Comparing base (b868d0e) to head (7b3c2f1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/utils/url.ts 96.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5973      +/-   ##
==========================================
+ Coverage   85.34%   85.35%   +0.01%     
==========================================
  Files         318      318              
  Lines       31922    31947      +25     
  Branches     8834     8851      +17     
==========================================
+ Hits        27244    27269      +25     
  Misses       4246     4246              
  Partials      432      432              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, adding some comments below.

Comment thread src/test/unit/url.test.ts
Comment thread src/utils/url.ts Outdated
Comment thread src/utils/url.ts Outdated
Comment thread src/utils/url.ts Outdated
Comment thread src/test/unit/url.test.ts
@cathaysia cathaysia force-pushed the main branch 2 times, most recently from 7dcfd1c to ee13e17 Compare April 30, 2026 10:37
@cathaysia
Copy link
Copy Markdown
Author

before:

image

after:

image

@cathaysia
Copy link
Copy Markdown
Author

can this pr be merged?

@fatadel
Copy link
Copy Markdown
Contributor

fatadel commented May 27, 2026

Hey @cathaysia, you need to re-request review when you've done working on the feedback. I'll do that for you now :)

@fatadel fatadel requested a review from canova May 27, 2026 09:53
@cathaysia
Copy link
Copy Markdown
Author

thanks

Copy link
Copy Markdown
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

Oops, missed this one.

Comment thread src/utils/url.ts Outdated
Comment on lines +24 to +30
/^127\./.test(hostname) || // Loopback 127.0.0.0/8
/^10\./.test(hostname) || // Private 10.0.0.0/8
/^172\.(1[6-9]|2[0-9]|3[0-1])\./.test(hostname) || // Private 172.16.0.0/12
/^192\.168\./.test(hostname) || // Private 192.168.0.0/16
/^100\.(6[4-9]|[7-9][0-9]|1[0-1][0-9]|12[0-7])\./.test(hostname) || // CGNAT 100.64.0.0/10
/^169\.254\./.test(hostname) || // Link-local 169.254.0.0/16
/^198\.(1[8-9])\./.test(hostname) // Benchmark 198.18.0.0/15
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These match hostnames like:

192.168.1.1.foo.com

URL.hostname will happily return these host names. Please make sure that the host names are actually ipv4 addresses.

Copy link
Copy Markdown
Author

@cathaysia cathaysia May 28, 2026

Choose a reason for hiding this comment

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

I updated the regex, could you please check it again?

Comment thread src/test/unit/url.test.ts
Comment thread src/utils/url.ts Outdated
@cathaysia cathaysia force-pushed the main branch 2 times, most recently from a76c6c5 to 1ae49e4 Compare May 28, 2026 07:28
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.

3 participants