Opened 7 weeks ago
Last modified 6 weeks ago
#65738 new enhancement
Introduce a CSS tokenizer.
| Reported by: | dmsnell | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | dev-feedback |
| Cc: | Focuses: |
Description (last modified by )
Many operations within WordPress need to process CSS content, whether inline style elements with their list of declarations, STYLE element CSS, or theme.json-like CSS-inside-JSON.
Currently, code tends to run naive PCRE patterns against the CSS. This presents a number of common problems related to string values, basic tokenization, and the like.
WordPress could use at least a CSS lexer which would allow code to speak at the level of language tokens instead of repeating the same overly-simplific parsers in different places, leaving opportunity for parsing differentials or misparses.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Thanks for opening this, @dmsnell. I spent some time auditing the current state of CSS handling in
trunk(7.1-beta3), confirmed the problem, and quantified it. There are twelve ad-hoc CSS parsers insrc/today, and I was able to reproduce misparses in four of them againsttrunk(7.1-beta3).safecss_filter_attr()(source:trunk/src/wp-includes/kses.php#L2646) splits on;withexplode(), so any semicolon inside a string orurl()cuts a declaration in half. Running the real function, not a re-implementation:font-family:foo\;bar => bar background-image:url("a;b.png") => b.png") font-family:"Foo;bar:baz",serif => font-family:"Foo COLOR:red => '' color:/* hi */red => '' font-family:"Ben & Jerry" => '' color:red;this-is-not-css-at-all => color:red;this-is-not-css-at-allThe first two are the worst: the property name is discarded and a fragment of the value survives as though it were a declaration. Cases three and four are the inverse — valid CSS rejected, because property names are compared case-sensitively and the
/\*in the character blacklist kills any declaration containing a comment.The last case has a structural cause at source:trunk/src/wp-includes/kses.php#L2934 — when a fragment contains no colon,
$foundis set totrueunconditionally and thesafe_style_cssallowlist is never consulted. That is also why the third case truncates rather than erroring: the tail is admitted unvalidated.That third case matters beyond truncation.
font-family:"Foois an unterminated string, and source:trunk/src/wp-includes/blocks/post-featured-image.php#L49 concatenates four separately-filtered values, so an unterminated string from one can swallow the next. It is the same concatenation hazard already documented for</styleinWP_REST_Global_Styles_Controller::validate_custom_css()(source:trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php#L683), which is the one place in core that scans rather than pattern-matches.WP_Theme_JSON::process_blocks_custom_css()(source:trunk/src/wp-includes/class-wp-theme-json.php#L2048) splits nested CSS withexplode( '&' )and thenexplode( '{' ), which breaks on an ampersand in a string, aurl(), or a comment:content: "a & b"; color: red; => :root :where(.wp-x){content: "a}:root :where(.wp-x){b"; color: red;} /* & */ color: red; => :root :where(.wp-x){/*}:root :where(.wp-x){*/ color: red;} @media (min-width:600px){…} => :root :where(.wp-x@media (min-width: 600px) ){color: red;} & p { & b { color: red } } => :root :where(.wp-x p){}:root :where(.wp-x b){color: red;}The
count( $part ) !== 2guard means a second level of nesting is silently dropped, and an at-rule is folded into the selector rather than rejected.Unrelated live bug found while auditing:
WP_Interactivity_API::merge_style_property()(source:trunk/src/wp-includes/interactivity-api/class-wp-interactivity-api.php#L1180) doeslist( $name, $value ) = explode( ':', $style_assignment ), so every URL value is truncated at the scheme colon —background-image:url(https://example.com/a.png)merges tobackground-image:url(https;…. A colon-less fragment also emits anUndefined array key 1warning. Happy to open that as its own ticket; it stands independent of how this one resolves.On scoping: the twelve call sites split cleanly in two, which I think bears on how much of the stack this ticket should cover. Six need nothing more than a token stream -
safecss_filter_attr(), the Interactivity API merge above, the%[\\\(&=}]|/\*%blacklist copied out of kses into source:trunk/src/wp-includes/block-supports/layout.php#L89, thefont sizepreg_replacein source:trunk/src/wp-includes/block-supports/typography.php#L339, thevar()matcher at source:trunk/src/wp-includes/class-wp-theme-json.php#L5795, and source:trunk/src/wp-includes/blocks/gallery.php#L73, whose comment states outright that its regex was "borrowed fromsafecss_filter_attr". The other six need{}structure on top:process_blocks_custom_css(), thepreg_match( '#</?\w+#' )guard at source:trunk/src/wp-includes/block-supports/custom-css.php#L53, both custom-CSS validators, and the selector handling instates.phpandWP_Style_Engine_CSS_Rule.A lexer alone resolves the first six outright and is a prerequisite for the rest, so "at least a CSS lexer" reads to me as the right decomposition rather than a compromise.
Worth noting that the two related tickets, #46197 and #46498, were both closed in 5.8 by adding regex rather than removing it — the recursive
\b(?:var|calc|min|max|minmax|clamp|repeat)(\((?:[^()]|(?1))*\))pattern now insafecss_filter_attr(). Every new CSS function costs another pattern, and each one widens the gap between what core thinks it parsed and what a browser will.Two things worth settling before any code exists.
romainmenke/css-tokenizer-tests, the obvious conformance corpus, declares no license on GitHub; the PHP Toolkit ships a ~90KB derivedcss-test-cases.json, so it would be good to know how that provenance was cleared before core depends on it. And separately, fixing theCOLOR:red,color:/* hi */redand"Ben & Jerry"cases means admitting CSS thatsafecss_filter_attr()currentlyrejects — a behaviour change on a security-adjacent function that likely deserves its own
review rather than riding along with the tokenizer.
Proposed scope:
WP_CSS_Tokenizer— a cursor over CSS Syntax Level 3 tokens, with no setters and noget_updated_css(). With no mutation surface it cannot corrupt a document, and it keeps parsing separate from policy: nosanitize(), novalidate(), no allowlists, no at-rule semantics. That also makes the name honest and leaves…_Processorfree for a rewriting layer if and when real consumers need one.foo\;barandfoo\}barcases.safecss_filter_attr()against the existing 22 assertions intests/phpunit/tests/kses.php, so a parsing regression stays bisectable to one consumer.Open questions I would want answered before starting: whether v1 stops at tokens or also lands a component-value/block layer for the second group; whether
url("…")tokenizing as function-token plus string-token rather than url-token warrants a helper so consumers do not each rediscover it; and whether non-UTF-8 input should yield replacement characters or refuse to construct.Happy to open a PR upon clarification.