Repository navigation
Conversation
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
6ff95c3 to
9f2f45b
Compare
| /* | ||
| * Replace all instances of the ASCII case-insensitive match of "</style" | ||
| * when followed by whitespace or "/" or ">", by using a CSS Unicode | ||
| * escape sequence for the "s" (or the "S"). | ||
| * | ||
| * CSS Unicode escape sequences will terminate at the first non-hexadecimal, | ||
| * so the `t` character in `style` ensures that a Unicode escape sequence | ||
| * like `\73t` is correctly interpreted as `st`. | ||
| */ | ||
| while ( $at < $end ) { | ||
| $tag_at = stripos( $text, '</style', $at ); | ||
| if ( false === $tag_at ) { | ||
| break; | ||
| } | ||
|
|
||
| if ( 1 !== strspn( $text, " \t\f\r\n/>", $tag_at + 7, 1 ) ) { | ||
| $at = $tag_at + 7; | ||
| continue; | ||
| } | ||
|
|
||
| $escaped .= substr( $text, $was_at, $tag_at - $was_at + 2 ); | ||
| $escaped .= 's' === $text[ $tag_at + 2 ] ? '\73' : '\53'; | ||
| $was_at = $tag_at + 3; | ||
| $at = $tag_at + 8; | ||
| } |
There was a problem hiding this comment.
Would there be an opportunity to factor this out into a common method to be reused by both escape_style_contents and escape_javascript_script_contents?
There was a problem hiding this comment.
My first intuition says that may be more trouble than it's worth, but I will take a look to consider what that would look like in practice.
These are effectively tailored processing algorithms for exactly the content we're working with.
The Script replacement is more complex because it needs to replace both <script and </script, so it scans to < and checks for matches at that point.
Style is a bit simpler because it only needs to find </style, so it uses stripos and can save some additional checks.
| * like `\73t` is correctly interpreted as `st`. | ||
| */ | ||
| while ( $at < $end ) { | ||
| $tag_at = stripos( $text, '</style', $at ); |
There was a problem hiding this comment.
Note the PHPStan error being flagged here. This is strange because stripos() definitely takes 3 params! https://www.php.net/manual/en/function.stripos.php
The loop built its result with `.=` on `substr()` spans, which copies the text about twice when a closer is escaped. One preg_replace_callback() pass writes it once. On a PCRE error the escaper returns null and set_modifiable_text() returns false without enqueuing an update. Escape step alone against trunk (e182141), tracing JIT: within 1% with no `</style`; 5% to 24% faster with one closer on sheets up to 18 KB and within 1% on larger ones (2.9 MB Tailwind with one closer: 192 µs in both, where the loop took 451 µs); 62% to 86% faster with eight `</stylesheet>` near misses, which only trunk escapes. End to end with no closer: within 0.2% of trunk. Output is byte-identical to the loop's on 200,000 random inputs and 171 CSS, HTML and JavaScript files, with PCRE JIT on and off, on PHP 7.4, 8.4 and 8.5. New test rows cover every tag name terminator and the STYLE start tag.
escape_javascript_script_contents() searched for SCRIPT tag names with its own loop. find_tag_names() returns the byte offset of each tag name which the tokenizer would read as a tag of a given name: after `</`, or also after `<` when start tags count, in any ASCII case, followed by whitespace, `/` or `>`. The SCRIPT escape builds its result from those offsets, and the STYLE escape and the closing tag checks in set_modifiable_text() can use the same search. Output equals trunk's on 200,000 random inputs, 180 files (mostly JavaScript and CSS) as they are and with inserted tags, and 2 MB runs of partial tags. Collecting the offsets first costs a little. Escape step alone against trunk, geometric mean over 11 core scripts and JSON documents, opcache tracing JIT / JIT off: - PHP 8.5.11, macOS, PCRE2 10.49: +8% / +10% with no tag, +5% / +4% with one tag. - PHP 8.4.25, Linux, PCRE2 10.44: +8% / +10% with no tag, +7% / +5% with one tag. - PHP 7.4.33, Linux, PCRE2 10.35, no JIT: +11% with no tag, +7% with one tag. On core's small JSON with no `<` the step takes 26 ns against 19 ns to 20 ns on PHP 8.4 and 8.5, and 63 ns against 46 ns on PHP 7.4. End to end (new processor, next_token(), set_modifiable_text(), get_updated_html()) every group is within 1.3% of trunk. Each variant ran in its own process: 8 rounds on PHP 8.5, 3 on PHP 8.4 and 7.4. A new test row covers every tag name terminator.
escape_style_contents() made one preg_replace_callback() pass. It now builds its result from the offsets find_tag_names() returns, with the same output. It has no error result, so set_modifiable_text() does not check for one. Most style sheets hold no `</style`, and there the search is faster than the regular expression. With a closer the result is built from substr() spans, which copies the text about twice where preg_replace_callback() writes it once, so escaping is slower. Escape step alone against the regular expression pass, geometric mean over 11 style sheets, opcache tracing JIT / JIT off: - PHP 8.5.11, macOS, PCRE2 10.49: -24% / -16% with no closer, +55% / +39% with one closer (Tailwind 2.9 MB: 208 to 484 µs). - PHP 8.4.25, Linux, PCRE2 10.44: -59% / -55% with no closer, +9% / +2% with one closer. - PHP 7.4.33, Linux, PCRE2 10.35, no JIT: -60% with no closer, +1% with one closer. End to end, with no closer: -6% / -2% on PHP 8.5, -17% / -15% on PHP 8.4, -16% on PHP 7.4. With one closer: +10% / +18%, +4% / +1%, +3%; Tailwind with one closer +52% / +32%, +5% / +16%, +14%. Each variant ran in its own process: 8 rounds on PHP 8.5, 3 on PHP 8.4 and 7.4. Output equals the regular expression pass's on 200,000 random inputs, 180 files as they are and with inserted tags, and 2 MB runs of partial tags.
The comment check, the check for SCRIPT text with a type other than JavaScript, and the check for IFRAME, NOEMBED, NOFRAMES and XMP text used preg_match(). The tag checks use find_tag_names(), and has_comment_closer() walks each `>` and reads the bytes before it. WP_HTML_Tag_Processor calls no PCRE function, so the speed of these checks does not depend on the PCRE2 build: on tinymce.min.js as comment text, trunk's check takes 66 µs on PHP 8.4 and 17 µs on PHP 8.5. End to end against trunk on text the checks accept, geometric mean, opcache tracing JIT / JIT off, on PHP 8.5.11 (macOS, PCRE2 10.49); PHP 8.4.25 (Linux, PCRE2 10.44); PHP 7.4.33 (Linux, PCRE2 10.35, no JIT): - Comment text, 22 sources: -27% / -20%; -54% / -46%; -46%. - SCRIPT with a non-JavaScript type, 11 sources: -28% / -18%; -30% / -19%; -12%. With a SCRIPT tag in the text, which the check rejects: -52% / -37%; -56% / -41%; -37%. - IFRAME, 22 sources: +2% / +3%; -11% / -9%; -11%. Text dense in `<` is slower: tinymce.min.js 5.3 to 10.6 µs on PHP 8.5; the preload JSON with raw HTML +30% / +26%; +13% / +11%; +17%. Each variant ran in its own process: 8 rounds on PHP 8.5, 3 on PHP 8.4 and 7.4. New tests cover a comment which starts with `>` or `->`, and comment text which resembles a closer but cannot end the comment; they pass on trunk's check as well.
HTML tag names match in any ASCII case. find_tag_names() now compares each candidate with strncasecmp(), which compares ASCII letters without regard to case, in place of substr_compare( ..., true ). strncasecmp() takes no offset, so the name is copied out with substr(). The first letter is compared before the copy, which keeps most `<` to one call. Time per candidate tag name, PHP 8.5 with JIT / PHP 8.4 / PHP 7.4: - First letter differs: 13 / 13 / 14 ns before, 11 / 13 / 14 ns after. - First letter matches: 18 / 17 / 18 ns before, 30 / 33 / 36 ns after.
Aligns STYLE contents escaping with the SCRIPT contents escaping added in r61477 (#10635).
Trac ticket: TBD
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.