process: stricter sourceMappingURL regex #34392
Conversation
The library brrp, which itself processes source maps, had its processing logic erroneously loaded as a source map URL.
| } else { | ||
| const c = 102; | ||
| } | ||
| const sm = '//# sourceMappingURL=https://ci.nodejs.org/402' |
There was a problem hiding this comment.
this is technically an unsolvable problem, assuming an adversary opponent. V8 has APIs for this on UnboundScript, can we use those instead?
There was a problem hiding this comment.
V8 also just looks for comments of the form //[#@]\s<name>=\s*<value>\s*.*…
There was a problem hiding this comment.
For reference, here's a helper in the v8 inspector . I believe it might be susceptible to the same bug.
Should we just leave this as a known issue with source maps?
There was a problem hiding this comment.
What I mean is, UnboundScript::GetSourceMappingURL uses information from the actual parser, it knows the difference between an actual comment and a tricky string.
There was a problem hiding this comment.
Looking into this further, the helper in the V8 inspector is only used if parsing the script failed.
There was a problem hiding this comment.
@devsnek I think this would be a pretty significant refactor, and we'd need a similar fallback for when parsing fails.
I like the idea that in some cases we could just use v8's source map extraction, mind you.
There was a problem hiding this comment.
yeah not blocking or anything, but I think it would be much nicer.
There was a problem hiding this comment.
I'm on the fence with this PR, just because I fear the complex regex slowing down module loading ... perhaps before we land a fix like this I could dig in to your suggestion.
The library brrp, which itself processes source maps, had its
processing logic erroneously loaded as a source map URL.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes@jasnell I screwed up a rebase in such a way that I couldn't get #34305 to reopen.
This is #34305 rebased against the main branch.