UglifyJS icon indicating copy to clipboard operation
UglifyJS copied to clipboard

[ES6] Bug: block eliminated that protects constant globals

Open pvdz opened this issue 7 years ago • 6 comments

  • Bug report or feature request? Bug: by fuzzer

  • uglify-js version (uglifyjs -V) uglify-js 2.8.15 (git master)

var undefined = 0;
{
  function undefined() {
  }

  undefined();
}
console.log('PASS')
$ node s.js
PASS
$ bin/uglifyjs s.js -c
function undefined(){}var undefined=0;undefined(),console.log("PASS");
$ bin/uglifyjs s.js -c | node
[stdin]:1
function undefined(){}var undefined=0;undefined(),console.log("PASS");
^

TypeError: Identifier 'undefined' has already been declared
    at [stdin]:1:1
...

(Changed desc) In nodejs it's okay to do var undefined in global space, but not for function names. It's fine to do that in a block, still in global space, for some reason. Probably only affects undefined, NaN, and Infinity...

pvdz avatar Mar 24 '17 23:03 pvdz

I wouldn't concentrate on undefined in this issue. It doesn't work with regular variables and functions:

$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | bin/uglifyjs -c | node
[stdin]:1
function a(){}var a=0;a(),console.log("PASS");
                      ^

TypeError: a is not a function

As there's no such thing as block scope in ES5 I can't fault the code uglify produces in this case. Its transform looks correct for ES5.

Node 6 is applying ES6 block scoping rules to this code:

$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | node690
PASS

Notice that Node 4 does what's expected for ES5 which does not respect block scope:

$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | node421
[stdin]:1
var a=0; { function a(){} a(); } console.log("PASS");
                          ^

TypeError: a is not a function

As does Node 0.12.x:

$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | node0_12_9 
[stdin]:1
var a=0; { function a(){} a(); } console.log("PASS");
                          ^
TypeError: number is not a function

kzc avatar Mar 25 '17 06:03 kzc

This error supports the analysis above:

$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | node0_12_9 --use_strict
[stdin]:1
var a=0; { function a(){} a(); } console.log("PASS");
           ^^^^^^^^
SyntaxError: In strict mode code, functions can only be declared at top level or immediately within another function.
$ echo 'var a=0; { function a(){} a(); } console.log("PASS");' | node0_10_41 --use_strict

[stdin]:1
var a=0; { function a(){} a(); } console.log("PASS");
           ^^^^^^^^
SyntaxError: In strict mode code, functions can only be declared at top level or immediately within another function.

This code is not valid ES5.

kzc avatar Mar 25 '17 06:03 kzc

So do you think I should tag this as [ES6] bug and leave it open?

alexlamsl avatar Mar 25 '17 06:03 alexlamsl

Yeah, it seems to be a harmony issue.

kzc avatar Mar 25 '17 13:03 kzc

#2023 will retain the block, but then collapse_vars would go in substitute (undefined=0)() which is correct on ES5 but not ES6.

alexlamsl avatar May 30 '17 05:05 alexlamsl

It looks like current Uglify-es doesn't remove the block anymore:

fabio@fabio-thinkpad ♥  echo "var undefined = 0;                                     
{                                                                                    
  function undefined() {                                                             
  }                                                                                  
                                                                                     
  undefined();                                                                       
}                                                                                    
console.log('PASS')" | bin/uglifyjs -mc                                              
var undefined;{function undefined(){}(undefined=0)()}console.log("PASS");            

Should this be closed?

fabiosantoscode avatar Mar 15 '18 19:03 fabiosantoscode

Patch released in [email protected]

alexlamsl avatar Jun 10 '24 14:06 alexlamsl