Remote source mapping css files breaks when using url-escaped data-urls

Description

Css background-images can be defined with data-urls. Most of the time these are base64 encoded, but there are cases where they are url-escaped.

One such example is the inline svg output by the svg-gradient function in LESS, which returns an inline url-encoded svg which itself contains a fill="url(#gradient)" attribute at some point. (see: http://lesscss.org/functions/#misc-functions-svg-gradient)

Currently Enyo Minifier's concatCss function does not correctly extract data-urls and breaks on such nested url() statements.

Environment

Any

Activity

Show:
Lis Hammel
March 24, 2015, 6:40 PM

update: ttps://github.com/enyojs/enyo/pull/1032 has gone through first review and is ready for second review/merging

Ruben Vreeken
March 24, 2015, 7:24 PM

Thanks for the update. I added my signoff for the commit.

Cole Davis (Enyo Team)
April 1, 2015, 9:17 PM
Edited

Hi , thanks for all of your recent activity both with questions, insight, suggestions and PR's. We definitely appreciate it.

Now, about this particular issue I wanted to share some thoughts I had and some testing I did.

First, I'd like to start by saying that you've found a real issue here and its surprising to me that its been this long without it coming up. Lets break down what the implications and true areas of concern are to be able to have an appropriate solution.

In CSS specs (all that I am aware of that support URI linking) there is no inherent support for nested URI's. The only case where this can be an issue is if you are doing these specific things:

1. Inlining SVG (as far as I know, there are no other safe encodings that support URI references in plain utf8) - in other words, inlining any other data structure must be base64 encoded to ensure it is transferable over HTTP so we can ignore those cases
2. The inline SVG is utf8 encoded (standard URI, escaped or not) thus leaving us with the potential case of encountering IRI links (URIs) in the form `url(...)`

So, we need to handle the standard case (where there is no concern of nested IRI links) and the inline SVG utf8 encoded case. Clearly, the current implementation does not handle that second scenario properly.

Thank you for submitting a well-thought-out solution that does appear to successfully navigate the problem at hand. Your solution, however, covers some things that are not necessarily within the scope of what we're trying to accomplish with the method you're modifying. It simply needs to rewrite relative URI paths to local files. That's it. We don't want it to do a lot of fancy re-writing of bad CSS or even error handle them - we will leave that to a complete implementation of a CSS parser and we won't unintentionally modify output code beyond what is required (or basic, such as changing " -> '). While it is impressive what your function handles, it will accept and modify invalid CSS cases and we simply don't need to consider them to solve the problem at hand.

But, before I dive into solution discussion I wanted to bring up that in your original description you linked us (http://lesscss.org/functions/#misc-functions-svg-gradient) to the svg-gradient function from Less - but that method always returns base64. And base64 won't ever be an issue. So you must be arbitrarily substituting the base64 for utf8 uri-encoded output or perhaps an older version of Less didn't output base64? Either way, it appears using base64 would have solved the problem without a change - but that doesn't mean it isn't a real issue

So, for the solution I think a small tweak to the current handler for search-and-replace using RegEx probably will suffice even if at first glance it appears that it can't, or would be sub-optimal. To see my alternative test go to https://github.com/enyojs/enyo/compare/2.5-upkeep...ENYO-948-coledavis. For this little change I was able to confirm that it produces identical output in known cases (that were already working) and I arbitrarily tested against several others to ensure that it would handle them properly. As far as I can telll, this does the trick with minimal changes to the existing code (which is being replaced in 2.6 so we will ensure it handles the case properly in the future).

Note, as far as I know the only thing that may need to be tweaked in my proposal is the inclusion of any valid URI characters I missed in the accept-set of the RegEx. Not positive that is all-inclusive but definitely handles the majority case.

Why does it work?

Well, even though we're matching nested URI links its not actually recursive (which is where it
would be assumed JavaScript RegEx would fall over) because we don't need to match the outer URI link. Any inline SVG must be flagged as data so we can immediately exclude matching that case. Same with anything beginning with http, https or file protocol (//). This means we can cleanly catch the one-deep nested URI link (you cannot nest beyond this level by any spec).

From there it is simply matching the content to the end.

Will it handle bad syntax? No. But it doesn't need to. It isn't a parser. Its a relative-uri-rewriter only. If I could would I rewrite the whole thing? Yes! Funny thing is...I already am...but to keep 2.5 running we can submit a change like this.

Hopefully this relatively small change will suffice. Please let me know if this does not cover the case you were running into, and, if for some reason it doesn't, please provide a solid test case so we can verify on both ends.

Oh, and an example output...

When using the example from Less but converted from base64 to uri-encoded:

The outcome from the minify.sh in master (obviously, wrong):

The outcome using my update:

Ruben Vreeken
April 2, 2015, 12:08 PM

Hey Cole,

Thank you for taking the time to dive into this issue in such detail. You make a good point that there is no need to handle malformed css.

The Less version I was using when I ran into this bug didn't generate base64-encoded inline svg's. Since this problem seemed to suddenly appeared out of nowhere I suspect there has probably been a narrow window of one or more versions of Less that changed the encoding of inline svg and the problem has been addressed by the Less devs since.

Anyways, with the reduced problem space you describe, I think that negative lookahead in your regex is indeed a better solution. You sir, are one seriously smart cookie.

Lis Hammel
April 25, 2016, 7:37 PM

closing as will not fix. Not a priority for resolution in 2.7.

Assignee

Screener (Enyo Team)

Reporter

Ruben Vreeken

Labels

None

External issue ID

None

Tango Test Run Elements

None

Old Issue Key

None

Components

Affects versions

Priority

None
Configure