Skip to content
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

[Bug]: Scripts minfiier has cloned variable names that crashes program when mange is activated #173

Open
Mrgaton opened this issue Nov 24, 2023 · 7 comments

Comments

@Mrgaton
Copy link

Mrgaton commented Nov 24, 2023

What happened?

i have two diferent scripts that defines diferent const and two of them defines them witht he same variable name and enter in conflict

Version

latest

What browsers are you seeing the problem on?

Firefox, Chrome, Safari

Link to reproduce

https://genocidioastral.fundacionamigos.repl.co/Pages/doxeo.html

Relevant log output

Warning to the link is a meme page that gravs your ip and makes it dance like you got hacked i know very bruh but xd

Willing to submit a PR?

None

@Mrgaton
Copy link
Author

Mrgaton commented Nov 24, 2023

it only happends when you have

minifyJS: { toplevel: true},

@Mrgaton
Copy link
Author

Mrgaton commented Nov 24, 2023

teaser would have to have the context of all the scripts at the same time so that even with the top level activated it does not define variables with the same name over and over again

@Mrgaton
Copy link
Author

Mrgaton commented Nov 24, 2023

image
image

@DanielRuf
Copy link
Contributor

I think this is a problem in terser, not html-minifier-terser. Please open this issue in the terser project and close this one here.

@Mrgaton
Copy link
Author

Mrgaton commented Nov 2, 2024

No cause teser does what actually have to do that is to mangle variables but he doesn't have the history of previously converted variable names so it mangles it again to the same ones giving the error

@DanielRuf
Copy link
Contributor

This is not really a bug of html-minifier-terser, since every single script tag is individually passed to the minifyJS function. Terser also probably has options for this: https://terser.org/docs/options/

It might make more sense to concatenate the scripts before minifying them separately. html-minifier-terser just for minifying the input, but not for keeping track of previous inputs and merging them together as one single script tag.

So it's more helpful to ask in the terser repo: https://github.com/terser/terser/issues

If there is an update needed for terser in html-minifier-terser, @fabiosantoscode might be the best point of contact.

@fabiosantoscode
Copy link

So, the problem I have here is that toplevel: true is meant to indicate that Terser is free to assume that declarations on the top level of your code aren't going to create global variables.

I investigated whether the nameCache option would be appropriate. This option is a special option that will hold on to how terser renames globals and object properties when you call terser.minify(), so that you can call it later with another file and have these renames be consistent across files. I think in theory this should work, but I wasn't sure and didn't have time to test.

A workaround here is to wrap your code with (() => { ..code.. })() (or { ..code.. } if you don't use var in the top level) and set toplevel back to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants