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

url containing closing parenthesis are parsed incorrectly #23773

Closed
2 of 15 tasks
snocorp opened this issue Aug 21, 2022 · 1 comment · Fixed by #23774
Closed
2 of 15 tasks

url containing closing parenthesis are parsed incorrectly #23773

snocorp opened this issue Aug 21, 2022 · 1 comment · Fixed by #23774

Comments

@snocorp
Copy link

snocorp commented Aug 21, 2022

🐞 Bug report

Command (mark with an x)

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

Yes, the previous version in which this bug was not present was: @angular-devkit/build-angular 14.1.1

Description

When loading a CSS file containing a url that contains a closing parenthesis ) the regular expression fails to capture the full value in the url. In addition if the value contains another url expression (inside the svg markup) then the parser will try to resolve that url as well.

🔬 Minimal Reproduction

  1. Clone https://github.com/snocorp/angular-url-issue (Base angular app with mapbox-gl installed for CSS)
  2. Run npm install
  3. Run ng build (or ng build --verbose to see a trace of the error)

🔥 Exception or Error


./node_modules/mapbox-gl/dist/mapbox-gl.css - Error: Module Error (from ./node_modules/postcss-loader/dist/cjs.js):
/Users/dave/dev/snocorp/angular-url-issue/url-issue/node_modules/mapbox-gl/dist/mapbox-gl.css:1:28378: Can't resolve '%23clip' in '/Users/dave/dev/snocorp/angular-url-issue/url-issue/node_modules/mapbox-gl/dist'

🌍 Your Environment


     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 14.1.3
Node: 16.15.1
Package Manager: npm 6.14.15
OS: darwin arm64

Angular: 14.1.3
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1401.3
@angular-devkit/build-angular   14.1.3
@angular-devkit/core            14.1.3
@angular-devkit/schematics      14.1.3
@schematics/angular             14.1.3
rxjs                            7.5.6
typescript                      4.7.4

Anything else relevant?

This is a regression caused by #23691

The current regular expression is /url(?:\(\s*['"]?)(.*?)(?:['"]?\s*\))/g which is problematic because it can find a closing parenthesis without finding a closing quote, since the closing quote is optional.

A regular expression that solves the problem and still fixes #23680 is something like /url(?:\(\s*(['"]?))(.*?)(?:\1\s*\))/. This uses a back reference to ensure that the quotes are the same. This does mean there is an extra capturing group and so const originalUrl = match[1]; would need to be const originalUrl = match[2]; instead.

A small-ish test case:

background-image:url("data:image/svg+xml;charset=utf-8,<svg><mask id='clip'><rect x='0' y='0' width='100%' height='100%' fill='white'/></mask> <g> <circle mask='url(%23clip)' cx='11.5' cy='11.5' r='9.25'/> <use xlink:href='#text' mask='url(#clip)'/> </g></svg>")
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Aug 22, 2022
PR angular#23691 introduced a regression that caused paranthesis in url not to be handled correctly.

This change correct this behaviour and adds a test case to valid this.

Closes angular#23773
dgp1130 pushed a commit that referenced this issue Aug 22, 2022
PR #23691 introduced a regression that caused paranthesis in url not to be handled correctly.

This change correct this behaviour and adds a test case to valid this.

Closes #23773
dgp1130 pushed a commit that referenced this issue Aug 22, 2022
PR #23691 introduced a regression that caused paranthesis in url not to be handled correctly.

This change correct this behaviour and adds a test case to valid this.

Closes #23773

(cherry picked from commit 147f8c3)
dgp1130 pushed a commit that referenced this issue Aug 22, 2022
PR #23691 introduced a regression that caused paranthesis in url not to be handled correctly.

This change correct this behaviour and adds a test case to valid this.

Closes #23773

(cherry picked from commit 147f8c3)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants