Repository navigation
fix(bugs): allow mailto urls through the opener validation - #10073
Open
ManoharPaturi wants to merge 1 commit into
Open
ManoharPaturi wants to merge 1 commit into
ManoharPaturi wants to merge 1 commit into
Conversation
The bugs command turns a bugs.email manifest field into a mailto url and hands it to openUrl, whose protocol check only accepts http and https, so a package documenting only an email address failed with Invalid URL instead of opening anything. The command tests mock openUrl entirely, so the rejection never showed up in the suite. Accept mailto alongside http and https. The scheme is inert in browsers and mail clients, unlike the scriptable schemes the check exists to keep away from the opener.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
npm bugssupports packages that only document an email address by building amailto:url frombugs.emailand passing it toopenUrl(lib/commands/bugs.js). The protocol guard inopenUrlonly acceptshttpandhttps, so the command fails before opening anything:The existing bugs tests mock
openUrlitself, so the rejection never surfaced in the suite, and the mailto cases in those tests only assert the url is requested, never that it survives validation.Solution
Accept
mailtoalongsidehttp(s)inassertValidUrl. The scheme is inert in browsers and mail clients, unlike the scriptable schemes (javascript,data,file) the guard exists to keep away from the opener, and the explicitisFilecarve out for local files is untouched.Test Evidence
test/lib/utils/open-url.jsasserts amailto:url is passed through to the opener, next to the existing tests that still rejectftp:,file:and unparseable urls.test/lib/commands/bugs.jsruns the realopenUrl(only the process opener is mocked) for a package withbugs.emailand asserts the mailto url reaches the opener.The regression test fails on the previous code with
Invalid URL: mailto:hello@example.comand passes with this change. Both files green:npx tap test/lib/utils/open-url.js test/lib/commands/bugs.js.References
lib/commands/bugs.jsgetUrl