fix: inline origin resolution, drop original dependency#281
Merged
Conversation
joeybaker
approved these changes
Jun 8, 2022
Contributor
joeybaker
left a comment
joeybaker
left a comment
There was a problem hiding this comment.
I've got a bit of a nit suggestion, but seems good!
| */ | ||
| function getOrigin (url) { | ||
| if (typeof url === 'string') url = parse(url) | ||
| if (!url.protocol || !url.hostname) return 'null' |
Contributor
There was a problem hiding this comment.
IMO 'null' is a little confusing. It could be perceived as a bug? Maybe we should return 'invalid url'?
Member
Author
There was a problem hiding this comment.
It is confusing, but it is also in accordance with RFC6464, so I think I will leave it like this.
glensc
reviewed
Jul 5, 2022
| * | ||
| * @param {String|Object} url URL to transform to it's origin. | ||
| * @returns {String} The origin. | ||
| * @api private |
There was a problem hiding this comment.
should leave @link https://github.com/unshiftio/original/blob/507b362269c0ad4405d095aedc227c40aecaf68a/index.js as credit to orignal code.
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
4 participants
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.
Apparently the
originaldependency depends onurl-parsemodule which recently saw some security issues reported. We are no longer depending on this as of 2.0, but the 1.x range does. In order to minimize noise related to this vulnerability, this PR moves the few lines of logic we depended on from this module into the 1.x branch. This ensures we still support node 12 (and 10?).If approved, I will update the changelog and release a new patch release on the 1.x range - but I am eager to call this the last fix I will be doing for the 1.x range - ideally I would like to move to node 14 as the lowest supported version, following node LTS.