Skip to content

Commit 8897d44

Browse files
kgrammichael-ciniawsky
kgram
authored and
michael-ciniawsky
committed
fix: proper URL escaping and wrapping (url()) (webpack-contrib#627)
1 parent 0dccfa9 commit 8897d44

File tree

6 files changed

+53
-12
lines changed

6 files changed

+53
-12
lines changed

‎lib/loader.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,13 @@ module.exports = function(content, map) {
8383
}
8484

8585
cssAsString = cssAsString.replace(result.importItemRegExpG, importItemMatcher.bind(this));
86-
if(query.url !== false) {
86+
87+
// helper for ensuring valid CSS strings from requires
88+
var urlEscapeHelper = "";
89+
90+
if(query.url !== false && result.urlItems.length > 0) {
91+
urlEscapeHelper = "var escape = require(" + loaderUtils.stringifyRequest(this, require.resolve("./url/escape.js")) + ");\n";
92+
8793
cssAsString = cssAsString.replace(result.urlItemRegExpG, function(item) {
8894
var match = result.urlItemRegExp.exec(item);
8995
var idx = +match[1];
@@ -95,15 +101,14 @@ module.exports = function(content, map) {
95101
if(idx > 0) { // idx === 0 is catched by isUrlRequest
96102
// in cases like url('webfont.eot?#iefix')
97103
urlRequest = url.substr(0, idx);
98-
return "\" + require(" + loaderUtils.stringifyRequest(this, urlRequest) + ") + \"" +
104+
return "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"" +
99105
url.substr(idx);
100106
}
101107
urlRequest = url;
102-
return "\" + require(" + loaderUtils.stringifyRequest(this, urlRequest) + ") + \"";
108+
return "\" + escape(require(" + loaderUtils.stringifyRequest(this, urlRequest) + ")) + \"";
103109
}.bind(this));
104110
}
105-
106-
111+
107112
var exportJs = compileExports(result, importItemMatcher.bind(this), camelCaseKeys);
108113
if (exportJs) {
109114
exportJs = "exports.locals = " + exportJs + ";";
@@ -127,7 +132,8 @@ module.exports = function(content, map) {
127132
}
128133

129134
// embed runtime
130-
callback(null, "exports = module.exports = require(" +
135+
callback(null, urlEscapeHelper +
136+
"exports = module.exports = require(" +
131137
loaderUtils.stringifyRequest(this, require.resolve("./css-base.js")) +
132138
")(" + sourceMap + ");\n" +
133139
"// imports\n" +

‎lib/processCss.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,8 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) {
104104
break;
105105
case "url":
106106
if (options.url && item.url.replace(/\s/g, '').length && !/^#/.test(item.url) && (isAlias(item.url) || loaderUtils.isUrlRequest(item.url, options.root))) {
107-
// Don't remove quotes around url when contain space
108-
if (item.url.indexOf(" ") === -1) {
109-
item.stringType = "";
110-
}
107+
// Strip quotes, they will be re-added if the module needs them
108+
item.stringType = "";
111109
delete item.innerSpacingBefore;
112110
delete item.innerSpacingAfter;
113111
var url = item.url;

‎lib/url/escape.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
module.exports = function escape(url) {
2+
// If url is already wrapped in quotes, remove them
3+
if (/^['"].*['"]$/.test(url)) {
4+
url = url.slice(1, -1);
5+
}
6+
// Should url be wrapped?
7+
// See https://drafts.csswg.org/css-values-3/#urls
8+
if (/["'() \t\n]/.test(url)) {
9+
return '"' + url.replace(/"/g, '\\"').replace(/\n/g, '\\n') + '"'
10+
}
11+
12+
return url
13+
}

‎test/helpers.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ function getEvaluated(output, modules) {
1212
fn(m, m.exports, function(module) {
1313
if(module.indexOf("css-base") >= 0)
1414
return require("../lib/css-base");
15+
if(module.indexOf("url/escape") >= 0)
16+
return require("../lib/url/escape");
1517
if(module.indexOf("-!/path/css-loader!") === 0)
1618
module = module.substr(19);
1719
if(modules && modules[module])

‎test/moduleTestCases/urls/expected.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
background: url({./module});
33
background: url({./module});
44
background: url("{./module module}");
5-
background: url('{./module module}');
5+
background: url("{./module module}");
66
background: url({./module});
77
background: url({./module}#?iefix);
88
background: url("#hash");

‎test/urlTest.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe("url", function() {
1919
[1, ".class { background: green url(\"{./img img.png}\") xyz }", ""]
2020
]);
2121
test("background 2 img contain space in name", ".class { background: green url( 'img img.png' ) xyz }", [
22-
[1, ".class { background: green url('{./img img.png}') xyz }", ""]
22+
[1, ".class { background: green url(\"{./img img.png}\") xyz }", ""]
2323
]);
2424
test("background img absolute", ".class { background: green url(/img.png) xyz }", [
2525
[1, ".class { background: green url(/img.png) xyz }", ""]
@@ -103,6 +103,28 @@ describe("url", function() {
103103
test("external schema-less url", ".class { background: green url(//raw.githubusercontent.com/webpack/media/master/logo/icon.png) xyz }", [
104104
[1, ".class { background: green url(//raw.githubusercontent.com/webpack/media/master/logo/icon.png) xyz }", ""]
105105
]);
106+
107+
test("module wrapped in spaces", ".class { background: green url(module) xyz }", [
108+
[1, ".class { background: green url(module-wrapped) xyz }", ""]
109+
], "", { './module': "\"module-wrapped\"" });
110+
test("module with space", ".class { background: green url(module) xyz }", [
111+
[1, ".class { background: green url(\"module with space\") xyz }", ""]
112+
], "", { './module': "module with space" });
113+
test("module with quote", ".class { background: green url(module) xyz }", [
114+
[1, ".class { background: green url(\"module\\\"with\\\"quote\") xyz }", ""]
115+
], "", { './module': "module\"with\"quote" });
116+
test("module with quote wrapped", ".class { background: green url(module) xyz }", [
117+
[1, ".class { background: green url(\"module\\\"with\\\"quote\\\"wrapped\") xyz }", ""]
118+
], "", { './module': "\"module\"with\"quote\"wrapped\"" });
119+
test("module with parens", ".class { background: green url(module) xyz }", [
120+
[1, ".class { background: green url(\"module(with-parens)\") xyz }", ""]
121+
], "", { './module': 'module(with-parens)' });
122+
test("module with newline", ".class { background: green url(module) xyz }", [
123+
[1, ".class { background: green url(\"module\\nwith\\nnewline\") xyz }", ""]
124+
], "", { './module': "module\nwith\nnewline" });
125+
test("module from url-loader", ".class { background: green url(module) xyz }", [
126+
[1, ".class { background: green url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAA) xyz }", ""]
127+
], "", { './module': "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAA" });
106128

107129
test("background img with url", ".class { background: green url( \"img.png\" ) xyz }", [
108130
[1, ".class { background: green url( \"img.png\" ) xyz }", ""]

0 commit comments

Comments
 (0)