xml2js incorrectly uglified

[kzc]

Bug report or feature request?

bug

ES5 or ES6+ input?

ES5

Uglify version (uglifyjs -V)

uglify-js@3.3.7

JavaScript input

From: angular/angular-cli#9206

$ npm install xml2js@0.4.19 browserify@15.1.0 uglify-js@3.3.7

$ cat test.js
var Parser = require('xml2js').Parser;
var parser =  new Parser();
var xml = "<root>a &amp; b</root>"
parser.parseString(xml, function (err, result) {
    console.dir(result);
});

Expected:

$ node_modules/.bin/browserify test.js | node
{ root: 'a & b' }

Actual:

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -bmc | node
undefined

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -bm | node
{ root: 'a & b' }

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -bc | node
{ root: 'a & b' }

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -bmc --no-rename | node
{ root: 'a & b' }

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -bmc reduce_vars=false | node
{ root: 'a & b' }

Last working version was v3.3.4. Issue introduced in commit 9b58b54.

[bayy]

Thank you @kzc for filing this bug!

Please please have this fixed at your earliest convenience. We have a customer-facing product depending on this.

[kzc]

Workaround:

$ node_modules/.bin/browserify test.js | node_modules/.bin/uglifyjs -mc inline=false | node
{ root: 'a & b' }

[bayy]

@kzc What changes should I make in my angular project to use this workaround? Thx!

Edit: One way to use the workaround to stick to version "uglify-es": "3.3.4" with npm-shrinkwrap.json as kzc suggested below.

[kzc]

@bayy Either set the uglify minify option compress: { inline: false } or have yarn or npm lock in uglify-es@3.3.4. Ask the angular project how to configure this.

[alexlamsl]

The test case isn't trivial to narrow down, so I'll leave it for others to work on a fix for this.

[kzc]

@alexlamsl Single file test case: t2783.js

This file is the same as the browserify output in the top post, except that it was run through minify() with rename: true.

$ cat t2783.js | node
{ root: 'a & b' }

Notice that -c inline=2 passes:

$ cat t2783.js | bin/uglifyjs -c inline=2 -b bracketize > 2.js

$ node 2.js
{ root: 'a & b' }

and -c inline=3 fails:

$ cat t2783.js | bin/uglifyjs -c inline=3 -b bracketize > 3.js

$ node 3.js
undefined

Diff of 2.js (pass) and 3.js (fail):

$ diff -u 2.js 3.js
--- 2.js	2018-01-14 20:14:54.000000000 -0500
+++ 3.js	2018-01-14 20:15:05.000000000 -0500
@@ -1930,17 +1930,13 @@
             }, bs.prototype.pipe = function() {
                 this.emit("error", new Error("Cannot pipe, not readable"));
             }, bs.prototype.write = function(Ws, Xs, Ys) {
-                var zs, ys, Zs = this._writableState, $s = !1, _s = (zs = Ws, (Vr.isBuffer(zs) || zs instanceof Wr) && !Zs.objectMode);
+                var zs, ys, Qs, Rs, Ss, Ts, Us, Vs, Ns, Os, Ps, Zs = this._writableState, $s = !1, _s = (zs = Ws, 
+                (Vr.isBuffer(zs) || zs instanceof Wr) && !Zs.objectMode);
                 return _s && !Vr.isBuffer(Ws) && (ys = Ws, Ws = Vr.from(ys)), "function" == typeof Xs && (Ys = Xs, 
                 Xs = null), _s ? Xs = "buffer" : Xs || (Xs = Zs.defaultEncoding), "function" != typeof Ys && (Ys = $r), 
-                Zs.ended ? function(Ns, Os) {
-                    var Ps = new Error("write after end");
-                    Ns.emit("error", Ps), Nr(Os, Ps);
-                }(this, Ys) : (_s || function(Qs, Rs, Ss, Ts) {
-                    var Us = !0, Vs = !1;
-                    return null === Ss ? Vs = new TypeError("May not write null values to stream") : "string" == typeof Ss || void 0 === Ss || Rs.objectMode || (Vs = new TypeError("Invalid non-string/buffer chunk")), 
-                    Vs && (Qs.emit("error", Vs), Nr(Ts, Vs), Us = !1), Us;
-                }(this, Zs, Ws, Ys)) && (Zs.pendingcb++, $s = function(ht, it, jt, kt, lt, mt) {
+                Zs.ended ? (Ns = this, Os = Ys, Ps = new Error("write after end"), Ns.emit("error", Ps), 
+                Nr(Os, Ps)) : (_s || (Qs = this, Rs = Zs, Ts = Ys, Us = !0, Vs = !1, null === (Ss = Ws) ? Vs = new TypeError("May not write null values to stream") : "string" == typeof Ss || void 0 === Ss || Rs.objectMode || (Vs = new TypeError("Invalid non-string/buffer chunk")), 
+                Vs && (Qs.emit("error", Vs), Nr(Ts, Vs), Us = !1), Us)) && (Zs.pendingcb++, $s = function(ht, it, jt, kt, lt, mt) {
                     if (!jt) {
                         var nt = function(et, ft, gt) {
                             et.objectMode || !1 === et.decodeStrings || "string" != typeof ft || (ft = Vr.from(ft, gt));
@@ -2563,7 +2559,7 @@
                 function Bw(Jx, Kx) {
                     return Jx.test(Kx);
                 }
-                var Dw = 0;
+                var jz, kz, lz, Dw = 0;
                 fw.STATE = {
                     BEGIN: Dw++,
                     BEGIN_WHITESPACE: Dw++,
@@ -3010,7 +3006,7 @@
                     Hy.attribList.length = 0, Hy.state = Dw.TEXT;
                 }
                 function Rw(Ty) {
-                    var Wy, Uy = Ty.entity, Vy = Uy.toLowerCase(), Xy = "";
+                    var Wy, Uy = (Ty = cz).entity, Vy = Uy.toLowerCase(), Xy = "";
                     return Ty.ENTITIES[Uy] ? Ty.ENTITIES[Uy] : Ty.ENTITIES[Vy] ? Ty.ENTITIES[Vy] : ("#" === (Uy = Vy).charAt(0) && ("x" === Uy.charAt(1) ? (Uy = Uy.slice(2), 
                     Xy = (Wy = parseInt(Uy, 16)).toString(16)) : (Uy = Uy.slice(1), Xy = (Wy = parseInt(Uy, 10)).toString(10))), 
                     Uy = Uy.replace(/^0+/, ""), isNaN(Wy) || Xy.toLowerCase() !== Uy ? (Lw(Ty, "Invalid character entity"), 
@@ -3024,29 +3020,27 @@
                     var az = "";
                     return _y < $y.length && (az = $y.charAt(_y)), az;
                 }
-                Dw = fw.STATE, String.fromCodePoint || function() {
-                    var jz = String.fromCharCode, kz = Math.floor, lz = function() {
-                        var oz, pz, nz = [], qz = -1, rz = arguments.length;
-                        if (!rz) {
-                            return "";
-                        }
-                        for (var sz = ""; ++qz < rz; ) {
-                            var tz = Number(arguments[qz]);
-                            if (!isFinite(tz) || tz < 0 || tz > 1114111 || kz(tz) !== tz) {
-                                throw RangeError("Invalid code point: " + tz);
-                            }
-                            tz <= 65535 ? nz.push(tz) : (oz = 55296 + ((tz -= 65536) >> 10), pz = tz % 1024 + 56320, 
-                            nz.push(oz, pz)), (qz + 1 === rz || nz.length > 16384) && (sz += jz.apply(null, nz), 
-                            nz.length = 0);
-                        }
-                        return sz;
-                    };
-                    Object.defineProperty ? Object.defineProperty(String, "fromCodePoint", {
-                        value: lz,
-                        configurable: !0,
-                        writable: !0
-                    }) : String.fromCodePoint = lz;
-                }();
+                Dw = fw.STATE, String.fromCodePoint || (jz = String.fromCharCode, kz = Math.floor, 
+                lz = function() {
+                    var oz, pz, nz = [], qz = -1, rz = arguments.length;
+                    if (!rz) {
+                        return "";
+                    }
+                    for (var sz = ""; ++qz < rz; ) {
+                        var tz = Number(arguments[qz]);
+                        if (!isFinite(tz) || tz < 0 || tz > 1114111 || kz(tz) !== tz) {
+                            throw RangeError("Invalid code point: " + tz);
+                        }
+                        tz <= 65535 ? nz.push(tz) : (oz = 55296 + ((tz -= 65536) >> 10), pz = tz % 1024 + 56320, 
+                        nz.push(oz, pz)), (qz + 1 === rz || nz.length > 16384) && (sz += jz.apply(null, nz), 
+                        nz.length = 0);
+                    }
+                    return sz;
+                }, Object.defineProperty ? Object.defineProperty(String, "fromCodePoint", {
+                    value: lz,
+                    configurable: !0,
+                    writable: !0
+                }) : String.fromCodePoint = lz);
             }(void 0 === dw ? this.sax = {} : dw);
         }).call(this, bw("buffer").Buffer);
     }, {

There is a function Uw in t2783.js and 3.js which contains this expression:

                                cz[iz] += Rw(cz)

The single-use function is Rw is not inlined into function Uw (which itself was inlined) in 3.js, but function Rw was altered as seen in this fragment of the diff above:

                 function Rw(Ty) {
-                    var Wy, Uy = Ty.entity, Vy = Uy.toLowerCase(), Xy = "";
+                    var Wy, Uy = (Ty = cz).entity, Vy = Uy.toLowerCase(), Xy = "";

If (Ty = cz) is changed to Ty in the file 3.js then it runs correctly.

[alexlamsl]

@kzc thanks for the effort - fix coming right up 👍