From abb6aa544f3252e12092c10edacae1c054441a50 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 13:39:33 +0100 Subject: [PATCH 01/12] Add 'debug' option to mangle_properties Add a 'debug' option to mangle_properties and add a test that verifies it works as expected both with and without a cache passed as well. --- lib/propmangle.js | 23 ++++++++++++++++++++- test/mocha/mangle-debug.js | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/mocha/mangle-debug.js diff --git a/lib/propmangle.js b/lib/propmangle.js index 4187db12..cf436c47 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -83,13 +83,20 @@ function mangle_properties(ast, options) { cache : null, only_cache : false, regex : null, - ignore_quoted : false + ignore_quoted : false, + debug : false }); var reserved = options.reserved; if (reserved == null) reserved = find_builtins(); + // To properly simulate different mangling per different invocations, default a random number + // to be appended to each mangled name for this invocation. If a cache is passed, use an empty + // string to correctly simulate using consistent names when caches are passed around. + // (Of course this assumes the same cache is being passed - it's good enough for a debug check though.) + var debugCacheName = Math.floor(Math.random() * 1000).toString(); + var cache = options.cache; if (cache == null) { cache = { @@ -97,9 +104,14 @@ function mangle_properties(ast, options) { props: new Dictionary() }; } + else + { + debugCacheName = ""; + } var regex = options.regex; var ignore_quoted = options.ignore_quoted; + var debug = options.debug; var names_to_mangle = []; var unmangleable = []; @@ -199,6 +211,15 @@ function mangle_properties(ast, options) { if (!should_mangle(name)) { return name; } + + // If debug mode is enabled, return a predictably-mangled name with a prefix and suffix, + // so the name is different but code can still be read for diagnostic purposes. + // Also add the debug cache name which is empty if a (assumedly) consistent cache is + // passed around, otherwise is a random number to ensure separate invocations mangle + // differently as they would in practice. + // e.g. "foo" -> "_$foo$123$_" (or "_$foo$_" with consistent cache) + if (debug) + return "_$" + name + (debugCacheName ? "$" + debugCacheName : "") + "$_"; var mangled = cache.props.get(name); if (!mangled) { diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js new file mode 100644 index 00000000..146449b0 --- /dev/null +++ b/test/mocha/mangle-debug.js @@ -0,0 +1,42 @@ +var Uglify = require('../../'); +var assert = require("assert"); + +describe("mangle_properties 'debug' option ", function() { + it("Should test the 'debug' option of mangle_properties works with no cache passed", function() { + var js = 'var o = {}; o.foo = "bar";'; + + var ast = Uglify.parse(js); + ast.figure_out_scope(); + ast = Uglify.mangle_properties(ast, { + debug: true + }); + + let stream = Uglify.OutputStream(); + ast.print(stream); + var result = stream.toString(); + + // Should match: var o={};o._$foo$NNN$="bar"; + // where NNN is a number. + assert(/var o=\{\};o\._\$foo\$\d+\$_="bar";/.test(result)); + }); + + it("Should test the 'debug' option of mangle_properties works with a cache passed", function() { + var js = 'var o = {}; o.foo = "bar";'; + + var ast = Uglify.parse(js); + ast.figure_out_scope(); + ast = Uglify.mangle_properties(ast, { + cache: { + cname: -1, + props: new Uglify.Dictionary() + }, + debug: true + }); + + let stream = Uglify.OutputStream(); + ast.print(stream); + var result = stream.toString(); + + assert.strictEqual(result, 'var o={};o._$foo$_="bar";'); + }); +}); From 52cd38d268741e2447da4d072122f3692b443ee8 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 13:44:16 +0100 Subject: [PATCH 02/12] Remove use of 'let' 'let' appeared to break one of the builds so changed it to 'var'. --- test/mocha/mangle-debug.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js index 146449b0..a6377514 100644 --- a/test/mocha/mangle-debug.js +++ b/test/mocha/mangle-debug.js @@ -11,7 +11,7 @@ describe("mangle_properties 'debug' option ", function() { debug: true }); - let stream = Uglify.OutputStream(); + var stream = Uglify.OutputStream(); ast.print(stream); var result = stream.toString(); @@ -33,7 +33,7 @@ describe("mangle_properties 'debug' option ", function() { debug: true }); - let stream = Uglify.OutputStream(); + var stream = Uglify.OutputStream(); ast.print(stream); var result = stream.toString(); From 57c9b97ec4392c841c5c8b7948aafe6cb33cca55 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 13:48:36 +0100 Subject: [PATCH 03/12] Replace tabs with spaces Use consistent whitespace with the rest of the project. --- test/mocha/mangle-debug.js | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js index a6377514..6b1c8405 100644 --- a/test/mocha/mangle-debug.js +++ b/test/mocha/mangle-debug.js @@ -4,39 +4,39 @@ var assert = require("assert"); describe("mangle_properties 'debug' option ", function() { it("Should test the 'debug' option of mangle_properties works with no cache passed", function() { var js = 'var o = {}; o.foo = "bar";'; - - var ast = Uglify.parse(js); - ast.figure_out_scope(); - ast = Uglify.mangle_properties(ast, { - debug: true - }); - - var stream = Uglify.OutputStream(); - ast.print(stream); - var result = stream.toString(); - - // Should match: var o={};o._$foo$NNN$="bar"; - // where NNN is a number. + + var ast = Uglify.parse(js); + ast.figure_out_scope(); + ast = Uglify.mangle_properties(ast, { + debug: true + }); + + let stream = Uglify.OutputStream(); + ast.print(stream); + var result = stream.toString(); + + // Should match: var o={};o._$foo$NNN$="bar"; + // where NNN is a number. assert(/var o=\{\};o\._\$foo\$\d+\$_="bar";/.test(result)); }); - + it("Should test the 'debug' option of mangle_properties works with a cache passed", function() { var js = 'var o = {}; o.foo = "bar";'; - - var ast = Uglify.parse(js); - ast.figure_out_scope(); - ast = Uglify.mangle_properties(ast, { - cache: { - cname: -1, - props: new Uglify.Dictionary() - }, - debug: true - }); - - var stream = Uglify.OutputStream(); - ast.print(stream); - var result = stream.toString(); - - assert.strictEqual(result, 'var o={};o._$foo$_="bar";'); + + var ast = Uglify.parse(js); + ast.figure_out_scope(); + ast = Uglify.mangle_properties(ast, { + cache: { + cname: -1, + props: new Uglify.Dictionary() + }, + debug: true + }); + + let stream = Uglify.OutputStream(); + ast.print(stream); + var result = stream.toString(); + + assert.strictEqual(result, 'var o={};o._$foo$_="bar";'); }); }); From 252a48f04f688da994170f80832c68d95142c50a Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 13:52:16 +0100 Subject: [PATCH 04/12] Replace tabs with spaces + replace let with var Because I haven't made enough of a mess already, the last commit switched back to let... so this one uses var and has spaces :P --- test/mocha/mangle-debug.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js index 6b1c8405..45e8b3cb 100644 --- a/test/mocha/mangle-debug.js +++ b/test/mocha/mangle-debug.js @@ -11,7 +11,7 @@ describe("mangle_properties 'debug' option ", function() { debug: true }); - let stream = Uglify.OutputStream(); + var stream = Uglify.OutputStream(); ast.print(stream); var result = stream.toString(); @@ -33,7 +33,7 @@ describe("mangle_properties 'debug' option ", function() { debug: true }); - let stream = Uglify.OutputStream(); + var stream = Uglify.OutputStream(); ast.print(stream); var result = stream.toString(); From 2dbf8bfaadaee2fc7f5de917c038990c4e165b99 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 18:02:13 +0100 Subject: [PATCH 05/12] CLI, readme and code style updates Add support for --debug-mangling CLI option, describe it in the readme, and update code changes to use consistent style. --- README.md | 18 ++++++++++++++++++ bin/test.js | 5 +++++ bin/uglifyjs | 5 ++++- lib/propmangle.js | 11 +++++------ 4 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 bin/test.js diff --git a/README.md b/README.md index 4f5b21a0..0ab92bfc 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,23 @@ of mangled property names. Using the name cache is not necessary if you compress all your files in a single call to UglifyJS. +#### Debugging property name mangling + +You can also pass `--debug-mangling` in order to mangle property names +without completely obscuring them. For example the property `o.foo` +would mangle to `o._$foo$_` with this option. This allows property mangling +of a large codebase while still being able to debug the code and identify +where mangling is breaking things. + +Note that by default a random number is added to the name per invocation, +e.g. `o._foo$123$_` where `123` is a random number. This is used to ensure +that mangling is still non-deterministic and that independently mangled +scripts are still broken if they access the same properties. If you provide a +name cache (either with `--name-cache` or the `cache` option in the API) then +the random number is removed, producing just `o._$foo$_`. This ensures that +if you correctly share a cache when mangling separate scripts, then they will +work together. + ## Compressor options You need to pass `--compress` (`-c`) to enable the compressor. Optionally @@ -743,6 +760,7 @@ Other options: - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mangle-regex` CLI arguments option) - `ignore_quoted` – Only mangle unquoted property names (maps to the `--mangle-props 2` CLI arguments option) + - `debug` – Mangle names with the original name still present (maps to the `--debug-mangling` CLI arguments option) We could add more options to `UglifyJS.minify` — if you need additional functionality please suggest! diff --git a/bin/test.js b/bin/test.js new file mode 100644 index 00000000..ea864bc0 --- /dev/null +++ b/bin/test.js @@ -0,0 +1,5 @@ +"use strict"; + +let o = {}; +o.foo = "bar"; +console.log(o.foo); \ No newline at end of file diff --git a/bin/uglifyjs b/bin/uglifyjs index 3f0c8254..eb864b6d 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -76,6 +76,7 @@ You need to pass an argument to this option to specify the name that your module .describe("name-cache", "File to hold mangled names mappings") .describe("pure-funcs", "List of functions that can be safely removed if their return value is not used") .describe("dump-spidermonkey-ast", "Dump SpiderMonkey AST to stdout.") + .describe("debug-mangling", "Use debug mode for diagnosing property mangling errors.") .alias("p", "prefix") .alias("o", "output") @@ -130,6 +131,7 @@ You need to pass an argument to this option to specify the name that your module .boolean("bare-returns") .boolean("keep-fnames") .boolean("reserve-domprops") + .boolean("debug-mangling") .wrap(80) @@ -425,7 +427,8 @@ async.eachLimit(files, 1, function (file, cb) { cache : cache, only_cache : !ARGS.mangle_props, regex : regex, - ignore_quoted : ARGS.mangle_props == 2 + ignore_quoted : ARGS.mangle_props == 2, + debug : ARGS.debug_mangling }); writeNameCache("props", cache); })(); diff --git a/lib/propmangle.js b/lib/propmangle.js index cf436c47..d18d5a4c 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -95,7 +95,7 @@ function mangle_properties(ast, options) { // to be appended to each mangled name for this invocation. If a cache is passed, use an empty // string to correctly simulate using consistent names when caches are passed around. // (Of course this assumes the same cache is being passed - it's good enough for a debug check though.) - var debugCacheName = Math.floor(Math.random() * 1000).toString(); + var debug_cache_name = Math.floor(Math.random() * 1000).toString(); var cache = options.cache; if (cache == null) { @@ -104,9 +104,8 @@ function mangle_properties(ast, options) { props: new Dictionary() }; } - else - { - debugCacheName = ""; + else { + debug_cache_name = ""; } var regex = options.regex; @@ -218,8 +217,8 @@ function mangle_properties(ast, options) { // passed around, otherwise is a random number to ensure separate invocations mangle // differently as they would in practice. // e.g. "foo" -> "_$foo$123$_" (or "_$foo$_" with consistent cache) - if (debug) - return "_$" + name + (debugCacheName ? "$" + debugCacheName : "") + "$_"; + if (debug && can_mangle(name)) + return "_$" + name + (debug_cache_name ? "$" + debug_cache_name : "") + "$_"; var mangled = cache.props.get(name); if (!mangled) { From 31763799ae4be93a046672b24dca94fb0b159b56 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Wed, 5 Oct 2016 18:03:15 +0100 Subject: [PATCH 06/12] Remove temp file Oops --- bin/test.js | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 bin/test.js diff --git a/bin/test.js b/bin/test.js deleted file mode 100644 index ea864bc0..00000000 --- a/bin/test.js +++ /dev/null @@ -1,5 +0,0 @@ -"use strict"; - -let o = {}; -o.foo = "bar"; -console.log(o.foo); \ No newline at end of file From b28d2d3cf2da8975c2247c25fe7c099e9201b20a Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Thu, 6 Oct 2016 11:33:42 +0100 Subject: [PATCH 07/12] Refactoring considering feedback - the CLI option is now --mangle-debug - the implementation is now a bit more elegant, always using a random number and relying on the existing cache mechanism - takes in to account can_mangle - updated readme & test --- README.md | 20 +++++++++---------- bin/uglifyjs | 6 +++--- lib/propmangle.js | 39 ++++++++++++++++++-------------------- test/mocha/mangle-debug.js | 26 +++---------------------- 4 files changed, 34 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 0ab92bfc..cd9a99db 100644 --- a/README.md +++ b/README.md @@ -286,20 +286,20 @@ single call to UglifyJS. #### Debugging property name mangling -You can also pass `--debug-mangling` in order to mangle property names +You can also pass `--mangle-debug` in order to mangle property names without completely obscuring them. For example the property `o.foo` -would mangle to `o._$foo$_` with this option. This allows property mangling +would mangle to `o._$foo$123_` with this option. This allows property mangling of a large codebase while still being able to debug the code and identify where mangling is breaking things. Note that by default a random number is added to the name per invocation, -e.g. `o._foo$123$_` where `123` is a random number. This is used to ensure -that mangling is still non-deterministic and that independently mangled -scripts are still broken if they access the same properties. If you provide a -name cache (either with `--name-cache` or the `cache` option in the API) then -the random number is removed, producing just `o._$foo$_`. This ensures that -if you correctly share a cache when mangling separate scripts, then they will -work together. +e.g. the `123` in `_$foo$123_` is random. This is because normal property +mangling is non-deterministic. It makes sure that if you mangle two scripts +separately without sharing a cache, they will be broken. It also makes sure +that if you accidentally attempt to use a mangled key name in storage, then +the storage reads will be broken between builds. This allows you to fix these +issues with readable keys, and then turning off this option should continue to +work with unreadable names. ## Compressor options @@ -760,7 +760,7 @@ Other options: - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mangle-regex` CLI arguments option) - `ignore_quoted` – Only mangle unquoted property names (maps to the `--mangle-props 2` CLI arguments option) - - `debug` – Mangle names with the original name still present (maps to the `--debug-mangling` CLI arguments option) + - `debug` – Mangle names with the original name still present (maps to the `--mangle-debug` CLI arguments option) We could add more options to `UglifyJS.minify` — if you need additional functionality please suggest! diff --git a/bin/uglifyjs b/bin/uglifyjs index eb864b6d..cf99faed 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -76,7 +76,7 @@ You need to pass an argument to this option to specify the name that your module .describe("name-cache", "File to hold mangled names mappings") .describe("pure-funcs", "List of functions that can be safely removed if their return value is not used") .describe("dump-spidermonkey-ast", "Dump SpiderMonkey AST to stdout.") - .describe("debug-mangling", "Use debug mode for diagnosing property mangling errors.") + .describe("mangle-debug", "Use debug mode for diagnosing property mangling errors.") .alias("p", "prefix") .alias("o", "output") @@ -131,7 +131,7 @@ You need to pass an argument to this option to specify the name that your module .boolean("bare-returns") .boolean("keep-fnames") .boolean("reserve-domprops") - .boolean("debug-mangling") + .boolean("mangle-debug") .wrap(80) @@ -428,7 +428,7 @@ async.eachLimit(files, 1, function (file, cb) { only_cache : !ARGS.mangle_props, regex : regex, ignore_quoted : ARGS.mangle_props == 2, - debug : ARGS.debug_mangling + debug : ARGS.mangle_debug }); writeNameCache("props", cache); })(); diff --git a/lib/propmangle.js b/lib/propmangle.js index d18d5a4c..eac51b44 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -90,12 +90,6 @@ function mangle_properties(ast, options) { var reserved = options.reserved; if (reserved == null) reserved = find_builtins(); - - // To properly simulate different mangling per different invocations, default a random number - // to be appended to each mangled name for this invocation. If a cache is passed, use an empty - // string to correctly simulate using consistent names when caches are passed around. - // (Of course this assumes the same cache is being passed - it's good enough for a debug check though.) - var debug_cache_name = Math.floor(Math.random() * 1000).toString(); var cache = options.cache; if (cache == null) { @@ -104,13 +98,13 @@ function mangle_properties(ast, options) { props: new Dictionary() }; } - else { - debug_cache_name = ""; - } var regex = options.regex; var ignore_quoted = options.ignore_quoted; var debug = options.debug; + + // Simulate non-deterministic mangling in debug mode by adding a random number to the name + var debug_cache_name = Math.floor(Math.random() * 1000).toString(); var names_to_mangle = []; var unmangleable = []; @@ -210,21 +204,24 @@ function mangle_properties(ast, options) { if (!should_mangle(name)) { return name; } - - // If debug mode is enabled, return a predictably-mangled name with a prefix and suffix, - // so the name is different but code can still be read for diagnostic purposes. - // Also add the debug cache name which is empty if a (assumedly) consistent cache is - // passed around, otherwise is a random number to ensure separate invocations mangle - // differently as they would in practice. - // e.g. "foo" -> "_$foo$123$_" (or "_$foo$_" with consistent cache) - if (debug && can_mangle(name)) - return "_$" + name + (debug_cache_name ? "$" + debug_cache_name : "") + "$_"; var mangled = cache.props.get(name); if (!mangled) { - do { - mangled = base54(++cache.cname); - } while (!can_mangle(mangled)); + if (debug) { + // debug mode: use a prefix and suffix to preserve readability, e.g. o.foo -> o._$foo$NNN_. + var debug_mangled = "_$" + name + "$" + debug_cache_name + "_"; + + if (can_mangle(debug_mangled)) + mangled = debug_mangled; + else + mangled = name; + } + else { + do { + mangled = base54(++cache.cname); + } while (!can_mangle(mangled)); + } + cache.props.set(name, mangled); } return mangled; diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js index 45e8b3cb..122245a7 100644 --- a/test/mocha/mangle-debug.js +++ b/test/mocha/mangle-debug.js @@ -2,7 +2,7 @@ var Uglify = require('../../'); var assert = require("assert"); describe("mangle_properties 'debug' option ", function() { - it("Should test the 'debug' option of mangle_properties works with no cache passed", function() { + it("Should test the 'debug' option of mangle_properties works as expected", function() { var js = 'var o = {}; o.foo = "bar";'; var ast = Uglify.parse(js); @@ -15,28 +15,8 @@ describe("mangle_properties 'debug' option ", function() { ast.print(stream); var result = stream.toString(); - // Should match: var o={};o._$foo$NNN$="bar"; + // Should match: var o={};o._$foo$NNN="bar"; // where NNN is a number. - assert(/var o=\{\};o\._\$foo\$\d+\$_="bar";/.test(result)); - }); - - it("Should test the 'debug' option of mangle_properties works with a cache passed", function() { - var js = 'var o = {}; o.foo = "bar";'; - - var ast = Uglify.parse(js); - ast.figure_out_scope(); - ast = Uglify.mangle_properties(ast, { - cache: { - cname: -1, - props: new Uglify.Dictionary() - }, - debug: true - }); - - var stream = Uglify.OutputStream(); - ast.print(stream); - var result = stream.toString(); - - assert.strictEqual(result, 'var o={};o._$foo$_="bar";'); + assert(/var o=\{\};o\._\$foo\$\d+_="bar";/.test(result)); }); }); From 7de8ffe77b6e5ffeb29f6aea8b19f339766b8782 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Thu, 6 Oct 2016 16:42:23 +0100 Subject: [PATCH 08/12] Make debug option take a string suffix Debug option (and CLI) now take a string to enable, and the string is used as the (possibly empty) suffix. Instead of generating a random number, the caller can do that if they wish. Updated tests accordingly (no need for regex now). --- README.md | 18 ++++----- bin/uglifyjs | 6 +-- lib/propmangle.js | 17 ++++---- test/compress/properties.js | 78 +++++++++++++++++++++++++++++++++++++ test/mocha/mangle-debug.js | 22 ----------- 5 files changed, 99 insertions(+), 42 deletions(-) delete mode 100644 test/mocha/mangle-debug.js diff --git a/README.md b/README.md index cd9a99db..4fc9083b 100644 --- a/README.md +++ b/README.md @@ -288,18 +288,16 @@ single call to UglifyJS. You can also pass `--mangle-debug` in order to mangle property names without completely obscuring them. For example the property `o.foo` -would mangle to `o._$foo$123_` with this option. This allows property mangling +would mangle to `o._$foo$_` with this option. This allows property mangling of a large codebase while still being able to debug the code and identify where mangling is breaking things. -Note that by default a random number is added to the name per invocation, -e.g. the `123` in `_$foo$123_` is random. This is because normal property -mangling is non-deterministic. It makes sure that if you mangle two scripts -separately without sharing a cache, they will be broken. It also makes sure -that if you accidentally attempt to use a mangled key name in storage, then -the storage reads will be broken between builds. This allows you to fix these -issues with readable keys, and then turning off this option should continue to -work with unreadable names. +You can also pass a custom suffix using `--mangle-debug=XYZ`. This would then +mangle `o.foo` to `o._$foo$XYZ_`. You can change this each time you compile a +script to identify how a property got mangled. One technique is to pass a +random number on every compile to simulate mangling changing with different +inputs (e.g. as you update the input script with new properties), and to help +identify mistakes like writing mangled keys to storage. ## Compressor options @@ -760,7 +758,7 @@ Other options: - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mangle-regex` CLI arguments option) - `ignore_quoted` – Only mangle unquoted property names (maps to the `--mangle-props 2` CLI arguments option) - - `debug` – Mangle names with the original name still present (maps to the `--mangle-debug` CLI arguments option) + - `debug` – Mangle names with the original name still present (maps to the `--mangle-debug` CLI arguments option). Defaults to `false`. Pass an empty string to enable, or a non-empty string to set the suffix. We could add more options to `UglifyJS.minify` — if you need additional functionality please suggest! diff --git a/bin/uglifyjs b/bin/uglifyjs index cf99faed..733c9a91 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -71,12 +71,12 @@ You need to pass an argument to this option to specify the name that your module .describe("quotes", "Quote style (0 - auto, 1 - single, 2 - double, 3 - original)") .describe("reserved-file", "File containing reserved names") .describe("reserve-domprops", "Make (most?) DOM properties reserved for --mangle-props") + .describe("mangle-debug", "Use debug mode for diagnosing property mangling errors. Optionally provide a mangled name suffix.") .describe("mangle-props", "Mangle property names (0 - disabled, 1 - mangle all properties, 2 - mangle unquoted properies)") .describe("mangle-regex", "Only mangle property names matching the regex") .describe("name-cache", "File to hold mangled names mappings") .describe("pure-funcs", "List of functions that can be safely removed if their return value is not used") .describe("dump-spidermonkey-ast", "Dump SpiderMonkey AST to stdout.") - .describe("mangle-debug", "Use debug mode for diagnosing property mangling errors.") .alias("p", "prefix") .alias("o", "output") @@ -97,6 +97,7 @@ You need to pass an argument to this option to specify the name that your module .string("beautify") .string("m") .string("mangle") + .string("mangle-debug") .string("c") .string("compress") .string("d") @@ -131,7 +132,6 @@ You need to pass an argument to this option to specify the name that your module .boolean("bare-returns") .boolean("keep-fnames") .boolean("reserve-domprops") - .boolean("mangle-debug") .wrap(80) @@ -428,7 +428,7 @@ async.eachLimit(files, 1, function (file, cb) { only_cache : !ARGS.mangle_props, regex : regex, ignore_quoted : ARGS.mangle_props == 2, - debug : ARGS.mangle_debug + debug : typeof ARGS.mangle_debug === "undefined" ? false : ARGS.mangle_debug }); writeNameCache("props", cache); })(); diff --git a/lib/propmangle.js b/lib/propmangle.js index eac51b44..4d29b3d0 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -90,7 +90,7 @@ function mangle_properties(ast, options) { var reserved = options.reserved; if (reserved == null) reserved = find_builtins(); - + var cache = options.cache; if (cache == null) { cache = { @@ -101,10 +101,13 @@ function mangle_properties(ast, options) { var regex = options.regex; var ignore_quoted = options.ignore_quoted; - var debug = options.debug; - // Simulate non-deterministic mangling in debug mode by adding a random number to the name - var debug_cache_name = Math.floor(Math.random() * 1000).toString(); + // note debug is either false (disabled), or a string of the debug cache name to use (enabled). + // note debug may be enabled as an empty string, which is falsey. + var debug = (options.debug !== false); + var debug_cache_name; + if (debug) + debug_cache_name = options.debug; var names_to_mangle = []; var unmangleable = []; @@ -213,10 +216,10 @@ function mangle_properties(ast, options) { if (can_mangle(debug_mangled)) mangled = debug_mangled; - else - mangled = name; } - else { + + // either debug mode is off, or it is on and can_mangle returned false + if (!mangled) { do { mangled = base54(++cache.cname); } while (!can_mangle(mangled)); diff --git a/test/compress/properties.js b/test/compress/properties.js index 7ef6a014..554dd4d2 100644 --- a/test/compress/properties.js +++ b/test/compress/properties.js @@ -143,6 +143,84 @@ mangle_unquoted_properties: { } } +mangle_debug: { + mangle_props = { + debug: "" + }; + input: { + a.foo = "bar"; + x = { baz: "ban" }; + } + expect: { + a._$foo$_ = "bar"; + x = { _$baz$_: "ban" }; + } +} + +mangle_debug_suffix: { + mangle_props = { + debug: "XYZ" + }; + input: { + a.foo = "bar"; + x = { baz: "ban" }; + } + expect: { + a._$foo$XYZ_ = "bar"; + x = { _$baz$XYZ_: "ban" }; + } +} + +mangle_debug_suffix_ignore_quoted: { + options = { + properties: false + } + mangle_props = { + ignore_quoted: true, + debug: "XYZ", + reserved: [] + } + beautify = { + beautify: false, + quote_style: 3, + keep_quoted_props: true, + } + input: { + a.top = 1; + function f1() { + a["foo"] = "bar"; + a.color = "red"; + a.stuff = 2; + x = {"bar": 10, size: 7}; + a.size = 9; + } + function f2() { + a.foo = "bar"; + a['color'] = "red"; + x = {bar: 10, size: 7}; + a.size = 9; + a.stuff = 3; + } + } + expect: { + a._$top$XYZ_ = 1; + function f1() { + a["foo"] = "bar"; + a.color = "red"; + a._$stuff$XYZ_ = 2; + x = {"bar": 10, _$size$XYZ_: 7}; + a._$size$XYZ_ = 9; + } + function f2() { + a.foo = "bar"; + a['color'] = "red"; + x = {bar: 10, _$size$XYZ_: 7}; + a._$size$XYZ_ = 9; + a._$stuff$XYZ_ = 3; + } + } +} + first_256_chars_as_properties: { beautify = { ascii_only: true, diff --git a/test/mocha/mangle-debug.js b/test/mocha/mangle-debug.js deleted file mode 100644 index 122245a7..00000000 --- a/test/mocha/mangle-debug.js +++ /dev/null @@ -1,22 +0,0 @@ -var Uglify = require('../../'); -var assert = require("assert"); - -describe("mangle_properties 'debug' option ", function() { - it("Should test the 'debug' option of mangle_properties works as expected", function() { - var js = 'var o = {}; o.foo = "bar";'; - - var ast = Uglify.parse(js); - ast.figure_out_scope(); - ast = Uglify.mangle_properties(ast, { - debug: true - }); - - var stream = Uglify.OutputStream(); - ast.print(stream); - var result = stream.toString(); - - // Should match: var o={};o._$foo$NNN="bar"; - // where NNN is a number. - assert(/var o=\{\};o\._\$foo\$\d+_="bar";/.test(result)); - }); -}); From 10cee1144baaafabd0f49a5789d03f790bd5fe07 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Thu, 6 Oct 2016 17:41:11 +0100 Subject: [PATCH 09/12] Fix issue #1321 Fixes name collision issue, which is not directly related to this patch but has an additional case in debug mode. Added regression test. --- lib/propmangle.js | 9 ++++++--- test/compress/issue-1321.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 test/compress/issue-1321.js diff --git a/lib/propmangle.js b/lib/propmangle.js index 4d29b3d0..78bb9a63 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -214,15 +214,18 @@ function mangle_properties(ast, options) { // debug mode: use a prefix and suffix to preserve readability, e.g. o.foo -> o._$foo$NNN_. var debug_mangled = "_$" + name + "$" + debug_cache_name + "_"; - if (can_mangle(debug_mangled)) + if (can_mangle(debug_mangled) && !(ignore_quoted && debug_mangled in ignored)) mangled = debug_mangled; } - // either debug mode is off, or it is on and can_mangle returned false + // either debug mode is off, or it is on and we could not use the mangled name if (!mangled) { + // note can_mangle() does not check if the name collides with the 'ignored' set + // (filled with quoted properties when ignore_quoted set). Make sure we add this + // check so we don't collide with a quoted name. do { mangled = base54(++cache.cname); - } while (!can_mangle(mangled)); + } while (!can_mangle(mangled) || (ignore_quoted && mangled in ignored)); } cache.props.set(name, mangled); diff --git a/test/compress/issue-1321.js b/test/compress/issue-1321.js new file mode 100644 index 00000000..8642cf38 --- /dev/null +++ b/test/compress/issue-1321.js @@ -0,0 +1,36 @@ +issue_1321_no_debug: { + mangle_props = { + ignore_quoted: true + } + input: { + var x = {}; + x.foo = 1; + x["a"] = 2 * x.foo; + console.log(x.foo, x["a"]); + } + expect: { + var x = {}; + x.b = 1; + x["a"] = 2 * x.b; + console.log(x.b, x["a"]); + } +} + +issue_1321_debug: { + mangle_props = { + ignore_quoted: true, + debug: "" + } + input: { + var x = {}; + x.foo = 1; + x["_$foo$_"] = 2 * x.foo; + console.log(x.foo, x["_$foo$_"]); + } + expect: { + var x = {}; + x.a = 1; + x["_$foo$_"] = 2 * x.a; + console.log(x.a, x["_$foo$_"]); + } +} From 3f3ab65e8a20ce1e81e278bad6984c74d343a59e Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Thu, 6 Oct 2016 18:09:23 +0100 Subject: [PATCH 10/12] Add extra test case for issue #1321 Verifies behavior with `ignore_quoted: false`. --- test/compress/issue-1321.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/compress/issue-1321.js b/test/compress/issue-1321.js index 8642cf38..2b6f17bf 100644 --- a/test/compress/issue-1321.js +++ b/test/compress/issue-1321.js @@ -34,3 +34,21 @@ issue_1321_debug: { console.log(x.a, x["_$foo$_"]); } } + +issue_1321_with_quoted: { + mangle_props = { + ignore_quoted: false + } + input: { + var x = {}; + x.foo = 1; + x["a"] = 2 * x.foo; + console.log(x.foo, x["a"]); + } + expect: { + var x = {}; + x.a = 1; + x["b"] = 2 * x.a; + console.log(x.a, x["b"]); + } +} From 65922fcbb260cc725e841cd69374f80aa7f95739 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Fri, 7 Oct 2016 12:38:48 +0100 Subject: [PATCH 11/12] Address style nits Also rename `debug_cache_name` to the more accurate `debug_name_suffix`. --- lib/propmangle.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/propmangle.js b/lib/propmangle.js index 78bb9a63..0a84b83b 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -102,12 +102,10 @@ function mangle_properties(ast, options) { var regex = options.regex; var ignore_quoted = options.ignore_quoted; - // note debug is either false (disabled), or a string of the debug cache name to use (enabled). + // note debug is either false (disabled), or a string of the debug suffix to use (enabled). // note debug may be enabled as an empty string, which is falsey. var debug = (options.debug !== false); - var debug_cache_name; - if (debug) - debug_cache_name = options.debug; + var debug_name_suffix = debug ? options.debug : null; var names_to_mangle = []; var unmangleable = []; @@ -212,10 +210,11 @@ function mangle_properties(ast, options) { if (!mangled) { if (debug) { // debug mode: use a prefix and suffix to preserve readability, e.g. o.foo -> o._$foo$NNN_. - var debug_mangled = "_$" + name + "$" + debug_cache_name + "_"; + var debug_mangled = "_$" + name + "$" + debug_name_suffix + "_"; - if (can_mangle(debug_mangled) && !(ignore_quoted && debug_mangled in ignored)) + if (can_mangle(debug_mangled) && !(ignore_quoted && debug_mangled in ignored)) { mangled = debug_mangled; + } } // either debug mode is off, or it is on and we could not use the mangled name From 1e19cd08fac29adb6907b853dd84b2d28bf5ee99 Mon Sep 17 00:00:00 2001 From: "Ashley (Scirra)" Date: Fri, 7 Oct 2016 16:11:58 +0100 Subject: [PATCH 12/12] Treat debug: true debug: "" Supports passing `true` for `debug` in `mangle_properties`. Added test case to verify this behavior. --- lib/propmangle.js | 8 ++++++-- test/compress/properties.js | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/propmangle.js b/lib/propmangle.js index 0a84b83b..50967887 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -103,9 +103,13 @@ function mangle_properties(ast, options) { var ignore_quoted = options.ignore_quoted; // note debug is either false (disabled), or a string of the debug suffix to use (enabled). - // note debug may be enabled as an empty string, which is falsey. + // note debug may be enabled as an empty string, which is falsey. Also treat passing 'true' + // the same as passing an empty string. var debug = (options.debug !== false); - var debug_name_suffix = debug ? options.debug : null; + var debug_name_suffix; + if (debug) { + debug_name_suffix = (options.debug === true ? "" : options.debug); + } var names_to_mangle = []; var unmangleable = []; diff --git a/test/compress/properties.js b/test/compress/properties.js index 554dd4d2..5598b45d 100644 --- a/test/compress/properties.js +++ b/test/compress/properties.js @@ -157,6 +157,20 @@ mangle_debug: { } } +mangle_debug_true: { + mangle_props = { + debug: true + }; + input: { + a.foo = "bar"; + x = { baz: "ban" }; + } + expect: { + a._$foo$_ = "bar"; + x = { _$baz$_: "ban" }; + } +} + mangle_debug_suffix: { mangle_props = { debug: "XYZ"