Custom CSS: Relax CSS validation rules

[sirreal]

Relax the CSS validation for custom CSS and global styles to allow any text that will not break an inline <style> tag container.

STYLE tags are processed using the "generic raw text parsing algorithm." They contain raw text up until a matching closing tag.

To do:

• Review customizer behavior

See https://html.spec.whatwg.org/multipage/parsing.html#generic-raw-text-element-parsing-algorithm

Trac ticket: https://core.trac.wordpress.org/ticket/64418

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[dmsnell]

it might be worth calling next_token() here to verify that there are no further tokens, as we expect none. if we get one, it means that something inside of $this->type_attr broke out of the tag and potentially out of the style.

in fact, if we do this before setting the modifiable text we can ensure those attributes don’t mess with the STYLE contents

[dmsnell]

we can also leverage wp_kses_hair() (when fixed to rely on the HTML API) and transfer the attributes, but this is tantamount to my suggestion above

[westonruter]

I don't think this is malformed type_attr is something we need to worry about. I did a quick search to see if it is being abused, and I don't see it. If someone were to do something bad with the attribute, then this would be _doing_it_wrong. I've actually been thinking lately that we might want to eliminate the $type_attr entirely as it is obsolete now with HTML5.

[dmsnell]

thanks. I didn’t look into it any further. this appears different in nature as well from some of our other HTML5 theme-support checks: we’re not changing the markup structure, so maybe we can remove it.

there is a case where this could lead to styling change, and that’s when someone is targeting the STYLE element based on the presence of the type attribute, such as style[type~=css], which I think would match or fail to match based on the presence of this attribute.

so maybe we propose dropping it entirely and always omit the type_attr since there is no place where having it with the privately-set value of text/css would lead to different interpretations than not having it.

[westonruter]

there is a case where this could lead to styling change, and that’s when someone is targeting the STYLE element based on the presence of the type attribute, such as style[type~=css], which I think would match or fail to match based on the presence of this attribute.

How meta 😄 I'm not aware of style rules having selectors which target other STYLE tags, but anything is possible. This seems extremely unlikely, however.

so maybe we propose dropping it entirely and always omit the type_attr since there is no place where having it with the privately-set value of text/css would lead to different interpretations than not having it.

I would endorse removing the type attribute since stylesheets are CSS by default, though this wouldn't have to be done as part of this PR. The same could be done for SCRIPT tags being JavaScript as well, but that's out of scope here. The $type_attr is a vestige of the XHTML/HTML4 past.

[dmsnell]

I'm not aware of style rules having selectors which target other STYLE tags

I guess I said styling change but was also thinking more about document.querySelector(). Either way, I think we are in agreement.

Good point about the <script> type as well, and on doing it in another change.

[dmsnell]

Tasked in Core-64428

[sirreal]

As I explored the space, this PR grew in scope. There are at least two concerns that I plan to split up:

• Use the HTML API to correctly produce STYLE tags across the codebase.
• Relax CSS "validation," (requires additional safety on STYLE tags).

[sirreal]

YES! KSES indeed breaks this as can be seen in the multisite tests:

1) WP_REST_Global_Styles_Controller_Test::test_update_allows_valid_css_with_more_syntax
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '@property --animate {
-	syntax: "<custom-ident>";
+	syntax: "";
 	inherits: true;
 	initial-value: false;
 }'

That's almost certainly a missing unfiltered_html cap triggering KSES and strip_html.