From afbcebddf63c7ffa5b0df9b3712ee3b560918f1e Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Fri, 5 Jan 2018 05:08:09 +0800 Subject: [PATCH] fix `mangle` name collision across files (#2722) --- lib/minify.js | 2 -- lib/propmangle.js | 25 +++++++++-------- lib/scope.js | 34 +++++++++++++++-------- test/input/issue-1242/qux.js | 6 ++-- test/mocha/minify.js | 53 ++++++++++++++++++++++++++++++++---- 5 files changed, 87 insertions(+), 33 deletions(-) diff --git a/lib/minify.js b/lib/minify.js index 806c3aeb..a68cbf3a 100644 --- a/lib/minify.js +++ b/lib/minify.js @@ -29,7 +29,6 @@ function set_shorthand(name, options, keys) { function init_cache(cache) { if (!cache) return; - if (!("cname" in cache)) cache.cname = -1; if (!("props" in cache)) { cache.props = new Dictionary(); } else if (!(cache.props instanceof Dictionary)) { @@ -39,7 +38,6 @@ function init_cache(cache) { function to_json(cache) { return { - cname: cache.cname, props: cache.props.toObject() }; } diff --git a/lib/propmangle.js b/lib/propmangle.js index c2f27c42..ffc8faf8 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -110,12 +110,15 @@ function mangle_properties(ast, options) { if (!Array.isArray(reserved)) reserved = []; if (!options.builtins) find_builtins(reserved); - var cache = options.cache; - if (cache == null) { - cache = { - cname: -1, - props: new Dictionary() - }; + var cname = -1; + var cache; + if (options.cache) { + cache = options.cache.props; + cache.each(function(mangled_name) { + push_uniq(reserved, mangled_name); + }); + } else { + cache = new Dictionary(); } var regex = options.regex; @@ -172,7 +175,7 @@ function mangle_properties(ast, options) { if (unmangleable.indexOf(name) >= 0) return false; if (reserved.indexOf(name) >= 0) return false; if (options.only_cache) { - return cache.props.has(name); + return cache.has(name); } if (/^-?[0-9]+(\.[0-9]+)?(e[+-][0-9]+)?$/.test(name)) return false; return true; @@ -181,7 +184,7 @@ function mangle_properties(ast, options) { function should_mangle(name) { if (regex && !regex.test(name)) return false; if (reserved.indexOf(name) >= 0) return false; - return cache.props.has(name) + return cache.has(name) || names_to_mangle.indexOf(name) >= 0; } @@ -199,7 +202,7 @@ function mangle_properties(ast, options) { return name; } - var mangled = cache.props.get(name); + var mangled = cache.get(name); if (!mangled) { if (debug) { // debug mode: use a prefix and suffix to preserve readability, e.g. o.foo -> o._$foo$NNN_. @@ -213,11 +216,11 @@ function mangle_properties(ast, options) { // either debug mode is off, or it is on and we could not use the mangled name if (!mangled) { do { - mangled = base54(++cache.cname); + mangled = base54(++cname); } while (!can_mangle(mangled)); } - cache.props.set(name, mangled); + cache.set(name, mangled); } return mangled; } diff --git a/lib/scope.js b/lib/scope.js index de92fc94..801c888c 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -242,10 +242,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ } })); } - - if (options.cache) { - this.cname = options.cache.cname; - } }); AST_Toplevel.DEFMETHOD("def_global", function(node){ @@ -329,10 +325,10 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol, init){ return symbol.thedef = def; }); -AST_Scope.DEFMETHOD("next_mangled", function(options){ - var ext = this.enclosed; +function next_mangled(scope, options) { + var ext = scope.enclosed; out: while (true) { - var m = base54(++this.cname); + var m = base54(++scope.cname); if (!is_identifier(m)) continue; // skip over "do" // https://github.com/mishoo/UglifyJS2/issues/242 -- do not @@ -349,6 +345,18 @@ AST_Scope.DEFMETHOD("next_mangled", function(options){ } return m; } +} + +AST_Scope.DEFMETHOD("next_mangled", function(options){ + return next_mangled(this, options); +}); + +AST_Toplevel.DEFMETHOD("next_mangled", function(options){ + var name; + do { + name = next_mangled(this, options); + } while (member(name, this.mangled_names)); + return name; }); AST_Function.DEFMETHOD("next_mangled", function(options, def){ @@ -362,7 +370,7 @@ AST_Function.DEFMETHOD("next_mangled", function(options, def){ var tricky_name = tricky_def ? tricky_def.mangled_name || tricky_def.name : null; while (true) { - var name = AST_Lambda.prototype.next_mangled.call(this, options, def); + var name = next_mangled(this, options); if (!tricky_name || tricky_name != name) return name; } @@ -413,8 +421,14 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options){ var lname = -1; var to_mangle = []; + var mangled_names = this.mangled_names = []; if (options.cache) { this.globals.each(collect); + if (options.cache.props) { + options.cache.props.each(function(mangled_name) { + push_uniq(mangled_names, mangled_name); + }); + } } var tw = new TreeWalker(function(node, descend){ @@ -443,10 +457,6 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options){ this.walk(tw); to_mangle.forEach(function(def){ def.mangle(options) }); - if (options.cache) { - options.cache.cname = this.cname; - } - function collect(symbol) { if (!member(symbol.name, options.reserved)) { to_mangle.push(symbol); diff --git a/test/input/issue-1242/qux.js b/test/input/issue-1242/qux.js index 94171f38..4a72ffc2 100644 --- a/test/input/issue-1242/qux.js +++ b/test/input/issue-1242/qux.js @@ -1,4 +1,4 @@ -var a = bar(1+2); -var b = baz(3+9); -print('q' + 'u' + 'x', a, b); +var x = bar(1+2); +var y = baz(3+9); +print('q' + 'u' + 'x', x, y); foo(5+6); diff --git a/test/mocha/minify.js b/test/mocha/minify.js index 5fa9254b..a304c5b5 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -42,8 +42,15 @@ describe("minify", function() { original += code; compressed += result.code; }); - assert.strictEqual(JSON.stringify(cache).slice(0, 20), '{"cname":5,"props":{'); - assert.strictEqual(compressed, 'function n(n){return 3*n}function r(n){return n/2}var c=console.log.bind(console);function l(o){c("Foo:",2*o)}var f=n(3),i=r(12);c("qux",f,i),l(11);'); + assert.strictEqual(JSON.stringify(cache).slice(0, 10), '{"props":{'); + assert.strictEqual(compressed, [ + "function n(n){return 3*n}", + "function r(n){return n/2}", + "var o=console.log.bind(console);", + 'function c(n){o("Foo:",2*n)}', + "var a=n(3),b=r(12);", + 'o("qux",a,b),c(11);', + ].join("")); assert.strictEqual(run_code(compressed), run_code(original)); }); @@ -68,12 +75,48 @@ describe("minify", function() { original += code; compressed += result.code; }); - assert.strictEqual(JSON.stringify(cache).slice(0, 28), '{"vars":{"cname":5,"props":{'); - assert.strictEqual(compressed, 'function n(n){return 3*n}function r(n){return n/2}var c=console.log.bind(console);function l(o){c("Foo:",2*o)}var f=n(3),i=r(12);c("qux",f,i),l(11);'); + assert.strictEqual(JSON.stringify(cache).slice(0, 18), '{"vars":{"props":{'); + assert.strictEqual(compressed, [ + "function n(n){return 3*n}", + "function r(n){return n/2}", + "var o=console.log.bind(console);", + 'function c(n){o("Foo:",2*n)}', + "var a=n(3),b=r(12);", + 'o("qux",a,b),c(11);', + ].join("")); assert.strictEqual(run_code(compressed), run_code(original)); }); - it("should not parse invalid use of reserved words", function() { + it("Should avoid mangled names in cache", function() { + var cache = {}; + var original = ""; + var compressed = ""; + [ + '"xxxyy";var i={s:1};', + '"xxyyy";var j={t:2,u:3},k=4;', + 'console.log(i.s,j.t,j.u,k);', + ].forEach(function(code) { + var result = Uglify.minify(code, { + compress: false, + mangle: { + properties: true, + toplevel: true + }, + nameCache: cache + }); + if (result.error) throw result.error; + original += code; + compressed += result.code; + }); + assert.strictEqual(compressed, [ + '"xxxyy";var x={x:1};', + '"xxyyy";var y={y:2,a:3},a=4;', + 'console.log(x.x,y.y,y.a,a);', + ].join("")); + assert.strictEqual(run_code(compressed), run_code(original)); + }); + + it("Should not parse invalid use of reserved words", function() { assert.strictEqual(Uglify.minify("function enum(){}").error, undefined); assert.strictEqual(Uglify.minify("function static(){}").error, undefined); assert.strictEqual(Uglify.minify("function this(){}").error.message, "Unexpected token: name (this)"); -- 2.34.1