-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
Backport allowAbsoluteUrls vuln fix to v0.x #6829
Backport allowAbsoluteUrls vuln fix to v0.x #6829
Conversation
running tests locally, 1 test fails
the same test fails on the v0.x branch |
lib/core/buildFullPath.js
Outdated
if (baseURL && !isAbsoluteURL(requestedURL)) { | ||
module.exports = function buildFullPath(baseURL, requestedURL, allowAbsoluteUrls) { | ||
var isRelativeURL = !isAbsoluteURL(requestedURL); | ||
if (baseURL && isRelativeURL || allowAbsoluteUrls === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the logic here is wrong. see #6833.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you kind sir for pointing that out 🙏🏽
@jasonsaayman Please take a look when you get a chance. |
@thatguyinabeanie you have pushed a compiled file, can you please remove that? thanks |
all set |
Hi @mhassan1 please review the changes if you get a chance. |
The typing changes are missing from this PR; see #6818. |
Thanks, @thatguyinabeanie, for the fix, and @jasonsaayman and @mhassan1 for reviewing! Is this scheduled for release soon, or should we wait for the typing changes to be merged first? Also, I noticed that #6833 by @mhassan1 doesn’t include the strict equal comparison @jasonsaayman included in this PR. Is that intentional, or should we open an issue for it? |
@mhassan1 thanks for pointing that out! @jasonsaayman i opened a new PR that adds the types #6849 |
thanks |
Describe your pull request here.
Backports fix for github/advisory-database#5356 on v1.x onto v0.x
Existing issue: #6824