Skip to content

Conversation

@timint
Copy link

@timint timint commented Jul 10, 2021

file_exists() will return false for a file on a remote server.
This method could need a rewrite to handle URLs e.g. Google Fonts.

@Krinkle
Copy link
Member

Krinkle commented Aug 10, 2021

See also:

In short, I worry that this would open up evaluation of external resources if mis-configured. However, I think I see something I previously missed - there is a point in the code earlier in the stack, in the Import constructor where we should treat these URLs as "literal CSS imports" much earlier so that they never end up in the parse code path in the first place.

Krinkle pushed a commit that referenced this pull request Aug 10, 2021
The regex that was meant to handle this already accounted for a lot
of edge cases, such as `/css` endpoints without `.css`, and query
parameters like `/css?foo` or `/css;foo`, all to carefully avoid
matching things like `foocss.less` or `foo.css.bar.less`.

However can still be any number of CSS resources served from URLs
that don't match that pattern.

Instead of trying to match the path segment, just match the start of
the URL, which should take care of any edge cases, given that we don't
allow importing remote Less code for compilation anyway.

Fixes #27.
Closes #28.
Closes #68.
@Krinkle Krinkle closed this in #73 Aug 10, 2021
Krinkle added a commit that referenced this pull request Aug 10, 2021
…73)

The regex that was meant to handle this already accounted for a lot
of edge cases, such as `/css` endpoints without `.css`, and query
parameters like `/css?foo` or `/css;foo`, all to carefully avoid
matching things like `foocss.less` or `foo.css.bar.less`.

However can still be any number of CSS resources served from URLs
that don't match that pattern.

Instead of trying to match the path segment, just match the start of
the URL, which should take care of any edge cases, given that we don't
allow importing remote Less code for compilation anyway.

Fixes #27.
Closes #28.
Closes #68.

Co-authored-by: Max Semenik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants