Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: suppress insecure resource warning for more local hostnames #30885

Merged
merged 3 commits into from
Sep 21, 2021

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Sep 8, 2021

Description of Change

Suppress security warnings for 127.0.0.1, [::1] and about:blank in addition to
'localhost'. Closes #30880.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • tests are changed or added

Release Notes

Notes: none

@nornagon nornagon added semver/patch backwards-compatible bug fixes target/13-x-y labels Sep 8, 2021
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 8, 2021
deepak1556
deepak1556 previously approved these changes Sep 8, 2021
const isLocal = (url: URL): boolean =>
['localhost', '127.0.0.1', '[::1]', ''].includes(url.hostname);
const isInsecure = (url: URL): boolean =>
['http:', 'ftp:'].includes(url.protocol) && !isLocal(url);
Copy link
Member

Choose a reason for hiding this comment

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

ftp urls will no longer resolve in main since #30503 :)

https://bugs.chromium.org/p/chromium/issues/detail?id=333943

@deepak1556 deepak1556 dismissed their stale review September 8, 2021 21:26

Failing tests looks related

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 9, 2021
@zcbenz zcbenz merged commit e38a0a6 into main Sep 21, 2021
@zcbenz zcbenz deleted the fix-insecure-local-warning branch September 21, 2021 06:47
@release-clerk
Copy link

release-clerk bot commented Sep 21, 2021

No Release Notes

trop bot pushed a commit that referenced this pull request Sep 21, 2021
)

* fix: suppress insecure resource warning for more local hostnames

* fix tests
trop bot pushed a commit that referenced this pull request Sep 21, 2021
)

* fix: suppress insecure resource warning for more local hostnames

* fix tests
trop bot pushed a commit that referenced this pull request Sep 21, 2021
)

* fix: suppress insecure resource warning for more local hostnames

* fix tests
@trop
Copy link
Contributor

trop bot commented Sep 21, 2021

I have automatically backported this PR to "13-x-y", please check out #31035

@trop
Copy link
Contributor

trop bot commented Sep 21, 2021

I have automatically backported this PR to "15-x-y", please check out #31036

@trop trop bot removed the target/13-x-y label Sep 21, 2021
@trop
Copy link
Contributor

trop bot commented Sep 21, 2021

I have automatically backported this PR to "14-x-y", please check out #31037

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Don't show security warning for localhost
3 participants