From 5838a527e4664577d5326f82ca194b124c20be67 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Fri, 23 Dec 2016 18:05:17 +0100 Subject: [PATCH] Fixes #847 - ignores selectors with invalid characters. Why: * Browsers won't apply such rules so neither should we. --- History.md | 1 + lib/optimizer/tidy-rules.js | 16 +++++----------- lib/tokenizer/tokenize.js | 6 ++++++ test/module-test.js | 3 ++- test/optimizer/basic-test.js | 12 ++++++++++++ test/tokenizer/tokenize-test.js | 17 +++++++++++++++++ 6 files changed, 43 insertions(+), 12 deletions(-) diff --git a/History.md b/History.md index 378b51e6..50747b1c 100644 --- a/History.md +++ b/History.md @@ -19,6 +19,7 @@ * Fixed issue [#839](https://github.com/jakubpawlowicz/clean-css/issues/839) - allows URIs in import inlining rules. * Fixed issue [#840](https://github.com/jakubpawlowicz/clean-css/issues/840) - allows input source map as map object. * Fixed issue [#843](https://github.com/jakubpawlowicz/clean-css/issues/843) - regression in selector handling. +* Fixed issue [#847](https://github.com/jakubpawlowicz/clean-css/issues/847) - regression in handling invalid selectors. [3.4.23 / 2016-12-17](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.22...v3.4.23) ================== diff --git a/lib/optimizer/tidy-rules.js b/lib/optimizer/tidy-rules.js index 296b757f..61231c36 100644 --- a/lib/optimizer/tidy-rules.js +++ b/lib/optimizer/tidy-rules.js @@ -4,16 +4,11 @@ var formatPosition = require('../utils/format-position'); var RELATION_PATTERN = /[>\+~]/; var WHITESPACE_PATTERN = /\s/; +var LESS_THAN = '<'; var STAR_PLUS_HTML_HACK = '*+html '; var STAR_FIRST_CHILD_PLUS_HTML_HACK = '*:first-child+html '; function hasInvalidCharacters(value) { - return value.indexOf(Marker.SINGLE_QUOTE) > -1 || value.indexOf(Marker.DOUBLE_QUOTE) > -1 ? - hasInvalidCharactersWithQuotes(value) : - hasInvalidCharactersWithoutQuotes(value); -} - -function hasInvalidCharactersWithQuotes(value) { var isEscaped; var isInvalid = false; var character; @@ -27,7 +22,10 @@ function hasInvalidCharactersWithQuotes(value) { // continue as always } else if (character == Marker.SINGLE_QUOTE || character == Marker.DOUBLE_QUOTE) { isQuote = !isQuote; - } else if (character == Marker.CLOSE_BRACE && !isQuote) { + } else if (!isQuote && (character == Marker.CLOSE_BRACE || character == Marker.EXCLAMATION || character == LESS_THAN)) { + isInvalid = true; + break; + } else if (!isQuote && i === 0 && RELATION_PATTERN.test(character)) { isInvalid = true; break; } @@ -38,10 +36,6 @@ function hasInvalidCharactersWithQuotes(value) { return isInvalid; } -function hasInvalidCharactersWithoutQuotes(value) { - return value.indexOf(Marker.CLOSE_BRACE) > -1; -} - function removeWhitespace(value, beautify) { var stripped = []; var character; diff --git a/lib/tokenizer/tokenize.js b/lib/tokenizer/tokenize.js index 6ecad7f5..3f509f6b 100644 --- a/lib/tokenizer/tokenize.js +++ b/lib/tokenizer/tokenize.js @@ -395,6 +395,12 @@ function intoTokens(source, externalContext, internalContext, isNested) { if (seekingValue && buffer.length > 0) { serializedBuffer = buffer.join('').replace(TAIL_BROKEN_VALUE_PATTERN, ''); propertyToken.push([Token.PROPERTY_VALUE, serializedBuffer, [originalMetadata(metadata, serializedBuffer, externalContext)]]); + + buffer = []; + } + + if (buffer.length > 0) { + externalContext.warnings.push('Invalid character(s) \'' + buffer.join('') + '\' at ' + formatPosition(metadata) + '. Ignoring.'); } return allTokens; diff --git a/test/module-test.js b/test/module-test.js index 4460c2b7..40baf705 100644 --- a/test/module-test.js +++ b/test/module-test.js @@ -111,8 +111,9 @@ vows.describe('module tests').addBatch({ assert.isEmpty(minified.errors); }, 'should raise one warning': function (error, minified) { - assert.lengthOf(minified.warnings, 1); + assert.lengthOf(minified.warnings, 2); assert.equal(minified.warnings[0], 'Unexpected \'}\' at 1:16.'); + assert.equal(minified.warnings[1], 'Invalid character(s) \'}\' at 1:16. Ignoring.'); } }, 'warnings on unexpected body': { diff --git a/test/optimizer/basic-test.js b/test/optimizer/basic-test.js index 4c9cd54d..40d9d01d 100644 --- a/test/optimizer/basic-test.js +++ b/test/optimizer/basic-test.js @@ -104,6 +104,18 @@ vows.describe('simple optimizations') 'no rule scope': [ '{overflow:hidden}', '' + ], + 'invalid characters #1': [ + '', + '' + ], + 'invalid characters #2': [ + '.funky{background:red}', + '' ] }, { advanced: false }) ) diff --git a/test/tokenizer/tokenize-test.js b/test/tokenizer/tokenize-test.js index bd92ab92..292a246d 100644 --- a/test/tokenizer/tokenize-test.js +++ b/test/tokenizer/tokenize-test.js @@ -3498,6 +3498,23 @@ vows.describe(tokenize) 'logs them correctly': function (warnings) { assert.deepEqual(warnings, ['Missing \'}\' at one.css:1:15.']); } + }, + 'warnings - extra characters': { + 'topic': function () { + var warnings = []; + + tokenize('', { + inputSourceMapTracker: inputSourceMapTracker(), + options: {}, + source: 'one.css', + warnings: warnings + }); + + return warnings; + }, + 'logs them correctly': function (warnings) { + assert.deepEqual(warnings, ['Invalid character(s) \']]>\' at one.css:1:28. Ignoring.']); + } } }) .addBatch({ -- 2.34.1