Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing quotes in url() for data URIs returned from another loader #615

Closed
edmorley opened this issue Sep 28, 2017 · 3 comments · Fixed by #627
Closed

Missing quotes in url() for data URIs returned from another loader #615

edmorley opened this issue Sep 28, 2017 · 3 comments · Fixed by #627

Comments

@edmorley
Copy link

edmorley commented Sep 28, 2017

Hi!

STR:

Using node 8.5.0:

  1. Clone https://github.com/edmorley/css-loader-missing-quotes-testcase
  2. yarn install
  3. yarn build
  4. Inspect dist/styles.bundle.css

Expected:

#inline-data-uri {
  background-image: url("data:image/svg+xml,%3C?xml version='1.0'?%3E %3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E %3Crect x='10' y='10' height='100' width='100' style='stroke:%23ff0000; fill: %230000ff'/%3E %3C/svg%3E");
}

#data-uri-from-loader {
  background-image: url("data:image/svg+xml,%3C?xml version='1.0'?%3E %3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E %3Crect x='10' y='10' height='100' width='100' style='stroke:%23ff0000; fill: %230000ff'/%3E %3C/svg%3E");
}

Actual

#inline-data-uri {
  background-image: url("data:image/svg+xml,%3C?xml version='1.0'?%3E %3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E %3Crect x='10' y='10' height='100' width='100' style='stroke:%23ff0000; fill: %230000ff'/%3E %3C/svg%3E");
}

#data-uri-from-loader {
  background-image: url(data:image/svg+xml,%3C?xml version='1.0'?%3E %3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E %3Crect x='10' y='10' height='100' width='100' style='stroke:%23ff0000; fill: %230000ff'/%3E %3C/svg%3E);
}

ie: Including the data URI inline in the CSS file works fine (thanks to the fix in #15), however when the resource is url('./svg-data-uri.txt') and so processed by another loader (which for simplicity for this testcase is just the raw-loader) the data URI string returned by the loader isn't wrapped in quotes, so is invalid CSS.

Contrast this to html-loader by inspecting dist/index.html, which successfully converts:

<img src="./svg-data-uri.txt">

...to:

<img src="data:image/svg+xml,%3C?xml version='1.0'?%3E %3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E %3Crect x='10' y='10' height='100' width='100' style='stroke:%23ff0000; fill: %230000ff'/%3E %3C/svg%3E">

Due to this issue, svg-url-loader unfortunately defaults to adding extra quotes around the string it returns, so that CSS url() references work. However this then breaks the html-loader (bhovhannes/svg-url-loader#126) and "require() string from JS" use-cases, which require consumers to remember to set the loader's noquotes option to false (eg neutrinojs/neutrino#334).

However I think this is ideally something that should be fixed in css-loader itself, meaning svg-url-loader could stop wrapping everything in quotes, solving the other problems.

Thoughts? :-)

For quick reference, the relevant package versions in the testcase repo's yarn.lock are:
webpack 3.6.0, css-loader 0.28.7, raw-loader 0.5.1, html-loader 0.5.1, extract-text-webpack-plugin 3.3.0.

@kayneb
Copy link

kayneb commented Oct 24, 2017

This is affecting us too. Looks like url-loader should be wrapping this kind of string in quotes, if not all URLs in quotes.

@kgram
Copy link
Contributor

kgram commented Oct 31, 2017

This is definitely an error on css-loaders part. At the moment, quotes are stripped unless there is a space in the url, but the rules for valid URLs are more complicated than that. To quote the spec:

Note: This unquoted syntax is cannot accept a argument and has extra escaping requirements: parentheses, whitespace characters, single quotes (') and double quotes (") appearing in a URL must be escaped with a backslash, e.g. url(open(parens), url(close)parens). (In quoted url()s, only newlines and the character used to quote the string need to be escaped.) Depending on the type of URL, it might also be possible to write these characters as URL-escapes (e.g. url(open%28parens) or url(close%29parens)) as described in [URL].

I think the correct approach would be to escape the URL when injecting it by escaping and wrapping the result of the require-call.

I'll try to put a pull-request together tonight. If anyone needs a temporary solution now, you can use this skeleton-loader:

{
    loader: 'skeleton-loader',
    options: {
        procedure: (content) => {
            return content.replace(
                /(?:\\")?" ?\+ ?require\((.+?)\) ?\+ ?"(?:\\")?/g,
               '\\"" + require($1).replace(/^"/, "").replace(/"$/, "").replace(/"/g, "\\\\\\"").replace(/\\n/g, "\\\\n") + "\\"'
            )
        },
    },
}

It should be placed right after css-loader. I haven't tested other cases than my own (inline svgs with svg-url-loader), so use it with caution. It should strip any wrapping quotes returned by the require, escape quotes and newlines, and handle both wrapped and unwrapped urls.

@alexander-akait
Copy link
Member

@kgram PR welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants