Fixes #839 - allows URIs in import inlining rules.
authorJakub Pawlowicz <contact@jakubpawlowicz.com>
Sun, 18 Dec 2016 11:24:53 +0000 (12:24 +0100)
committerJakub Pawlowicz <contact@jakubpawlowicz.com>
Sun, 18 Dec 2016 15:33:18 +0000 (16:33 +0100)
Why:

* To give more control over which resource can and cannot be
  inlined.

History.md
lib/reader/is-allowed-resource.js
test/protocol-imports-test.js
test/reader/is-allowed-resource-test.js [new file with mode: 0644]

index aa69208..2776dba 100644 (file)
@@ -7,6 +7,7 @@
 * Replaces the old tokenizer with a new one which doesn't use any escaping.
 * Replaces the old `@import` inlining with one on top of the new tokenizer.
 * Simplifies URL rebasing with a single `rebaseTo` option in API or inferred from `--output` in CLI.
+* 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.
 
index e7d0b06..826681e 100644 (file)
@@ -1,13 +1,18 @@
+var path = require('path');
 var url = require('url');
 
+var isRemoteResource = require('../utils/is-remote-resource');
 var hasProtocol = require('../utils/has-protocol');
 
 var HTTP_PROTOCOL = 'http:';
 
 function isAllowedResource(uri, isRemote, rules) {
   var match;
+  var absoluteUri;
   var allowed = true;
   var rule;
+  var isNegated;
+  var normalizedRule;
   var i;
 
   if (rules.length === 0) {
@@ -22,10 +27,22 @@ function isAllowedResource(uri, isRemote, rules) {
     url.parse(uri).host :
     uri;
 
+  absoluteUri = isRemote ?
+    uri :
+    path.resolve(uri);
+
   for (i = 0; i < rules.length; i++) {
     rule = rules[i];
+    isNegated = rule[0] == '!';
+    normalizedRule = rule.substring(1);
 
-    if (rule == 'all') {
+    if (isNegated && isRemote && isRemoteRule(normalizedRule)) {
+      allowed = allowed && !isAllowedResource(uri, true, [normalizedRule]);
+    } else if (isNegated && !isRemote && !isRemoteRule(normalizedRule)) {
+      allowed = allowed && !isAllowedResource(uri, false, [normalizedRule]);
+    } else if (isNegated) {
+      allowed = allowed && true;
+    } else if (rule == 'all') {
       allowed = true;
     } else if (isRemote && rule == 'local') {
       allowed = false;
@@ -35,7 +52,17 @@ function isAllowedResource(uri, isRemote, rules) {
       allowed = false;
     } else if (!isRemote && rule == 'local') {
       allowed = true;
-    } else if (rule[0] == '!' && rule.substring(1) === match) {
+    } else if (rule === match) {
+      allowed = true;
+    } else if (rule === uri) {
+      allowed = true;
+    } else if (isRemote && absoluteUri.indexOf(rule) === 0) {
+      allowed = true;
+    } else if (!isRemote && absoluteUri.indexOf(path.resolve(rule)) === 0) {
+      allowed = true;
+    } else if (isRemote != isRemoteRule(normalizedRule)) {
+      allowed = allowed && true;
+    } else {
       allowed = false;
     }
   }
@@ -43,4 +70,8 @@ function isAllowedResource(uri, isRemote, rules) {
   return allowed;
 }
 
+function isRemoteRule(rule) {
+  return isRemoteResource(rule) || url.parse(HTTP_PROTOCOL + '//' + rule).host == rule;
+}
+
 module.exports = isAllowedResource;
index 6f347dc..b73615b 100644 (file)
@@ -887,5 +887,49 @@ vows.describe('protocol imports').addBatch({
     'should process first imports': function (error, minified) {
       assert.equal(minified.styles, '@import url(//127.0.0.1/remote.css);.one{color:red}');
     }
+  },
+  'allowed imports - from specific URI': {
+    topic: function () {
+      var source = '@import url(http://127.0.0.1/remote.css);@import url(http://assets.127.0.0.1/remote.css);@import url(test/fixtures/partials/one.css);';
+      this.reqMocks = nock('http://assets.127.0.0.1')
+        .get('/remote.css')
+        .reply(200, 'p{width:100%}');
+      new CleanCSS({ processImportFrom: ['http://assets.127.0.0.1/remote.css', 'test/fixtures/partials/one.css'] }).minify(source, this.callback);
+    },
+    'should not raise errors': function (error, minified) {
+      assert.isEmpty(minified.errors);
+    },
+    'should raise warnings': function (error, minified) {
+      assert.lengthOf(minified.warnings, 1);
+    },
+    'should process imports': function (error, minified) {
+      assert.equal(minified.styles, '@import url(http://127.0.0.1/remote.css);p{width:100%}.one{color:red}');
+    },
+    teardown: function () {
+      assert.isTrue(this.reqMocks.isDone());
+      nock.cleanAll();
+    }
+  },
+  'allowed imports - from URI prefix': {
+    topic: function () {
+      var source = '@import url(http://127.0.0.1/remote.css);@import url(http://assets.127.0.0.1/remote.css);@import url(test/fixtures/partials/one.css);';
+      this.reqMocks = nock('http://assets.127.0.0.1')
+        .get('/remote.css')
+        .reply(200, 'p{width:100%}');
+      new CleanCSS({ processImportFrom: ['!http://127.0.0.1/', 'test/fixtures/partials'] }).minify(source, this.callback);
+    },
+    'should not raise errors': function (error, minified) {
+      assert.isEmpty(minified.errors);
+    },
+    'should raise warnings': function (error, minified) {
+      assert.lengthOf(minified.warnings, 1);
+    },
+    'should process imports': function (error, minified) {
+      assert.equal(minified.styles, '@import url(http://127.0.0.1/remote.css);p{width:100%}.one{color:red}');
+    },
+    teardown: function () {
+      assert.isTrue(this.reqMocks.isDone());
+      nock.cleanAll();
+    }
   }
 }).export(module);
diff --git a/test/reader/is-allowed-resource-test.js b/test/reader/is-allowed-resource-test.js
new file mode 100644 (file)
index 0000000..26eb9dd
--- /dev/null
@@ -0,0 +1,66 @@
+var assert = require('assert');
+
+var vows = require('vows');
+
+var isAllowedResource = require('../../lib/reader/is-allowed-resource');
+
+vows.describe(isAllowedResource)
+  .addBatch({
+    'local URI': {
+      'topic': 'test/fixtures/partials/one.css',
+      'is allowed': function (topic) {
+        assert.isTrue(isAllowedResource(topic, false, [topic]));
+      }
+    },
+    'local matching URI prefix': {
+      'topic': 'test/fixtures/partials/one.css',
+      'is allowed': function (topic) {
+        assert.isTrue(isAllowedResource(topic, false, ['test/fixtures/partials']));
+      }
+    },
+    'local not matching URI prefix': {
+      'topic': 'test/fixtures/partials/one.css',
+      'is not allowed': function (topic) {
+        assert.isFalse(isAllowedResource(topic, false, ['test/fixtures/partials-relative']));
+      }
+    },
+    'remote URI': {
+      'topic': 'http://127.0.0.1/styles.css',
+      'is allowed': function (topic) {
+        assert.isTrue(isAllowedResource(topic, true, [topic]));
+      }
+    },
+    'remote matching URI prefix': {
+      'topic': 'http://127.0.0.1/path/to/styles.css',
+      'is allowed': function (topic) {
+        assert.isTrue(isAllowedResource(topic, true, ['http://127.0.0.1/path/to']));
+      }
+    },
+    'remote not matching URI prefix': {
+      'topic': 'http://127.0.0.1/path/to/styles.css',
+      'is not allowed': function (topic) {
+        assert.isFalse(isAllowedResource(topic, true, ['http://127.0.0.1/another/path/to']));
+      }
+    }
+  })
+  .addBatch({
+    'negated rule': {
+      'topic': 'http://127.0.0.1/path/to/styles.css',
+      'is not allowed': function (topic) {
+        assert.isFalse(isAllowedResource(topic, true, ['!127.0.0.1']));
+      }
+    },
+    'negated rules': {
+      'topic': 'http://127.0.0.1/path/to/styles.css',
+      'is not allowed': function (topic) {
+        assert.isFalse(isAllowedResource(topic, true, ['!127.0.0.1', '!assets.127.0.0.1']));
+      }
+    },
+    'negated remote then local rules': {
+      'topic': 'http://127.0.0.1/path/to/styles.css',
+      'is not allowed': function (topic) {
+        assert.isFalse(isAllowedResource(topic, true, ['!127.0.0.1', '!assets.127.0.0.1', '!path/to/styles.css']));
+      }
+    }
+  })
+  .export(module);