Skip to content

Commit

Permalink
fix mangle name collision across files (#2722)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexlamsl
alexlamsl committed Jan 4, 2018
1 parent 484e484 commit afbcebd
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 33 deletions.
2 changes: 0 additions & 2 deletions lib/minify.js
View file
Expand Up @@ -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)) {
Expand All @@ -39,7 +38,6 @@ function init_cache(cache) {

function to_json(cache) {
return {
cname: cache.cname,
props: cache.props.toObject()
};
}
Expand Down
25 changes: 14 additions & 11 deletions lib/propmangle.js
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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_.
Expand All @@ -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;
}
Expand Down
34 changes: 22 additions & 12 deletions lib/scope.js
View file
Expand Up @@ -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){
Expand Down Expand Up @@ -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
Expand All @@ -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){
Expand All @@ -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;
}
Expand Down Expand Up @@ -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){
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions test/input/issue-1242/qux.js
View file
@@ -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);
53 changes: 48 additions & 5 deletions test/mocha/minify.js
View file
Expand Up @@ -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));
});

Expand All @@ -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)");
Expand Down

0 comments on commit afbcebd

Please sign in to comment.