From 8897d446c88eb7a6585a7954ae8766a7242c12da Mon Sep 17 00:00:00 2001 From: Kristoffer Gram Date: Thu, 4 Jan 2018 20:27:45 +0100 Subject: [PATCH] fix: proper URL escaping and wrapping (`url()`) (#627) --- lib/loader.js | 18 ++++++++++++------ lib/processCss.js | 6 ++---- lib/url/escape.js | 13 +++++++++++++ test/helpers.js | 2 ++ test/moduleTestCases/urls/expected.css | 2 +- test/urlTest.js | 24 +++++++++++++++++++++++- 6 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 lib/url/escape.js diff --git a/lib/loader.js b/lib/loader.js index d583a8b1..3d5033f6 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -83,7 +83,13 @@ module.exports = function(content, map) { } cssAsString = cssAsString.replace(result.importItemRegExpG, importItemMatcher.bind(this)); - if(query.url !== false) { + + // helper for ensuring valid CSS strings from requires + var urlEscapeHelper = ""; + + if(query.url !== false && result.urlItems.length > 0) { + 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); var idx = +match[1]; @@ -95,15 +101,14 @@ 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 "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"" + url.substr(idx); } urlRequest = url; - return "\" + require(" + loaderUtils.stringifyRequest(this, urlRequest) + ") + \""; + return "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \""; }.bind(this)); } - - + var exportJs = compileExports(result, importItemMatcher.bind(this), camelCaseKeys); if (exportJs) { exportJs = "exports.locals = " + exportJs + ";"; @@ -127,7 +132,8 @@ module.exports = function(content, map) { } // embed runtime - callback(null, "exports = module.exports = require(" + + callback(null, urlEscapeHelper + + "exports = module.exports = require(" + loaderUtils.stringifyRequest(this, require.resolve("./css-base.js")) + ")(" + 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; diff --git a/lib/url/escape.js b/lib/url/escape.js new file mode 100644 index 00000000..be9bd2c8 --- /dev/null +++ b/lib/url/escape.js @@ -0,0 +1,13 @@ +module.exports = function escape(url) { + // If url is already wrapped in quotes, remove them + if (/^['"].*['"]$/.test(url)) { + 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/test/helpers.js b/test/helpers.js index 132e2be5..df0d14b6 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("url/escape") >= 0) + return require("../lib/url/escape"); 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..4690319c 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 }", ""] @@ -103,6 +103,28 @@ 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("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 }", ""]