-
-
Notifications
You must be signed in to change notification settings - Fork 606
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
fix: ignore invalid URLs (url()
)
#663
Conversation
@@ -16,7 +16,7 @@ function getEvaluated(output, modules) { | |||
return require("../lib/url/escape"); | |||
if(module.indexOf("-!/path/css-loader!") === 0) | |||
module = module.substr(19); | |||
if(modules && modules[module]) | |||
if(modules && module in modules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why in
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evilebottnawi: To allow for falsy modules such as { './module': null }
to still be resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kgram thanks for clafiry 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One question, see above.
@evilebottnawi: Do you know what is wrong with the travis-build? It looks like the processes were aborted halfway through when looking at the logs. |
@kgram do not pay attention (legacy), main build doing in |
url()
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kgram Thx for taking care of this 👍
Released in v0.28.9 🎉 |
This Pull Request updates dependency [css-loader](https://github.com/webpack-contrib/css-loader) from `^0.28.11` to `^1.0.0` <details> <summary>Release Notes</summary> ### [`v1.0.0`](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#​100httpsgithubcomwebpack-contribcss-loadercomparev02811v100-2018-07-06) [Compare Source](webpack-contrib/css-loader@v0.28.11...v1.0.0) ##### BREAKING CHANGES * remove `minimize` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`cssnano`](https://github.com/cssnano/cssnano) or use [`optimize-cssnano-plugin`](https://github.com/intervolga/optimize-cssnano-plugin) plugin * remove `module` option, use `modules` option instead * remove `camelcase` option, use `camelCase` option instead * remove `root` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * remove `alias` option, use [`resolve.alias`](https://webpack.js.org/configuration/resolve/) feature or use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * update `postcss` to `6` version * minimum require `nodejs` version is `6.9` * minimum require `webpack` version is `4` #### [0.28.11](webpack-contrib/css-loader@v0.28.10...v0.28.11) (2018-03-16) ##### Bug Fixes * **lib/processCss:** don't check `mode` for `url` handling (`options.modules`) ([#​698](`https://github.com/webpack-contrib/css-loader/issues/698`)) ([c788450](webpack-contrib/css-loader@c788450)) #### [0.28.10](webpack-contrib/css-loader@v0.28.9...v0.28.10) (2018-02-22) ##### Bug Fixes * **getLocalIdent:** add `rootContext` support (`webpack >= v4.0.0`) ([#​681](`https://github.com/webpack-contrib/css-loader/issues/681`)) ([9f876d2](webpack-contrib/css-loader@9f876d2)) #### [0.28.9](webpack-contrib/css-loader@v0.28.8...v0.28.9) (2018-01-17) ##### Bug Fixes * ignore invalid URLs (`url()`) ([#​663](`https://github.com/webpack-contrib/css-loader/issues/663`)) ([d1d8221](webpack-contrib/css-loader@d1d8221)) #### [0.28.8](webpack-contrib/css-loader@v0.28.7...v0.28.8) (2018-01-05) ##### Bug Fixes * **loader:** correctly check if source map is `undefined` ([#​641](`https://github.com/webpack-contrib/css-loader/issues/641`)) ([0dccfa9](webpack-contrib/css-loader@0dccfa9)) * proper URL escaping and wrapping (`url()`) ([#​627](`https://github.com/webpack-contrib/css-loader/issues/627`)) ([8897d44](webpack-contrib/css-loader@8897d44)) #### [0.28.7](webpack-contrib/css-loader@v0.28.6...v0.28.7) (2017-08-30) ##### Bug Fixes * pass resolver to `localsLoader` (`options.alias`) ([#​601](`https://github.com/webpack/css-loader/issues/601`)) ([8f1b57c](webpack-contrib/css-loader@8f1b57c)) #### [0.28.6](webpack-contrib/css-loader@v0.28.5...v0.28.6) (2017-08-30) ##### Bug Fixes * add support for aliases starting with `/` (`options.alias`) ([#​597](`https://github.com/webpack/css-loader/issues/597`)) ([63567f2](webpack-contrib/css-loader@63567f2)) #### [0.28.5](webpack-contrib/css-loader@v0.28.4...v0.28.5) (2017-08-17) ##### Bug Fixes * match mutliple dashes (`options.camelCase`) ([#​556](`https://github.com/webpack/css-loader/issues/556`)) ([1fee601](webpack-contrib/css-loader@1fee601)) * stricter `[@import]` tolerance ([#​593](`https://github.com/webpack/css-loader/issues/593`)) ([2e4ec09](webpack-contrib/css-loader@2e4ec09)) #### [0.28.4](webpack-contrib/css-loader@v0.28.3...v0.28.4) (2017-05-30) ##### Bug Fixes * preserve leading underscore in class names ([#​543](`https://github.com/webpack/css-loader/issues/543`)) ([f6673c8](webpack-contrib/css-loader@f6673c8)) #### [0.28.3](webpack-contrib/css-loader@v0.28.2...v0.28.3) (2017-05-25) ##### Bug Fixes * correct plugin order for CSS Modules ([#​534](`https://github.com/webpack/css-loader/issues/534`)) ([b90f492](webpack-contrib/css-loader@b90f492)) #### [0.28.2](webpack-contrib/css-loader@v0.28.1...v0.28.2) (2017-05-22) ##### Bug Fixes * source maps path on `windows` ([#​532](`https://github.com/webpack/css-loader/issues/532`)) ([c3d0d91](webpack-contrib/css-loader@c3d0d91)) #### [0.28.1](webpack-contrib/css-loader@v0.28.0...v0.28.1) (2017-05-02) ##### Bug Fixes * allow to specify a full hostname as a root URL ([#​521](`https://github.com/webpack/css-loader/issues/521`)) ([06d27a1](webpack-contrib/css-loader@06d27a1)) * case insensitivity of [@​import] ([#​514](`https://github.com/webpack/css-loader/issues/514`)) ([de4356b](webpack-contrib/css-loader@de4356b)) * don't handle empty [@​import] and url() ([#​513](`https://github.com/webpack/css-loader/issues/513`)) ([868fc94](webpack-contrib/css-loader@868fc94)) * imported variables are replaced in exports if followed by a comma ([#​504](`https://github.com/webpack/css-loader/issues/504`)) ([956bad7](webpack-contrib/css-loader@956bad7)) * loader now correctly handles `url` with space(s) ([#​495](`https://github.com/webpack/css-loader/issues/495`)) ([534ea55](webpack-contrib/css-loader@534ea55)) * url with a trailing space is now handled correctly ([#​494](`https://github.com/webpack/css-loader/issues/494`)) ([e1ec4f2](webpack-contrib/css-loader@e1ec4f2)) * use `btoa` instead `Buffer` ([#​501](`https://github.com/webpack/css-loader/issues/501`)) ([fbb0714](webpack-contrib/css-loader@fbb0714)) ##### Performance Improvements * generate source maps only when explicitly set ([#​478](`https://github.com/webpack/css-loader/issues/478`)) ([b8f5c8f](webpack-contrib/css-loader@b8f5c8f)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
What kind of change does this PR introduce?
Closes #659
Did you add tests for your changes?
Test added for dependency exporting an empty object and null.
If relevant, did you update the README?
Not relevant
Summary
#627 introduced an assumption about all dependencies exporting strings. To avoid including font-files, some users use
null-loader
, which effectively exports an empty object.This PR checks for non-strings, restoring previous functionality for all non-string dependencies.
While this PR would enable previous usage patterns, I hope someone can come up with a better solution. The users are relying on
url([object Object])
being invalid CSS to avoid making requests for font files. This seems fragile in the long run.Ideally, a null/undefined value (or what we here know should be a null value,
{}
) should result in no request being made. As far as I know, this requires the entireurl(...)
block be replaced withnone
, and I'm still not sure this would be valid at all times. This would be a complicated solution.Does this PR introduce a breaking change?
No.