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

unescaped urls are being passed to downstream loaders #1048

Closed
aholtkamp opened this issue Jan 27, 2020 · 5 comments
Closed

unescaped urls are being passed to downstream loaders #1048

aholtkamp opened this issue Jan 27, 2020 · 5 comments

Comments

@aholtkamp
Copy link

  • Operating System: Linux Mint 18.3
  • Node Version: 10.16.0
  • NPM Version: Yarn 1.19.2
  • webpack Version: 4.41.5
  • css-loader Version: 3.4.2

Code

The "css-loader" processes our css files, our custom loader processes svg files.

Example CSS processed by the css-loader

div {
  background-image: url("my-special-stuff.svg?color=rgba(0%2C%200%2C%200%2C%200.5)&foo=bar");
}

Code of the custom downstream loader processing the svg (and its query parameters):

const loaderUtils = require("loader-utils");

module.exports = function(originalData) {
  let transformedData = originalData;
  if (this.resourceQuery) {
    const queryParams = loaderUtils.parseQuery(this.resourceQuery);
    // do something with the queryParams...
    // ...
  }
  return transformedData;
}

Expected Behavior

When an url contains a query string the query string should not be changed if the css-loader replaces the url statement.

Before the changes in #1016:
this.resourceQuery was "?color=rgba(0%2C%200%2C%200%2C%200.5)&foo=bar"

The resulting queryParams has looked as follows:

{
  "color": "rgba(0, 0, 0, 0.5)",
  "foo": "bar"
}

Actual Behavior

The uri components are decoded before being passed to the corresponding loader.

this.resourceQuery is now "?rgba(0, 0, 0, 0.5)"

As loaderUtils.parseQuery splits by ("&" and ",") this leads to:

{
  "color": "rgba(0",
  "0": true,
  "0.5)": true,
  "foo": "bar"
}
@alexander-akait
Copy link
Member

Please provide configuration, you can setup loader before css-loader or create postcss plugin

@aholtkamp
Copy link
Author

Nothing special about the loader configuration:

{
  module: {
      rules: [
        {
          test: /\.svg$/,
          loader: require.resolve("../loaders/CustomSvgLoader"),
        },
        {
          test: /\.css$/,
          use: [
            {
              loader: MiniCssExtractWithCleanupPlugin.loader,
            },
            {
              loader: "css-loader",
            },
          ],
        },
      ],
  },
}

@alexander-akait
Copy link
Member

Thanks i will look at that in near future

@alexander-akait
Copy link
Member

Here the interesting use case, remember what css-loader convert CSS to JS what allow to use CSS inside JS files. Any url converting to require/import, let's investigate that.

Let's look on the specification - https://drafts.csswg.org/css-values-3/#urls.

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].

According the specification developers can escape some characters in URL.

But when you use require you need use real filename without escaped characters, because require('./test?foo=rgba(0, 0, 0, 0.5)') and ./test?foo=rgba(0%2C%200%2C%200%2C%200.5) is difference files.

Other example, how web servers work:

index.html

<style>
  div {
    background: url("./image.php?foo=rgba(0%2C%200%2C%200%2C%200.5)")
  }
</style>
<div>
  TEXT
</div>

index.js

const express = require('express');
const bodyParser = require('body-parser');
const url = require('url');
const querystring = require('querystring');

let app = express();

app.use(bodyParser.urlencoded({ extended: false }));
app.use(bodyParser.json());

// Function to handle the root path
app.get('*', async function(req, res) {
  // You will see `{ foo: 'rgba(0, 0, 0, 0.5)' }`
  console.log(req.query);

  res.send('Hello World!');
});

let server = app.listen(8080, function() {
  console.log('Server is listening on port 8080');
});

Let's look how ES import work:

index.html

<script type="module" crossorigin="use-credentials">
  import foo from './index.js?color=rgba(0%2C%200%2C%200%2C%200.5)';
</script>

index.js

console.log(import.meta);
const url = new URL(import.meta.url);

console.log(url);
// You will see `rgba(0, 0, 0, 0.5)`
console.log(url.searchParams.get('color'));

export default 'HERE';

server.js

var express = require('express')
var serveStatic = require('serve-static')

var app = express()

app.use(serveStatic('.', { 'index': ['index.html', 'default.htm'] }))
app.listen(3000)

Other example:

index.html

<script type="module" crossorigin="use-credentials">
  import foo from './index%20test.js';
</script>

A browser request index test.js file.

So we should covert all escaped characters to real characters when we do request.

My recommendations:

  • use loader options, you can use the setup loader based on issuer (file from where was requested)
  • write own parser for query
  • implement same logic using postcss plugin

Also loaderUtils.parseQuery will deprecated for webpack@5 - we move all logic inside webpack core.

Feel free to leave feedback

@aholtkamp
Copy link
Author

Thanks for the detailed analysis, I appreciate that.

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

No branches or pull requests

2 participants