Replace ansi-html and fix the ReDoS vulnerability #3798
Conversation
carlobeltrame
requested review from
alexander-akait,
anshumanv,
hiroppy,
knagaitsev and
snitin315
as code owners
|
|
|
Well done..thank you.! refactor : Remove ansi-html dependency ? |
|
Oh, right, semantic commit messages. Done. |
package-lock.json
Outdated
| @@ -1,17939 +1,8 @@ | |||
| { | |||
| "name": "webpack-dev-server", | |||
| "version": "4.1.0", | |||
| "lockfileVersion": 2, | |||
| "lockfileVersion": 1, | |||
There was a problem hiding this comment.
We use "lockfileVersion": 2, it would be nice if you can commit package-lock.json with npm@7, to avoid large diffs.
There was a problem hiding this comment.
Of course, my bad. Done.
| @@ -0,0 +1,202 @@ | |||
| Apache License | |||
There was a problem hiding this comment.
We use MIT 😕 , would it be a problem?
There was a problem hiding this comment.
Yes, ansi-html was published under Apache 2.0. I cannot simply re-license code that isn't mine. Apache 2.0 is more restrictive than MIT, but it doesn't stop us from copying and adapting code, as long as the license is preserved.
There was a problem hiding this comment.
We can't accept this, sorry
There was a problem hiding this comment.
WE need rewrite code in this case on our solution
There was a problem hiding this comment.
In that case it might be far easier to just change the dependency from ansi-html to ansi-html-community. Since the original author of ansi-html is not reacting, someone else created a new npm package with the same code plus the fix.
| @@ -0,0 +1,122 @@ | |||
| /** | |||
There was a problem hiding this comment.
Not sure if modules is the correct directory for this, I see that webpack builds some separate bundles based on subdirectories of modules. Let me know if there is a better suited location.
As suggested by @alexander-akait in #3576 (comment)
For Bugs and Features; did you add new tests?
I moved and simplified the code from an unmaintained and vulnerable dependency to this project and fixed a ReDoS vulnerability, as suggested by @alexander-akait. I did not move the tests from that project - let me know if I should do that.
Motivation / Use-Case
Fixes #3576
As suggested by @alexander-akait in #3576 (comment)
Breaking Changes
None
Additional Info
If the Apache license and altogether slightly awkward position of this code in webpack-dev-server doesn't seem right, an alternative would be to use the new ansi-html-community package, which includes the CVE fix and promises to be better maintained. I'd be happy to create a PR for that as well. Let me know what you think.