HTML API: Internal updates in 6.9

[dmsnell]

Tracking ticket for updates to the HTML API itself in 6.9.

• Interface enhancements
• Performance improvements
• Spec-compatability improvements
• Other internal details.

Trac ticket: Core-63738

Simplify META tag handling with regards to encoding confidence.

With any of the standard HTML Processor creation paths (WP_HTML_Processor::create_fragment() and WP_HTML_Processor::create_full_parser()), the encoding is never set to tentative. It is either irrelevant (on fragments) or certain on full parsers. In both cases, only the UTF-8 encoding is supported.

This is an optimization to lift a necessary condition to the top level and avoid processing in the case we expect to encounter in almost every case.

---

Fragments follow this path to set irrelevant:
​https://github.com/WordPress/wordpress-develop/blob/1c8185e12dd9fb1606eb22118f09aef46e6737d0/src/wp-includes/html-api/class-wp-html-processor.php#L319
​https://github.com/WordPress/wordpress-develop/blob/1c8185e12dd9fb1606eb22118f09aef46e6737d0/src/wp-includes/html-api/class-wp-html-processor.php#L547

And full parsers set certain here:
​https://github.com/WordPress/wordpress-develop/blob/1c8185e12dd9fb1606eb22118f09aef46e6737d0/src/wp-includes/html-api/class-wp-html-processor.php#L345

Trac ticket:

Trac ticket: Core-63738

Trac ticket: Core-63738

Reduce the number of length checks in WP_HTML_Tag_Processor::skip_script_data().

[jonsurrell]

​There's a follow-up task in this comment for meta tags setting UTF-8 charset to be handled.

[jonsurrell]

In 60501:

HTML API: Mark doctype info class as private.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9301.

Props jonsurrell, dmsnell.
See #63738.

Merged in [60501].

[jonsurrell]

In 60502:

HTML API: Simplify META tag processing.

META tag processing can be simplified in most cases. Add an early check and return to avoid additional processing.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9231.

Props jonsurrell, dmsnell.
See #63738.

Merged in [60502].

Trac ticket: Core-63738

Follow-up to https://core.trac.wordpress.org/changeset/60502 / #9231

Flagged by @mukeshpanchal27 ​https://github.com/WordPress/wordpress-develop/pull/9231#discussion_r2227595923

Thanks for quick PR 🚀

[jonsurrell]

In 60503:

HTML API: Make META tag test data provider static.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9318.

Follow-up to [60502].

Props jonsurrell, mukesh27.
See #63738.

Merged in [60503].

I've updated the comment and added a number of tests for different script data states. @dmsnell I'd appreciate if you could take another quick look.

This includes ​https://github.com/WordPress/wordpress-develop/pull/9230 (merged here).

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

Address two small HTML API mis-parses of script contents.

---

<script>
`. This abruptly closed empty comment does not transition to _escaped_, but remains in the _unescaped_ state.

In the example, the _double-escaped_ state should not be reached on `<script>` (because the processor should not be in the _escaped_ state) and the script tag should close correctly at `</script>`.

[https://playground.wordpress.net/?plugin=html-api-debugger&url=%2Fwp-admin%2Fadmin.php%3Fpage%3Dhtml-api-debugger%26html%3D%253Cscript%253E%250A%253C%2521--%253E%250A%253Cscript%253E%250A%253C%252Fscript%253E%25F0%259F%258E%2589&wp=6.8.2 before] / [https://playground.wordpress.net/?plugin=html-api-debugger&url=%2Fwp-admin%2Fadmin.php%3Fpage%3Dhtml-api-debugger%26html%3D%253Cscript%253E%250A%253C%2521--%253E%250A%253Cscript%253E%250A%253C%252Fscript%253E%25F0%259F%258E%2589&core-pr=9397 after]

---

```html
<script>
<!--
<script</script>🎉

In this case <script< is correctly recognized as _not_ a sequence that should transition from _escaped_ to _double-escaped_, however it incorrectly advances beyond the following < character that starts the script close tag and does not close correctly at </script>.

​before / ​after

I've pulled the updated comment and moved the test adjacent to the other script parsing test, things which were noted in ​https://github.com/WordPress/wordpress-develop/pull/9397.

[jonsurrell]

In 60617:

HTML API: Reduce length checks in skip_script_data.

Apply an optimization to remove several repeated string length checks in WP_HTML_Tag_Processor::skip_script_data().

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9230.

Props jonsurrell, dmsnell.
See #63738.

Merged in [60617].

Good reminder on the coverage reports. I've added more tests and all the lines in skip_script_data are now hit with the exception of this condition, the body of this is never entered:

​https://github.com/WordPress/wordpress-develop/blob/057f585b2e7698038ff44d299a2cfdb394318a61/src/wp-includes/html-api/class-wp-html-tag-processor.php#L1631-L1633

I think that it's impossible to hit now (after [60617] / ​https://github.com/WordPress/wordpress-develop/pull/9230). It may always have been impossible. However, I don't mind leaving that check even if it's redundant.

I believe the condition would be hit on inputs like <script></script that end right after the closing script tag name. With the early length check it shouldn't be possible to hit this condition because the function would already have returned false here:

​https://github.com/WordPress/wordpress-develop/blob/057f585b2e7698038ff44d299a2cfdb394318a61/src/wp-includes/html-api/class-wp-html-tag-processor.php#L1531-L1533

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

Address a minor HTML API mis-parse of script contents, where \f is not recognized as a valid trailing/termination character of SCRIPT tag names.

---

In this case, the \f _form feed_ should be recognized as the end of the script tag name and close the script:

<script></script␌>🎉

​before / ​after

---

This is another example of the same issue. Again, \f _form feed_ should be recognized as the end of the script tag name and enter the double-escaped state (this script tag is not closed and consumes the rest of the document):

<script><!--<script␌</script>🎉

In this case <script< is correctly recognized as _not_ a sequence that should transition from _escaped_ to _double-escaped_, however it incorrectly advances beyond the following < character that starts the script close tag and does not close correctly at </script>.

​before / ​after

Is it possible to have Copilot not pollute the PR with its comments? Can it not run locally, or at least leave comments as the result of some command you run? or is it only designed to run from the Github interface and leave erroneous comments?

Thanks for diving in and doing your own investigation, I'm happy to have my research confirmed ❤️

The blog post we're referencing is available here: ​Safe JSON in script tags: How not to break a site

On that note, the way I read your diagram, the </script transitions into Close SCRIPT tag are misleading, because the only exit out of the element is a complete closing tag whose name is SCRIPT. This means that if you encounter </script- in those places, we stay in script data or in script data escaped.

That's a good point, it was a bit ambiguous. I alluded to the trailing character in a foot note but it wasn't sufficiently clear. I added a note after the diagram to help clarify because this is an important point.

It’s fine to keep these separate, but would we want to combine this with #9402? Maybe merge it into here?

This is mostly about accounting and not about code.

Merged into ​https://github.com/WordPress/wordpress-develop/pull/9397.

I've merged ​https://github.com/WordPress/wordpress-develop/pull/9402 here.

[jonsurrell]

In 60649:

HTML API: Improve script tag escape state processing.

Addresses some edge cases parsing of script tag contents:

• "<!-->" remains in the unescaped state and does not enter the escaped state.
• Contents in the escaped state that end with "<script" do not enter double-escaped state.
• "\f" (Form Feed) was missing as a tag name terminating character.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9397 and ​https://github.com/WordPress/wordpress-develop/pull/9402.

Props jonsurrell, dmsnell.
See #63738.

Merged in [60649].

The WP_Tag_Processor::set_modifiable_text() method should treat <script the same way as it treats </script. Either of these sequences may affect the script elements close.

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

This could allow some more safe inputs with a regular expression based check. There may also be ways to escape the script tag contents.

Before exploring those improvements, I'd like to address this issue where some potentially dangerous script tag contents are allowed.

[jonsurrell]

In 60706:

HTML API: Prevent adding dangerous double-escape SCRIPT contents.

Prevent WP_Tag_Processor::set_modifiable_text() from allowing SCRIPT contents with "<script" like it does with "</script". Either of these sequences may affect the script element's close.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9560.

Props jonsurrell, westonruter, dmsnell.
See #63738.

Merged in [60706].

[wildworks]

@dmsnell The 6.9 Beta1 release is coming soon, can this ticket be closed now? Are there any remaining tasks that need to be addressed on this ticket?

[dmsnell]

@wildworks I’m still working on these; I don’t know what the practice is, but if we could wait to close them until the Beta is out that would be a big help to me (keep me from having to address the same comment or closure multiple times).

Either way, if you need to close them I won’t stand in your way, but I will continue to try and merge work from them before the Beta.

[wildworks]

As the Beta1 release begins, I will close this ticket. If there are any other issues that need to be addressed, please leave a comment.

Merged in r63179.