From d14e8b8b24d6e62789f73d3354ac130dcf5b62f6 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Tue, 31 Oct 2017 19:25:27 +0100 Subject: [PATCH 1/8] Escape invalid urls --- lib/escape-url.js | 13 +++++++++++++ lib/loader.js | 19 +++++++++++++------ lib/processCss.js | 6 ++---- 3 files changed, 28 insertions(+), 10 deletions(-) create mode 100644 lib/escape-url.js diff --git a/lib/escape-url.js b/lib/escape-url.js new file mode 100644 index 00000000..a27483c0 --- /dev/null +++ b/lib/escape-url.js @@ -0,0 +1,13 @@ +module.exports = function(url) { + // If url is already wrapped in quotes, remove them + if ((url[0] === '"' || url[0] === '\'') && url[0] === url[url.length - 1]) { + url = url.slice(1, -1); + } + // Should url be wrapped? + // See https://drafts.csswg.org/css-values-3/#urls + if (/["'() \t\n]/.test(url)) { + return '"' + url.replace(/"/g, '\\"').replace(/\n/g, '\\n') + '"' + } + + return url +} diff --git a/lib/loader.js b/lib/loader.js index cb8ff307..4cc523bf 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -82,8 +82,13 @@ module.exports = function(content, map) { "[" + JSON.stringify(importItem.export) + "] + \""; } + var escapeUrlHelper; cssAsString = cssAsString.replace(result.importItemRegExpG, importItemMatcher.bind(this)); - if(query.url !== false) { + if(query.url !== false && result.urlItems.length > 0) { + escapeUrlHelper = "var escapeUrl = require(" + + loaderUtils.stringifyRequest(this, require.resolve("./escape-url.js")) + + ")\n" + cssAsString = cssAsString.replace(result.urlItemRegExpG, function(item) { var match = result.urlItemRegExp.exec(item); var idx = +match[1]; @@ -95,15 +100,16 @@ module.exports = function(content, map) { if(idx > 0) { // idx === 0 is catched by isUrlRequest // in cases like url('webfont.eot?#iefix') urlRequest = url.substr(0, idx); - return "\" + require(" + loaderUtils.stringifyRequest(this, urlRequest) + ") + \"" + + return "\" + escapeUrl(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"" + url.substr(idx); } urlRequest = url; - return "\" + require(" + loaderUtils.stringifyRequest(this, urlRequest) + ") + \""; + return "\" + escapeUrl(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \""; }.bind(this)); + } else { + escapeUrlHelper = "" } - - + var exportJs = compileExports(result, importItemMatcher.bind(this), camelCaseKeys); if (exportJs) { exportJs = "exports.locals = " + exportJs + ";"; @@ -127,7 +133,8 @@ module.exports = function(content, map) { } // embed runtime - callback(null, "exports = module.exports = require(" + + callback(null, escapeUrlHelper + + "exports = module.exports = require(" + loaderUtils.stringifyRequest(this, require.resolve("./css-base.js")) + ")(" + query.sourceMap + ");\n" + "// imports\n" + diff --git a/lib/processCss.js b/lib/processCss.js index 8979f7ad..ab38c804 100644 --- a/lib/processCss.js +++ b/lib/processCss.js @@ -104,10 +104,8 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) { break; case "url": if (options.url && item.url.replace(/\s/g, '').length && !/^#/.test(item.url) && (isAlias(item.url) || loaderUtils.isUrlRequest(item.url, options.root))) { - // Don't remove quotes around url when contain space - if (item.url.indexOf(" ") === -1) { - item.stringType = ""; - } + // Strip quotes, they will be re-added if the module needs them + item.stringType = ""; delete item.innerSpacingBefore; delete item.innerSpacingAfter; var url = item.url; From 2db8469e943cb76031eb21d4542b1578703007f1 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Tue, 31 Oct 2017 19:26:47 +0100 Subject: [PATCH 2/8] Correct existing tests to expect double-quotes --- test/helpers.js | 2 ++ test/moduleTestCases/urls/expected.css | 2 +- test/urlTest.js | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/helpers.js b/test/helpers.js index 132e2be5..00acf983 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -12,6 +12,8 @@ function getEvaluated(output, modules) { fn(m, m.exports, function(module) { if(module.indexOf("css-base") >= 0) return require("../lib/css-base"); + if(module.indexOf("escape-url") >= 0) + return require("../lib/escape-url"); if(module.indexOf("-!/path/css-loader!") === 0) module = module.substr(19); if(modules && modules[module]) diff --git a/test/moduleTestCases/urls/expected.css b/test/moduleTestCases/urls/expected.css index ea4dba76..9578f0ba 100644 --- a/test/moduleTestCases/urls/expected.css +++ b/test/moduleTestCases/urls/expected.css @@ -2,7 +2,7 @@ background: url({./module}); background: url({./module}); background: url("{./module module}"); - background: url('{./module module}'); + background: url("{./module module}"); background: url({./module}); background: url({./module}#?iefix); background: url("#hash"); diff --git a/test/urlTest.js b/test/urlTest.js index bfe5c4ee..7309b26a 100644 --- a/test/urlTest.js +++ b/test/urlTest.js @@ -19,7 +19,7 @@ describe("url", function() { [1, ".class { background: green url(\"{./img img.png}\") xyz }", ""] ]); test("background 2 img contain space in name", ".class { background: green url( 'img img.png' ) xyz }", [ - [1, ".class { background: green url('{./img img.png}') xyz }", ""] + [1, ".class { background: green url(\"{./img img.png}\") xyz }", ""] ]); test("background img absolute", ".class { background: green url(/img.png) xyz }", [ [1, ".class { background: green url(/img.png) xyz }", ""] From 74431ba591835294beac0972b315d521f398da61 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Tue, 31 Oct 2017 19:27:28 +0100 Subject: [PATCH 3/8] Add url-escape specific tests --- test/urlTest.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/urlTest.js b/test/urlTest.js index 7309b26a..87718b6c 100644 --- a/test/urlTest.js +++ b/test/urlTest.js @@ -103,6 +103,25 @@ describe("url", function() { test("external schema-less url", ".class { background: green url(//raw.githubusercontent.com/webpack/media/master/logo/icon.png) xyz }", [ [1, ".class { background: green url(//raw.githubusercontent.com/webpack/media/master/logo/icon.png) xyz }", ""] ]); + + test("module wrapped in spaces", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(module-wrapped) xyz }", ""] + ], "", { './module': "\"module-wrapped\"" }); + test("module with space", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(\"module with space\") xyz }", ""] + ], "", { './module': "module with space" }); + test("module with quote", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(\"module\\\"with\\\"quote\") xyz }", ""] + ], "", { './module': "module\"with\"quote" }); + test("module with quote wrapped", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(\"module\\\"with\\\"quote\\\"wrapped\") xyz }", ""] + ], "", { './module': "\"module\"with\"quote\"wrapped\"" }); + test("module with parens", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(\"module(with-parens)\") xyz }", ""] + ], "", { './module': 'module(with-parens)' }); + test("module with newline", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(\"module\\nwith\\nnewline\") xyz }", ""] + ], "", { './module': "module\nwith\nnewline" }); test("background img with url", ".class { background: green url( \"img.png\" ) xyz }", [ [1, ".class { background: green url( \"img.png\" ) xyz }", ""] From fdac0b0ee30b67e6d56a7fcc5b0a08833333f350 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Tue, 31 Oct 2017 21:52:23 +0100 Subject: [PATCH 4/8] Switch to regex to detect quote-wrapped urls --- lib/escape-url.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/escape-url.js b/lib/escape-url.js index a27483c0..33253634 100644 --- a/lib/escape-url.js +++ b/lib/escape-url.js @@ -1,6 +1,6 @@ module.exports = function(url) { // If url is already wrapped in quotes, remove them - if ((url[0] === '"' || url[0] === '\'') && url[0] === url[url.length - 1]) { + if (/^['"].*['"]$/.test(url)) { url = url.slice(1, -1); } // Should url be wrapped? From 1ac76617f011eafda5f965e9432ef4e4dfc7d417 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Wed, 1 Nov 2017 19:54:42 +0100 Subject: [PATCH 5/8] Rename and rearrange url-escape variables --- lib/loader.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/loader.js b/lib/loader.js index 4cc523bf..4708aff3 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -82,10 +82,13 @@ module.exports = function(content, map) { "[" + JSON.stringify(importItem.export) + "] + \""; } - var escapeUrlHelper; cssAsString = cssAsString.replace(result.importItemRegExpG, importItemMatcher.bind(this)); + + // helper for ensuring valid CSS strings from requires + var urlEscapeHelper = ""; + if(query.url !== false && result.urlItems.length > 0) { - escapeUrlHelper = "var escapeUrl = require(" + + urlEscapeHelper = "var escape = require(" + loaderUtils.stringifyRequest(this, require.resolve("./escape-url.js")) + ")\n" @@ -100,14 +103,12 @@ module.exports = function(content, map) { if(idx > 0) { // idx === 0 is catched by isUrlRequest // in cases like url('webfont.eot?#iefix') urlRequest = url.substr(0, idx); - return "\" + escapeUrl(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"" + + return "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"" + url.substr(idx); } urlRequest = url; - return "\" + escapeUrl(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \""; + return "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \""; }.bind(this)); - } else { - escapeUrlHelper = "" } var exportJs = compileExports(result, importItemMatcher.bind(this), camelCaseKeys); @@ -133,7 +134,7 @@ module.exports = function(content, map) { } // embed runtime - callback(null, escapeUrlHelper + + callback(null, urlEscapeHelper + "exports = module.exports = require(" + loaderUtils.stringifyRequest(this, require.resolve("./css-base.js")) + ")(" + query.sourceMap + ");\n" + From 87caf335b5c6ea2c11c894c06113a688bf4a2067 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Wed, 1 Nov 2017 19:57:30 +0100 Subject: [PATCH 6/8] Rename lib/escape-url.js -> lib/url/escape.js --- lib/loader.js | 4 +--- lib/{escape-url.js => url/escape.js} | 2 +- test/helpers.js | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) rename lib/{escape-url.js => url/escape.js} (89%) diff --git a/lib/loader.js b/lib/loader.js index 4708aff3..eb72907b 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -88,9 +88,7 @@ module.exports = function(content, map) { var urlEscapeHelper = ""; if(query.url !== false && result.urlItems.length > 0) { - urlEscapeHelper = "var escape = require(" + - loaderUtils.stringifyRequest(this, require.resolve("./escape-url.js")) - + ")\n" + urlEscapeHelper = "var escape = require(" + loaderUtils.stringifyRequest(this, require.resolve("./url/escape.js")) + ")\n"; cssAsString = cssAsString.replace(result.urlItemRegExpG, function(item) { var match = result.urlItemRegExp.exec(item); diff --git a/lib/escape-url.js b/lib/url/escape.js similarity index 89% rename from lib/escape-url.js rename to lib/url/escape.js index 33253634..be9bd2c8 100644 --- a/lib/escape-url.js +++ b/lib/url/escape.js @@ -1,4 +1,4 @@ -module.exports = function(url) { +module.exports = function escape(url) { // If url is already wrapped in quotes, remove them if (/^['"].*['"]$/.test(url)) { url = url.slice(1, -1); diff --git a/test/helpers.js b/test/helpers.js index 00acf983..df0d14b6 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -12,8 +12,8 @@ function getEvaluated(output, modules) { fn(m, m.exports, function(module) { if(module.indexOf("css-base") >= 0) return require("../lib/css-base"); - if(module.indexOf("escape-url") >= 0) - return require("../lib/escape-url"); + if(module.indexOf("url/escape") >= 0) + return require("../lib/url/escape"); if(module.indexOf("-!/path/css-loader!") === 0) module = module.substr(19); if(modules && modules[module]) From d34841f0e7b41565b1909a7dc11705812655b71e Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Wed, 1 Nov 2017 20:02:47 +0100 Subject: [PATCH 7/8] Add semicolon to url escape-helper output --- lib/loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/loader.js b/lib/loader.js index eb72907b..6d28aa06 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -88,7 +88,7 @@ module.exports = function(content, map) { var urlEscapeHelper = ""; if(query.url !== false && result.urlItems.length > 0) { - urlEscapeHelper = "var escape = require(" + loaderUtils.stringifyRequest(this, require.resolve("./url/escape.js")) + ")\n"; + urlEscapeHelper = "var escape = require(" + loaderUtils.stringifyRequest(this, require.resolve("./url/escape.js")) + ");\n"; cssAsString = cssAsString.replace(result.urlItemRegExpG, function(item) { var match = result.urlItemRegExp.exec(item); From f144280386ae5268b93d259dedc5ec6b71dee287 Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Wed, 1 Nov 2017 20:14:25 +0100 Subject: [PATCH 8/8] Add test for url-loader output --- test/urlTest.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/urlTest.js b/test/urlTest.js index 87718b6c..4690319c 100644 --- a/test/urlTest.js +++ b/test/urlTest.js @@ -122,6 +122,9 @@ describe("url", function() { test("module with newline", ".class { background: green url(module) xyz }", [ [1, ".class { background: green url(\"module\\nwith\\nnewline\") xyz }", ""] ], "", { './module': "module\nwith\nnewline" }); + test("module from url-loader", ".class { background: green url(module) xyz }", [ + [1, ".class { background: green url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAA) xyz }", ""] + ], "", { './module': "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAA" }); test("background img with url", ".class { background: green url( \"img.png\" ) xyz }", [ [1, ".class { background: green url( \"img.png\" ) xyz }", ""]