Fixes #464 - handling escaped braces in URLs.

It was properly handled in URL processor but not URL rewriter.

This fix extracts a UrlScanner class with algorithm from URL processor
and reuses it in URL rewriter.

Technically it should be a fix not such a "major" refactor, but bending
the rules shouldn't hurt :-).

diff --git a/History.md b/History.md
index e1658c7..d2aae51 100644 (file)
--- a/History.md
+++ b/History.md
@@ -6,6 +6,7 @@
 [3.1.3 / 2015-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.2...3.1)
 ==================
 
+* Fixes issue [#464](https://github.com/jakubpawlowicz/clean-css/issues/464) - data URI with quoted braces.
 * Fixes issue [#475](https://github.com/jakubpawlowicz/clean-css/issues/475) - whitespace after closing brace.
 
 [3.1.2 / 2015-03-01](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.1...v3.1.2)

diff --git a/lib/images/url-rewriter.js b/lib/images/url-rewriter.js
index 955ea0c..be4a5eb 100644 (file)
--- a/lib/images/url-rewriter.js
+++ b/lib/images/url-rewriter.js
@@ -1,46 +1,30 @@
 var path = require('path');
 var url = require('url');
 
-function UrlRewriter(options) {
+var UrlScanner = require('../utils/url-scanner');
+
+function UrlRewriter(options, context) {
   this.options = options;
+  this.context = context;
 }
 
 UrlRewriter.prototype.process = function (data) {
-  var tempData = [];
-  var nextStart = 0;
-  var nextEnd = 0;
-  var cursor = 0;
-
-  for (; nextEnd < data.length;) {
-    nextStart = data.indexOf('url(', nextEnd);
-    if (nextStart == -1)
-      break;
-
-    nextEnd = data.indexOf(')', nextStart + 4);
-    if (nextEnd == -1)
-      break;
-
-    tempData.push(data.substring(cursor, nextStart));
-    var url = data.substring(nextStart + 4, nextEnd);
-    if (!/\/\*|\*\//.test(url))
-      url = url.replace(/['"]/g, '');
-
-    tempData.push('url(' + rebase(url, this.options) + ')');
-    cursor = nextEnd + 1;
-  }
+  var self = this;
 
-  return tempData.length > 0 ?
-    tempData.join('') + data.substring(cursor, data.length) :
-    data;
+  return new UrlScanner(data, this.context).reduce(function (url, tempData) {
+    url = url.replace(/^url\(\s*['"]?|['"]?\s*\)$/g, '');
+    tempData.push('url(' + rebase(url, self.options) + ')');
+  });
 };
 
 function rebase(resource, options) {
   // TODO: this is getting insane now - pending refactor in #436
   var importUrl = resource.substring(resource.length - 4) == '.css';
+  var dataUri = resource.indexOf('data:') === 0;
   var specialUrl = resource[0] == '/' ||
     resource[0] == '#' ||
     (!options.imports && importUrl) ||
-    resource.indexOf('data:') === 0 ||
+    dataUri ||
     /^https?:\/\//.exec(resource) !== null ||
     /__\w+__/.exec(resource) !== null;
   var rebased;
@@ -53,7 +37,7 @@ function rebase(resource, options) {
   }
 
   if (specialUrl)
-    return resource;
+    return dataUri ? '\'' + resource + '\'' : resource;
 
   if (/https?:\/\//.test(options.toBase))
     return url.resolve(options.toBase, resource);

diff --git a/lib/imports/inliner.js b/lib/imports/inliner.js
index 697388c..9440550 100644 (file)
--- a/lib/imports/inliner.js
+++ b/lib/imports/inliner.js
@@ -260,7 +260,7 @@ function inlineRemoteResource(importedFile, mediaQuery, context) {
     res.on('end', function() {
       var importedData = chunks.join('');
       if (context.rebase)
-        importedData = new UrlRewriter({ toBase: importedUrl }).process(importedData);
+        importedData = new UrlRewriter({ toBase: importedUrl }, context).process(importedData);
       importedData = context.sourceTracker.store(importedUrl, importedData);
       importedData = rebaseMap(importedData, importedUrl);
 
@@ -312,7 +312,7 @@ function inlineLocalResource(importedFile, mediaQuery, context) {
       relative: true,
       fromBase: importRelativeTo,
       toBase: context.baseRelativeTo
-    });
+    }, context);
     importedData = rewriter.process(importedData);
   }
   importedData = context.sourceTracker.store(path.resolve(context.relativeTo, fullPath), importedData);

diff --git a/lib/text/urls-processor.js b/lib/text/urls-processor.js
index 2399388..841e8c0 100644 (file)
--- a/lib/text/urls-processor.js
+++ b/lib/text/urls-processor.js
@@ -1,8 +1,6 @@
 var EscapeStore = require('./escape-store');
+var UrlScanner = require('../utils/url-scanner');
 
-var URL_PREFIX = 'url(';
-var UPPERCASE_URL_PREFIX = 'URL(';
-var URL_SUFFIX = ')';
 var lineBreak = require('os').EOL;
 
 function UrlsProcessor(context, saveWaypoints, removeTrailingSpace) {
@@ -16,51 +14,13 @@ function UrlsProcessor(context, saveWaypoints, removeTrailingSpace) {
 // marker for further restoring. It's done via string scanning
 // instead of regexps to speed up the process.
 UrlsProcessor.prototype.escape = function (data) {
-  var nextStart = 0;
-  var nextStartUpperCase = 0;
-  var nextEnd = 0;
-  var cursor = 0;
-  var tempData = [];
   var breaksCount;
   var lastBreakAt;
   var indent;
   var saveWaypoints = this.saveWaypoints;
-  var hasUppercaseUrl = data.indexOf(UPPERCASE_URL_PREFIX) > -1;
-
-  for (; nextEnd < data.length;) {
-    nextStart = data.indexOf(URL_PREFIX, nextEnd);
-    nextStartUpperCase = hasUppercaseUrl ? data.indexOf(UPPERCASE_URL_PREFIX, nextEnd) : -1;
-    if (nextStart == -1 && nextStartUpperCase == -1)
-      break;
-
-    if (nextStart == -1 && nextStartUpperCase > -1)
-      nextStart = nextStartUpperCase;
-
-    if (data[nextStart + URL_PREFIX.length] == '"')
-      nextEnd = data.indexOf('"', nextStart + URL_PREFIX.length + 1);
-    else if (data[nextStart + URL_PREFIX.length] == '\'')
-      nextEnd = data.indexOf('\'', nextStart + URL_PREFIX.length + 1);
-    else
-      nextEnd = data.indexOf(URL_SUFFIX, 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) + '\'.');
-    } else {
-      if (data[nextEnd] != URL_SUFFIX)
-        nextEnd = data.indexOf(URL_SUFFIX, nextEnd);
-    }
-
-    var url = data.substring(nextStart, nextEnd + 1);
+  var self = this;
 
+  return new UrlScanner(data, this.context).reduce(function (url, tempData) {
     if (saveWaypoints) {
       breaksCount = url.split(lineBreak).length - 1;
       lastBreakAt = url.lastIndexOf(lineBreak);
@@ -69,16 +29,9 @@ UrlsProcessor.prototype.escape = function (data) {
         url.length;
     }
 
-    var placeholder = this.urls.store(url, saveWaypoints ? [breaksCount, indent] : null);
-    tempData.push(data.substring(cursor, nextStart));
+    var placeholder = self.urls.store(url, saveWaypoints ? [breaksCount, indent] : null);
     tempData.push(placeholder);
-
-    cursor = nextEnd + 1;
-  }
-
-  return tempData.length > 0 ?
-    tempData.join('') + data.substring(cursor, data.length) :
-    data;
+  });
 };
 
 function normalize(url) {

diff --git a/lib/utils/source-reader.js b/lib/utils/source-reader.js
index d6b9bf1..8b22a40 100644 (file)
--- a/lib/utils/source-reader.js
+++ b/lib/utils/source-reader.js
@@ -48,7 +48,7 @@ function fromHash(outerContext, sources) {
       urls: outerContext.options.rebase,
       fromBase: path.dirname(path.resolve(source)),
       toBase: toBase
-    });
+    }, this.outerContext);
     styles = rewriter.process(styles);
 
     if (outerContext.options.sourceMap && inputSourceMap) {

diff --git a/lib/utils/url-scanner.js b/lib/utils/url-scanner.js
new file mode 100644 (file)
index 0000000..79fb2ec
--- /dev/null
+++ b/lib/utils/url-scanner.js
@@ -0,0 +1,64 @@
+var URL_PREFIX = 'url(';
+var UPPERCASE_URL_PREFIX = 'URL(';
+var URL_SUFFIX = ')';
+
+function UrlScanner(data, context) {
+  this.data = data;
+  this.context = context;
+}
+
+UrlScanner.prototype.reduce = function (callback) {
+  var nextStart = 0;
+  var nextStartUpperCase = 0;
+  var nextEnd = 0;
+  var cursor = 0;
+  var tempData = [];
+  var data = this.data;
+  var hasUppercaseUrl = data.indexOf(UPPERCASE_URL_PREFIX) > -1;
+
+  for (; nextEnd < data.length;) {
+    nextStart = data.indexOf(URL_PREFIX, nextEnd);
+    nextStartUpperCase = hasUppercaseUrl ? data.indexOf(UPPERCASE_URL_PREFIX, nextEnd) : -1;
+    if (nextStart == -1 && nextStartUpperCase == -1)
+      break;
+
+    if (nextStart == -1 && nextStartUpperCase > -1)
+      nextStart = nextStartUpperCase;
+
+    if (data[nextStart + URL_PREFIX.length] == '"')
+      nextEnd = data.indexOf('"', nextStart + URL_PREFIX.length + 1);
+    else if (data[nextStart + URL_PREFIX.length] == '\'')
+      nextEnd = data.indexOf('\'', nextStart + URL_PREFIX.length + 1);
+    else
+      nextEnd = data.indexOf(URL_SUFFIX, 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) + '\'.');
+    } else {
+      if (data[nextEnd] != URL_SUFFIX)
+        nextEnd = data.indexOf(URL_SUFFIX, nextEnd);
+    }
+
+    tempData.push(data.substring(cursor, nextStart));
+
+    var url = data.substring(nextStart, nextEnd + 1);
+    callback(url, tempData);
+
+    cursor = nextEnd + 1;
+  }
+
+  return tempData.length > 0 ?
+    tempData.join('') + data.substring(cursor, data.length) :
+    data;
+};
+
+module.exports = UrlScanner;

diff --git a/test/fixtures/issue-464-min.css b/test/fixtures/issue-464-min.css
new file mode 100644 (file)
index 0000000..04d422b
--- /dev/null
+++ b/test/fixtures/issue-464-min.css
@@ -0,0 +1 @@
+div{background:url('data:image/svg+xml;charset=US-ASCII,%3C%3Fxml%20version%3D%221.0%22%20encoding%3D%22utf-8%22%3F%3E%3C!DOCTYPE%20svg%20PUBLIC%20%22-%2F%2FW3C%2F%2FDTD%20SVG%201.1%2F%2FEN%22%20%22http%3A%2F%2Fwww.w3.org%2FGraphics%2FSVG%2F1.1%2FDTD%2Fsvg11.dtd%22%3E%3Csvg%20version%3D%221.1%22%20id%3D%22Layer_1%22%20xmlns%3Asketch%3D%22http%3A%2F%2Fwww.bohemiancoding.com%2Fsketch%2Fns%22%20%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%20x%3D%220px%22%20y%3D%220px%22%20width%3D%2230px%22%20height%3D%2230px%22%20%20viewBox%3D%22-317%20386%2030%2030%22%20enable-background%3D%22new%20-317%20386%2030%2030%22%20xml%3Aspace%3D%22preserve%22%3E%3Ctitle%3ERectangle%20166%3C%2Ftitle%3E%3Cdesc%3ECreated%20with%20Sketch%20Beta.%3C%2Fdesc%3E%3Cg%20id%3D%22item-form1%22%20sketch%3Atype%3D%22MSPage%22%3E%20%3Cg%20id%3D%22in-course%22%20transform%3D%22translate(-613.000000%2C%20-1744.000000)%22%20sketch%3Atype%3D%22MSArtboardGroup%22%3E%20%20%3Cg%20id%3D%22Feedback-added-to-Answer%22%20transform%3D%22translate(526.000000%2C%201230.000000)%22%20sketch%3Atype%3D%22MSLayerGroup%22%3E%20%20%20%3Cg%20id%3D%22Answer-2%22%20transform%3D%22translate(0.000000%2C%20297.000000)%22%20sketch%3Atype%3D%22MSShapeGroup%22%3E%20%20%20%20%3Cg%20id%3D%22Clicked_x2F_Feedbac%22%20transform%3D%22translate(76.000000%2C%20208.000000)%22%3E%20%20%20%20%20%3Cg%20id%3D%22Text-stuff-copy%22%3E%20%20%20%20%20%20%3Cg%20id%3D%22B-_x7C_-i-_x7C_-_x3D_%22%20transform%3D%22translate(11.000000%2C%200.000000)%22%3E%20%20%20%20%20%20%20%3Cpath%20id%3D%22Rectangle-166%22%20fill%3D%22%23393F44%22%20d%3D%22M-298.5%2C414.5v-9h-4c-1.653%2C0-3%2C1.344-3%2C3c0%2C1.653%2C1.343%2C3%2C3%2C3v3h2v-8h1v8H-298.5%20%20%20%20%20%20%20%20z%22%2F%3E%20%20%20%20%20%20%3C%2Fg%3E%20%20%20%20%20%3C%2Fg%3E%20%20%20%20%3C%2Fg%3E%20%20%20%3C%2Fg%3E%20%20%3C%2Fg%3E%20%3C%2Fg%3E%3C%2Fg%3E%3C%2Fsvg%3E')}
\ No newline at end of file

diff --git a/test/fixtures/issue-464.css b/test/fixtures/issue-464.css
new file mode 100644 (file)
index 0000000..8ef37ef
--- /dev/null
+++ b/test/fixtures/issue-464.css
@@ -0,0 +1 @@
+div{background:url('data:image/svg+xml;charset=US-ASCII,%3C%3Fxml%20version%3D%221.0%22%20encoding%3D%22utf-8%22%3F%3E%3C!DOCTYPE%20svg%20PUBLIC%20%22-%2F%2FW3C%2F%2FDTD%20SVG%201.1%2F%2FEN%22%20%22http%3A%2F%2Fwww.w3.org%2FGraphics%2FSVG%2F1.1%2FDTD%2Fsvg11.dtd%22%3E%3Csvg%20version%3D%221.1%22%20id%3D%22Layer_1%22%20xmlns%3Asketch%3D%22http%3A%2F%2Fwww.bohemiancoding.com%2Fsketch%2Fns%22%20%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%20x%3D%220px%22%20y%3D%220px%22%20width%3D%2230px%22%20height%3D%2230px%22%20%20viewBox%3D%22-317%20386%2030%2030%22%20enable-background%3D%22new%20-317%20386%2030%2030%22%20xml%3Aspace%3D%22preserve%22%3E%3Ctitle%3ERectangle%20166%3C%2Ftitle%3E%3Cdesc%3ECreated%20with%20Sketch%20Beta.%3C%2Fdesc%3E%3Cg%20id%3D%22item-form1%22%20sketch%3Atype%3D%22MSPage%22%3E%20%3Cg%20id%3D%22in-course%22%20transform%3D%22translate(-613.000000%2C%20-1744.000000)%22%20sketch%3Atype%3D%22MSArtboardGroup%22%3E%20%20%3Cg%20id%3D%22Feedback-added-to-Answer%22%20transform%3D%22translate(526.000000%2C%201230.000000)%22%20sketch%3Atype%3D%22MSLayerGroup%22%3E%20%20%20%3Cg%20id%3D%22Answer-2%22%20transform%3D%22translate(0.000000%2C%20297.000000)%22%20sketch%3Atype%3D%22MSShapeGroup%22%3E%20%20%20%20%3Cg%20id%3D%22Clicked_x2F_Feedbac%22%20transform%3D%22translate(76.000000%2C%20208.000000)%22%3E%20%20%20%20%20%3Cg%20id%3D%22Text-stuff-copy%22%3E%20%20%20%20%20%20%3Cg%20id%3D%22B-_x7C_-i-_x7C_-_x3D_%22%20transform%3D%22translate(11.000000%2C%200.000000)%22%3E%20%20%20%20%20%20%20%3Cpath%20id%3D%22Rectangle-166%22%20fill%3D%22%23393F44%22%20d%3D%22M-298.5%2C414.5v-9h-4c-1.653%2C0-3%2C1.344-3%2C3c0%2C1.653%2C1.343%2C3%2C3%2C3v3h2v-8h1v8H-298.5%20%20%20%20%20%20%20%20z%22%2F%3E%20%20%20%20%20%20%3C%2Fg%3E%20%20%20%20%20%3C%2Fg%3E%20%20%20%20%3C%2Fg%3E%20%20%20%3C%2Fg%3E%20%20%3C%2Fg%3E%20%3C%2Fg%3E%3C%2Fg%3E%3C%2Fsvg%3E')}