-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(es/minifier): Fix detection of direct eval
#6215
Conversation
eval
eval
eval
c(); | ||
var d = a.b; | ||
d(); | ||
var e = eval; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an indirect eval, so it cannot use c
nor b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swc-bump:
- swc_ecma_transforms_base
- swc_ecma_minifier
foo(); | ||
(0, foo.bar)(); | ||
(0, eval)("console.log(foo);"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also an indirect eval, so we can mangle names
var i = e[r] = { | ||
exports: {} | ||
}; | ||
return t[r](i, i.exports, o), i.exports; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix
Does this also fix #5068? |
No |
Actually this is a workaround and we should fix #5068 ideally |
if let Callee::Expr(e) = c { | ||
if let Expr::Ident(Ident { | ||
sym: js_word!("eval"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how you represent optional chains, but eval?.()
is not a direct eval either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh it was OptCall
which does not contain Callee
function f1(a, eval, c, d, e) { | ||
return a("c") + eval; | ||
function f1(n, eval, c, r, t) { | ||
return n("c") + eval; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this eval
can be renamed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added code to explicitly prevent it because terser
does so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automated review comment generated by auto-rebase script
Description:
The existence of
eval
make the name mangler abort, but partial try logic causes minifier to generate an invalid code.This is what I get if I disable eval detection.
With eval detection, I get
which is wrong
Related issue:
Investigation
Conclusion
eval
breaks the name mangler.