From d0348d2a2da06ccd3292776ee94dcbf7042bbf3e Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Thu, 20 Nov 2014 23:14:00 +0000 Subject: [PATCH] Fixes #385 - edge cases in processing cut off CSS content. --- History.md | 5 +++ lib/clean.js | 4 +-- lib/selectors/tokenizer.js | 6 +++- lib/text/comments-processor.js | 9 +++-- lib/text/urls-processor.js | 15 +++++++- test/integration-test.js | 17 ++++++++++ test/module-test.js | 51 ++++++++++++++++++++++++++++ test/text/comments-processor-test.js | 2 +- 8 files changed, 101 insertions(+), 8 deletions(-) diff --git a/History.md b/History.md index 0f58f012..602991d2 100644 --- a/History.md +++ b/History.md @@ -16,6 +16,11 @@ * Fixed issue [#360](https://github.com/GoalSmashers/clean-css/issues/360) - adds 7 extra CSS colors. * Fixed issue [#363](https://github.com/GoalSmashers/clean-css/issues/363) - `rem` units overriding `px`. +[2.2.19 / 2014-11-20](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.18...v2.2.19) +================== + +* Fixed issue [#385](https://github.com/GoalSmashers/clean-css/issues/385) - edge cases in processing cut off data. + [2.2.18 / 2014-11-17](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.17...v2.2.18) ================== diff --git a/lib/clean.js b/lib/clean.js index 0d1dbb3d..4c6d9c6d 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -112,10 +112,10 @@ function minify(data) { var options = this.options; var context = this.context; - var commentsProcessor = new CommentsProcessor(options.keepSpecialComments, options.keepBreaks); + var commentsProcessor = new CommentsProcessor(context, options.keepSpecialComments, options.keepBreaks); var expressionsProcessor = new ExpressionsProcessor(); var freeTextProcessor = new FreeTextProcessor(); - var urlsProcessor = new UrlsProcessor(); + var urlsProcessor = new UrlsProcessor(context); var urlRebase = new UrlRebase(options, context); var selectorsOptimizer = new SelectorsOptimizer(options, context); diff --git a/lib/selectors/tokenizer.js b/lib/selectors/tokenizer.js index 7b6e9a9c..d5923d5f 100644 --- a/lib/selectors/tokenizer.js +++ b/lib/selectors/tokenizer.js @@ -140,7 +140,11 @@ function tokenize(context) { var firstOpenBraceAt = chunk.indexOf('{', nextSpecial); var firstSemicolonAt = chunk.indexOf(';', nextSpecial); var isSingle = firstSemicolonAt > -1 && (firstOpenBraceAt == -1 || firstSemicolonAt < firstOpenBraceAt); - if (isSingle) { + var isBroken = firstOpenBraceAt == -1 && firstSemicolonAt == -1; + if (isBroken) { + context.outer.warnings.push('Broken declaration: \'' + chunk.substring(context.cursor) + '\'.'); + context.cursor = chunk.length; + } else if (isSingle) { nextEnd = chunk.indexOf(';', nextSpecial + 1); var single = extractBlock(chunk.substring(context.cursor, nextEnd + 1)); diff --git a/lib/text/comments-processor.js b/lib/text/comments-processor.js index 748bd072..cb69d899 100644 --- a/lib/text/comments-processor.js +++ b/lib/text/comments-processor.js @@ -7,8 +7,9 @@ var COMMENT_SUFFIX = '*/'; var lineBreak = require('os').EOL; -var CommentsProcessor = function CommentsProcessor(keepSpecialComments, keepBreaks) { +var CommentsProcessor = function CommentsProcessor(context, keepSpecialComments, keepBreaks) { this.comments = new EscapeStore('COMMENT'); + this.context = context; this.keepAll = keepSpecialComments == '*'; this.keepOne = keepSpecialComments == '1' || keepSpecialComments === 1; this.keepBreaks = keepBreaks; @@ -49,8 +50,10 @@ CommentsProcessor.prototype.escape = function (data) { } nextEnd = data.indexOf(COMMENT_SUFFIX, nextStart + COMMENT_PREFIX.length); - if (nextEnd == -1) - break; + if (nextEnd == -1) { + this.context.warnings.push('Broken comment: \'' + data.substring(nextStart) + '\'.'); + nextEnd = data.length - 2; + } tempData.push(data.substring(cursor, nextStart)); diff --git a/lib/text/urls-processor.js b/lib/text/urls-processor.js index 24be9774..8554dc77 100644 --- a/lib/text/urls-processor.js +++ b/lib/text/urls-processor.js @@ -1,7 +1,8 @@ var EscapeStore = require('./escape-store'); -var UrlsProcessor = function UrlsProcessor() { +var UrlsProcessor = function UrlsProcessor(context) { this.urls = new EscapeStore('URL'); + this.context = context; }; // Strip urls by replacing them by a special @@ -19,6 +20,18 @@ UrlsProcessor.prototype.escape = function (data) { break; nextEnd = data.indexOf(')', nextStart); + // Following lines are a safety mechanism to ensure + // incorrectly terminated urls are processed correctly. + if (nextEnd == -1) { + nextEnd = data.indexOf('}', nextStart); + + if (nextEnd == -1) + nextEnd = data.length; + else + nextEnd--; + + this.context.warnings.push('Broken URL declaration: \'' + data.substring(nextStart, nextEnd + 1) + '\'.'); + } var url = data.substring(nextStart, nextEnd + 1); var placeholder = this.urls.store(url); diff --git a/test/integration-test.js b/test/integration-test.js index 4f267d83..cf9dcace 100644 --- a/test/integration-test.js +++ b/test/integration-test.js @@ -885,6 +885,15 @@ vows.describe('integration tests').addBatch({ '.icon-logo{background-image:url("data:image/x-icon;base64,AAABAAEAEBA")}', '.icon-logo{background-image:url(data:image/x-icon;base64,AAABAAEAEBA)}' ], + 'cut off url content on selector level': 'a{background:url(image/}', + 'cut off url content on block level': [ + '@font-face{src:url(data:application/x-font-woff;base64,d09GRk9UVE8AAENAAA0AAAAA}', + '@font-face{src:url(data:application/x-font-woff;base64,d09GRk9UVE8AAENAAA0AAAAA}' + ], + 'cut off url content on top level': [ + '@font-face{src:url(data:application/x-font-woff;base64,d09GRk9UVE8AAENAAA0AAAAA', + '@font-face{src:url(data:application/x-font-woff;base64,d09GRk9UVE8AAENAAA0AAAAA}' + ], 'strip single parentheses': [ "a{background:url('/images/blank.png')}", "a{background:url(/images/blank.png)}" @@ -1349,6 +1358,14 @@ title']{display:block}", "@import url(test/data/partials/five.css);", ".five{background:url(data:image/jpeg;base64,/9j/)}" ], + 'cut off': [ + '@impo', + '' + ], + 'cut off inside a comment': [ + '/* @impo', + '' + ], 'inside a comment': [ '/* @import url(test/data/partials/five.css); */a { color: red; }', 'a{color:red}' diff --git a/test/module-test.js b/test/module-test.js index 85fd24a2..a83ac7a0 100644 --- a/test/module-test.js +++ b/test/module-test.js @@ -154,6 +154,57 @@ vows.describe('module tests').addBatch({ assert.equal(minifier.warnings[0], 'Empty property \'color\' inside \'a\' selector. Ignoring.'); } }, + 'warnings on broken urls': { + topic: function () { + var minifier = new CleanCSS(); + var minified = minifier.minify('a{background:url(image/}'); + this.callback(null, minified, minifier); + }, + 'should output correct content': function(error, minified) { + assert.equal(minified, 'a{background:url(image/}'); + }, + 'should raise no errors': function(error, minified, minifier) { + assert.equal(minifier.errors.length, 0); + }, + 'should raise one warning': function(error, minified, minifier) { + assert.equal(minifier.warnings.length, 1); + assert.equal(minifier.warnings[0], 'Broken URL declaration: \'url(image/\'.'); + } + }, + 'warnings on broken imports': { + topic: function () { + var minifier = new CleanCSS(); + var minified = minifier.minify('@impor'); + this.callback(null, minified, minifier); + }, + 'should output correct content': function(error, minified) { + assert.equal(minified, ''); + }, + 'should raise no errors': function(error, minified, minifier) { + assert.equal(minifier.errors.length, 0); + }, + 'should raise one warning': function(error, minified, minifier) { + assert.equal(minifier.warnings.length, 1); + assert.equal(minifier.warnings[0], 'Broken declaration: \'@impor\'.'); + } + }, + 'warnings on broken comments': { + topic: function () { + var minifier = new CleanCSS(); + var minified = minifier.minify('a{}/* '); + this.callback(null, minified, minifier); + }, + 'should output correct content': function(error, minified) { + assert.equal(minified, ''); + }, + 'should raise no errors': function(error, minified, minifier) { + assert.equal(minifier.errors.length, 0); + }, + 'should raise one warning': function(error, minified, minifier) { + assert.equal(minifier.warnings.length, 1); + assert.equal(minifier.warnings[0], 'Broken comment: \'/* \'.'); + } + }, 'no errors': { topic: function() { var minifier = new CleanCSS(); diff --git a/test/text/comments-processor-test.js b/test/text/comments-processor-test.js index 877f2eaa..646e098a 100644 --- a/test/text/comments-processor-test.js +++ b/test/text/comments-processor-test.js @@ -17,7 +17,7 @@ function processorContext(name, context, keepSpecialComments, keepBreaks) { function restored (targetCSS) { return function (sourceCSS) { - var processor = new CommentsProcessor(keepSpecialComments, keepBreaks); + var processor = new CommentsProcessor(null, keepSpecialComments, keepBreaks); var result = processor.restore(processor.escape(sourceCSS)); assert.equal(result, targetCSS); }; -- 2.34.1