Fix unquoted and invalid url() handling

[meziantou]

Types of Changes

Prerequisites

Please make sure you can check the following two boxes:

• I have read the CONTRIBUTING document
• My code follows the code style of this project

Contribution Type

What types of changes does your code introduce? Put an x in all the boxes that apply:

• Bug fix (non-breaking change which fixes an issue, please reference the issue id)
• New feature (non-breaking change which adds functionality, make sure to open an associated issue first)
• Breaking change (fix or feature that would cause existing functionality to change)
• My change requires a change to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

Description

Two commits fixing url() handling. The first fixes where a url token ends; the second fixes what happens when one is invalid. Both were found from the same symptom — stylesheets silently getting a wrong value instead of the right one or none at all.

Behaviour throughout was validated against Chrome's CSSOM rather than from the spec alone, over a 30-case matrix. After both commits there is no remaining divergence in that matrix.

---

1. Unquoted url() values were truncated at ;, { or }

CssTokenizer.ContentFrom re-scans the raw source to recover a declaration value or an at-rule prelude, and breaks at the first ;, { or }. It special-cased quoted strings but knew nothing about url tokens, where all three characters are legal content.

a { background-image: url(data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=) }

parsed to url("data:image/svg+xml") — cut at the first semicolon. The remainder failed to re-tokenize as a declaration and was silently dropped, so a round-trip through CssText destroyed the asset. Unquoted data URIs are emitted by every major bundler for icon sprites and inlined SVG, so this affects a lot of real stylesheets.

The existing test CssSheetWithDataUrlAsBackgroundImage missed it because it uses the quoted form, which took the string branch and round-tripped fine.

Through GetArgument the same flaw discarded whole rules — a @supports condition containing url(a;b) lost its entire rule. Escaped separators were mangled too: url(a\;b) produced url("a\\"). @import was not affected; CreateImport takes the href straight off the token stream.

Three subtleties from the browser shaped the fix:

• The ident must be immediately followed by (. myurl(, -url(, local-url( and url\t( are ordinary function tokens where ; does terminate the declaration. A plain substring match for url( would have introduced a new bug; hence the ident-boundary guard.
• url( "a)b" ) is a function token, not a url token — the quoted string wins and the ) inside it does not close it. So the scan skips whitespace after ( and defers to the existing string handling on a quote.
• Bad-url cases still consume through the matching ), so a single "consume to unescaped )" rule extracts the correct span in every case.

2. Invalid url() values produced garbage instead of failing

The root cause was that a bad url had no way to be reported as a failure.

CssUriParser.Bad() returned a CssUrlValue built from whatever characters it had scanned past, so an invalid url produced a plausible-looking but wrong value instead of failing:

input	before	Chrome / now
url(a b)	url("ab")	declaration dropped
url(a(b)	url("a(b)")	declaration dropped
url(javascript:alert(1))	url("javascript:alert(1)")	declaration dropped
url()	declaration dropped	url("")

Bad() now returns null and ParseUri rewinds the source. The rewind matters: merely consuming the bad url let background: url(a b) red silently re-parse as background: red instead of being dropped.

ParseUri also never consumed the ) of an empty url(), leaving the source mid-value so the declaration was rejected. url() is valid and means the empty URL.

At the sheet level, CssTokenizer.NewUrl accepted a bad parameter and ignored it, so no bad-url token could ever exist and UrlBad's scanned-over characters became the url's content — @import url(a b) imported the garbage href a b). A BadUrl token type now carries the distinction (both CssTokenType and CssToken are internal, so this is not a public API change), and UrlBad discards the remnants it consumes.

Per the spec, EOF ends a url token rather than invalidating it, so the two EOF paths that flagged bad no longer do — @import url(abc still imports abc, as Chrome does.

Rule-level consequences, all matching Chrome:

• @import with a bad url is dropped instead of importing a garbage href.
• @namespace with a bad url is dropped. Fixing this forced a decision on the string form, since one condition governs both: @namespace accepted only a url token, so @namespace x "http://foo" silently produced an empty namespace URI. It now accepts a string as the spec requires. This is the one change here that is not strictly about url() — it was unavoidable, and it is browser-validated.

[FlorianRappl]

Is this final?

[FlorianRappl]

LGTM!