From ab050e7a9415b3d60d02a360becaa25b4a9ab2f1 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Wed, 25 Dec 2019 00:55:39 +0000 Subject: [PATCH] fix corner case in `directives` (#3645) --- lib/output.js | 80 +++++++++++++++-------------------- lib/parse.js | 7 +-- test/compress/directives.js | 38 +++++++++++++++++ test/mocha/directives.js | 82 +++++++++++++++++------------------- test/mocha/string-literal.js | 20 ++++----- 5 files changed, 124 insertions(+), 103 deletions(-) diff --git a/lib/output.js b/lib/output.js index d97778b2..cc39c2df 100644 --- a/lib/output.js +++ b/lib/output.js @@ -43,8 +43,6 @@ "use strict"; -var EXPECT_DIRECTIVE = /^$|[;{][\s\n]*$/; - function is_some_comments(comment) { // multiline comment return comment.type == "comment2" && /@preserve|@license|@cc_on/i.test(comment.value); @@ -378,7 +376,7 @@ function OutputStream(options) { }; function force_semicolon() { - might_need_semicolon = false; + if (might_need_semicolon) print(";"); print(";"); } @@ -585,17 +583,7 @@ function OutputStream(options) { force_semicolon : force_semicolon, to_utf8 : to_utf8, print_name : function(name) { print(make_name(name)) }, - print_string : function(str, quote, escape_directive) { - var encoded = encode_string(str, quote); - if (escape_directive === true && encoded.indexOf("\\") === -1) { - // Insert semicolons to break directive prologue - if (!EXPECT_DIRECTIVE.test(OUTPUT)) { - force_semicolon(); - } - force_semicolon(); - } - print(encoded); - }, + print_string : function(str, quote) { print(encode_string(str, quote)) }, next_indent : next_indent, with_indent : with_indent, with_block : with_block, @@ -633,17 +621,10 @@ function OutputStream(options) { nodetype.DEFMETHOD("_codegen", generator); } - var in_directive = false; - var active_scope = null; - var use_asm = null; + var use_asm = false; AST_Node.DEFMETHOD("print", function(stream, force_parens) { var self = this, generator = self._codegen; - if (self instanceof AST_Scope) { - active_scope = self; - } else if (!use_asm && self instanceof AST_Directive && self.value == "use asm") { - use_asm = active_scope; - } function doit() { stream.prepend_comments(self); self.add_source_map(stream); @@ -657,9 +638,6 @@ function OutputStream(options) { doit(); } stream.pop_node(); - if (self === use_asm) { - use_asm = null; - } }); AST_Node.DEFMETHOD("_print", AST_Node.prototype.print); @@ -828,7 +806,18 @@ function OutputStream(options) { /* -----[ PRINTERS ]----- */ DEFPRINT(AST_Directive, function(self, output) { - output.print_string(self.value, self.quote); + var quote = self.quote; + var value = self.value; + switch (output.option("quote_style")) { + case 0: + case 2: + if (value.indexOf('"') == -1) quote = '"'; + break; + case 1: + if (value.indexOf("'") == -1) quote = "'"; + break; + } + output.print(quote + value + quote); output.semicolon(); }); DEFPRINT(AST_Debugger, function(self, output) { @@ -840,30 +829,27 @@ function OutputStream(options) { function display_body(body, is_toplevel, output, allow_directives) { var last = body.length - 1; - in_directive = allow_directives; + var in_directive = allow_directives; + var was_asm = use_asm; body.forEach(function(stmt, i) { - if (in_directive === true && !(stmt instanceof AST_Directive || - stmt instanceof AST_EmptyStatement || - (stmt instanceof AST_SimpleStatement && stmt.body instanceof AST_String) - )) { - in_directive = false; - } - if (!(stmt instanceof AST_EmptyStatement)) { - output.indent(); - stmt.print(output); - if (!(i == last && is_toplevel)) { - output.newline(); - if (is_toplevel) output.newline(); + if (in_directive) { + if (stmt instanceof AST_Directive) { + if (stmt.value == "use asm") use_asm = true; + } else if (!(stmt instanceof AST_EmptyStatement)) { + if (stmt instanceof AST_SimpleStatement && stmt.body instanceof AST_String) { + output.force_semicolon(); + } + in_directive = false; } } - if (in_directive === true && - stmt instanceof AST_SimpleStatement && - stmt.body instanceof AST_String - ) { - in_directive = false; - } + if (stmt instanceof AST_EmptyStatement) return; + output.indent(); + stmt.print(output); + if (i == last && is_toplevel) return; + output.newline(); + if (is_toplevel) output.newline(); }); - in_directive = false; + use_asm = was_asm; } AST_StatementWithBody.DEFMETHOD("_do_print_body", function(output) { @@ -1348,7 +1334,7 @@ function OutputStream(options) { output.print(self.getValue()); }); DEFPRINT(AST_String, function(self, output) { - output.print_string(self.getValue(), self.quote, in_directive); + output.print_string(self.getValue(), self.quote); }); DEFPRINT(AST_Number, function(self, output) { if (use_asm && self.start && self.start.raw != null) { diff --git a/lib/parse.js b/lib/parse.js index c9ee01fd..740ef5f2 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -790,9 +790,10 @@ function parse($TEXT, options) { var dir = S.in_directives; var body = expression(true); if (dir) { - var token = body.start; - if (body instanceof AST_String && token.raw.indexOf("\\") == -1) { - S.input.add_directive(token.value); + if (body instanceof AST_String) { + var value = body.start.raw.slice(1, -1); + S.input.add_directive(value); + body.value = value; } else { S.in_directives = dir = false; } diff --git a/test/compress/directives.js b/test/compress/directives.js index 590d1623..69ecfdc4 100644 --- a/test/compress/directives.js +++ b/test/compress/directives.js @@ -93,3 +93,41 @@ issue_3166: { } } } + +valid_after_invalid_1: { + input: { + console.log(typeof function() { + "use\x20strict"; + "use strict"; + return this; + }()); + } + expect: { + console.log(typeof function() { + "use\x20strict"; + "use strict"; + return this; + }()); + } + expect_stdout: "undefined" +} + +valid_after_invalid_2: { + options = { + directives: true, + } + input: { + console.log(typeof function() { + "use\x20strict"; + "use strict"; + return this; + }()); + } + expect: { + console.log(typeof function() { + "use strict"; + return this; + }()); + } + expect_stdout: "undefined" +} diff --git a/test/mocha/directives.js b/test/mocha/directives.js index e872747a..524fbc48 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -69,13 +69,13 @@ describe("Directives", function() { ], [ '"use \\\nstrict";"use strict";', - [], - [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + [ "use strict" ], + [ "use\nstrict", "use \nstrict", "use asm" ] ], [ '"\\76";', - [], - [ ">", "\\76" ] + [ "\\76" ], + [ ">" ] ], [ // no ; or newline @@ -106,13 +106,13 @@ describe("Directives", function() { ], [ 'function foo() {"use \\\nstrict";"use strict";', - [], - [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + [ "use strict" ], + [ "use\nstrict", "use \nstrict", "use asm" ] ], [ 'var foo = function() {"\\76";', - [], - [ ">", "\\76" ] + [ "\\76" ], + [ ">" ] ], [ 'var foo = function() {"use strict"', // no ; or newline @@ -156,21 +156,24 @@ describe("Directives", function() { }); }); }); - it("Should test EXPECT_DIRECTIVE RegExp", function() { + it("Should print semicolon to separate strings from directives", function() { [ - [ "", true ], - [ "'test';", true ], - [ "'test';;", true ], - [ "'tests';\n", true ], - [ "'tests'", false ], - [ "'tests'; \n\t", true ], - [ "'tests';\n\n", true ], - [ "\n\n\"use strict\";\n\n", true ], + [ "", ';"";' ], + [ '"test";', '"test";;"";' ], + [ '"test";;', '"test";;"";' ], + [ '"tests";\n', '"tests";;"";' ], + [ '"tests"', '"tests";;"";' ], + [ '"tests"; \n\t', '"tests";;"";' ], + [ '"tests";\n\n', '"tests";;"";' ], + [ '\n\n"use strict";\n\n', '"use strict";;"";' ], ].forEach(function(test) { + var ast = UglifyJS.parse(test[0]); + ast.body.push(new UglifyJS.AST_SimpleStatement({ + body: new UglifyJS.AST_String({ value: "" }) + })); var out = UglifyJS.OutputStream(); - out.print(test[0]); - out.print_string("", null, true); - assert.strictEqual(out.get() === test[0] + ';""', test[1], test[0]); + ast.print(out); + assert.strictEqual(out.get(), test[1], test[0]); }); }); it("Should only print 2 semicolons spread over 2 lines in beautify mode", function() { @@ -178,8 +181,8 @@ describe("Directives", function() { '"use strict";', "'use strict';", '"use strict";', - '"use strict";;', - "'use strict';", + '"use strict";', + ";'use strict';", "console.log('use strict');" ].join(""), { compress: false, @@ -201,19 +204,23 @@ describe("Directives", function() { it("Should not add double semicolons in non-scoped block statements to avoid strings becoming directives", function() { [ [ - '{"use\x20strict"}', + '"use strict";"use\\x20strict";', + '"use strict";"use\\x20strict";' + ], + [ + '{"use\\x20strict"}', '{"use strict"}' ], [ - 'function foo(){"use\x20strict";}', // Valid place for directives - 'function foo(){"use strict"}' + 'function foo(){"use\\x20strict";}', // Valid place for directives + 'function foo(){"use\\x20strict"}' ], [ - 'try{"use\x20strict"}catch(e){}finally{"use\x20strict"}', + 'try{"use\\x20strict"}catch(e){}finally{"use\\x20strict"}', 'try{"use strict"}catch(e){}finally{"use strict"}' ], [ - 'if(1){"use\x20strict"} else {"use strict"}', + 'if(1){"use\\x20strict"} else {"use strict"}', 'if(1){"use strict"}else{"use strict"}' ] ].forEach(function(test) { @@ -225,16 +232,6 @@ describe("Directives", function() { assert.strictEqual(result.code, test[1], test[0]); }); }); - it("Should add double semicolon when relying on automatic semicolon insertion", function() { - var result = UglifyJS.minify('"use strict";"use\\x20strict";', { - compress: false, - output: { - semicolons: false - } - }); - if (result.error) throw result.error; - assert.strictEqual(result.code, '"use strict";;"use strict"\n'); - }); it("Should check quote style of directives", function() { [ // 0. Prefer double quotes, unless string contains more double quotes than single quotes @@ -249,9 +246,9 @@ describe("Directives", function() { '"use strict";' ], [ - '"\\\'use strict\\\'";', // Not a directive as it contains quotes + '"\\\'use strict\\\'";', 0, - ';"\'use strict\'";', + '"\\\'use strict\\\'";', ], [ "'\"use strict\"';", @@ -273,7 +270,7 @@ describe("Directives", function() { '"\'use strict\'";', 1, // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway - "'\\'use strict\\'';", + '"\'use strict\'";', ], [ "'\\'use strict\\'';", // Not a valid directive @@ -305,7 +302,7 @@ describe("Directives", function() { "'\"use strict\"';", 2, // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway - '"\\\"use strict\\\"";', + "'\"use strict\"';", ], [ '"\\"use strict\\"";', // Not a valid directive @@ -353,8 +350,7 @@ describe("Directives", function() { [ // Nothing gets optimised in the compressor because "use asm" is the first statement '"use asm";"use\\x20strict";1+1;', - // Yet, the parser noticed that "use strict" wasn't a directive - '"use asm";;"use strict";1+1;', + '"use asm";"use\\x20strict";1+1;' ], [ 'function f(){ "use strict" }', diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js index aca915b7..7b62c90f 100644 --- a/test/mocha/string-literal.js +++ b/test/mocha/string-literal.js @@ -59,13 +59,13 @@ describe("String literals", function() { it("Should not throw error outside strict mode if string contains escaped octalIntegerLiteral", function() { var tests = [ - ['"\\76";', ';">";'], - ['"\\0"', '"\\0";'], - ['"\\08"', '"\\x008";'], - ['"\\008"', '"\\x008";'], - ['"\\0008"', '"\\x008";'], - ['"use strict" === "use strict";\n"\\76";', '"use strict"==="use strict";">";'], - ['"use\\\n strict";\n"\\07";', ';"use strict";"\07";'] + [ ';"\\76";', ';">";' ], + [ ';"\\0";', ';"\\0";' ], + [ ';"\\08"', ';"\\x008";' ], + [ ';"\\008"', ';"\\x008";' ], + [ ';"\\0008"', ';"\\x008";' ], + [ ';"use\\\n strict";\n"\\07";', ';"use strict";"\07";' ], + [ '"use strict" === "use strict";\n"\\76";', '"use strict"==="use strict";">";' ], ]; for (var test in tests) { @@ -75,8 +75,8 @@ describe("String literals", function() { }); it("Should not throw error when digit is 8 or 9", function() { - assert.equal(UglifyJS.parse('"use strict";"\\08"').print_to_string(), '"use strict";"\\x008";'); - assert.equal(UglifyJS.parse('"use strict";"\\09"').print_to_string(), '"use strict";"\\x009";'); + assert.equal(UglifyJS.parse('"use strict";;"\\08"').print_to_string(), '"use strict";;"\\x008";'); + assert.equal(UglifyJS.parse('"use strict";;"\\09"').print_to_string(), '"use strict";;"\\x009";'); }); it("Should not unescape unpaired surrogates", function() { @@ -93,7 +93,7 @@ describe("String literals", function() { for (; i <= 0xFFFF; i++) { code.push("\\u" + i.toString(16)); } - code = '"' + code.join() + '"'; + code = ';"' + code.join() + '"'; var normal = UglifyJS.minify(code, { compress: false, mangle: false, -- 2.34.1