Remove url-regex from dependency tree

[richardowen]

This PR contains:

• bugfix
• feature
• refactor
• tests
• documentation
• metadata

Breaking Changes?

• yes
• no

If yes, please describe the breakage.

Please Describe Your Changes

This PR removes url-regex from the dependency tree. There is an open vulnerability in url-regex (kevva/url-regex#70) and no patch available.

The url-regex dependency wasn't actually used by this package but was a dependency of is-url-superb v3. It has been removed as a dependency of that package too in v4 so this PR upgrades is-url-superb to v4 as well.

Version 4 of is-url-superb requires Node.js v10+ so I've updated the requirements of this package to match. Older Node.js versions are now unsupported so I hope this is acceptable.

The test snapshot change is due to the change in how is-url-superb determines whether or not a string is a URL. You can see a similar change in sindresorhus/is-url-superb@663eb59. If removing that "url" from the fixture is preferable I can do that instead.

BREAKING CHANGE: This will require a major version bump due to the supported Node.js versions changing.

[richardowen]

The CI fail looks to be because of a different vulnerability in the dependency tree, this time with yargs-parser. It requires a new version of @commitlint/cli to be released first (see conventional-changelog/commitlint#1691).

[shellscape]

Thanks, I'll take a look at this in more depth later this week.

[richardowen]

@shellscape have you had time to look at this yet? It would be good to get this released so I can then move on to updating packages that depend on this package (which will be needed if this is going to be a major version bump).

[shellscape]

So we won't be able to merge this because a protocol-relative uri is still a valid uri in the CSS realm. Using

.foo { background: url(//mysite.example.com/mycursor.png); }

and https://jigsaw.w3.org/css-validator/#validate_by_input, we can see that it's still valid. Just changing the test snapshots won't do, as they're incorrect. I've opened this issue sindresorhus/is-url-superb#6 with is-url-superb in the mean time.

npm audit still doesn't identify this as a high vulnerability, and as this is only intended to be used in a toolchain in a dev environment, any DoS would be self-inflicted. For all of these reasons, I have to pass on this PR. I do appreciate the time and attention you've given to it, however.

[richardowen]

No problem, that makes sense.

It looks like is-url-superb just uses the Node.js URL class to determine if a URL is valid. So I'm not convinced that is-url-superb should be changed, given that it is a Node.js package and not concerned with CSS. It's this package (postcss-values-parser) that wants CSS compatibility, so perhaps the URL check should be implemented in this package? It's easy enough to do a check with the URL class for the majority of URLs anyway (not sure is-url-superb is really needed...) and then would just need to add an extra check for protocol-relative URLs.

[shellscape]

Yeah it was definitely used as a convenience. I'm up for removing it if there's an equivalent solution in place.

[shellscape]

Welp. Sindre is saying that because it's considered an anti-pattern by some influential people, that he's not going to support it. That doesn't change the fact that all of the vendors still support it and that it's still valid per CSS spec. So we'll have to drop that package in favor of another solution unfortunately.

[richardowen]

I'm afraid putting together a pull request to replace the dependency is not something I feel confident doing; I'm not a JavaScript developer!

I'll create an issue to track this and hopefully someone can look at it.

[shellscape]

Yes please do create that issue. I'll try to get to it sooner rather than later.

[davilima6]

@shellscape, I don't think the W3C validator is a good source for truth on this topic as it validates anything inside url(), even empty strings or no parameters at all. Would you have a link to the spec?