Fix: Support single character extension. (issue #38) #39
Conversation
lib/extension.js
Outdated
| @@ -1,7 +1,7 @@ | |||
| var path = require('path'); | |||
|
|
|||
| var EXTRE = /\.[^.]*/g; | |||
| var LONGEXTRE = /^[.]?[^.]+([.].+[^.])$/; | |||
| var LONGEXTRE = /^[.]?[^.]+([.].*[^.])$/; | |||
There was a problem hiding this comment.
@sttk will this cause a ReDOS issue? Maybe it needs to be this:
| var LONGEXTRE = /^[.]?[^.]+([.].*[^.])$/; | |
| var LONGEXTRE = /^[.]?[^.]+([.][^.]*[^.])$/; |
Do you agree?
There was a problem hiding this comment.
@phated That regexp does not pass tests.
$ node
> /^[.]?[^.]+([.][^.]*[^.])$/.test('aaa.js')
true
> /^[.]?[^.]+([.][^.]*[^.])$/.test('aaa.bbb.js')
false
> /^[.]?[^.]+([.][^.]*[^.])$/.test('aaa..bbb.js')
false
And I checked the regexp /^[.]?[^.]+([.].*[^.])$/ with some tools, and the vulnerability was not detected.
There was a problem hiding this comment.
@sttk I don't think those tools work correctly because I submitted the ReDOS RegExp from gulpjs/glob-parent@f923116 and all the tools claim it is safe.
There was a problem hiding this comment.
@phated Because I don't know how to avoid ReDOS, I replaced regex processes in extension.js to alternative ways.
The following table is the benchmark result of extension.js before and after modification.
| | before | after |
|:-------------------------|------------------:|-------------------:|
| file.js | 3,465,168 ops/sec | 17,745,432 ops/sec |
| file.tmp.dot.js | 1,453,870 ops/sec | 6,729,040 ops/sec |
| relative/path/to/file.js | 3,292,908 ops/sec | 14,813,461 ops/sec |
- Platform: Node.js 15.5.0 on Darwin 64-bit
- Machine: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz, 64GB
|
@phated Never mind. Thank you for merging this PR. |
This pr is to fix the issue #38.