Skip to content

Commit

Permalink
KSES: Fix tests and detection of HTML Bogus Comment spans.
Browse files Browse the repository at this point in the history
In [58418] a test was added without the `test_` prefix in its function
name, and because of that, it wasn't run in the test suite.
The prefix has been added to ensure that it runs.

In the original patch, due to a logical bug, a recursive loop to
transform the inside contents of the bogus comments was never run
more than once. This has been fixed.

This patch also includes one more case where `kses` wasn't
properly detecting the bogus comment state, and adds a test case
to cover this. It limits itself to some but not all constructions
of invalid markup declaration so that it doesn't conflict with
existing behaviors around those and other kinds of invalid comments.

Props ellatrix, dmsnell.
See #61009.
Follow-up to [58418].

Co-authored-by: ellatrix <[email protected]>
  • Loading branch information
dmsnell and ellatrix committed Jun 17, 2024
1 parent dc6c8c8 commit 800c254
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
33 changes: 22 additions & 11 deletions src/wp-includes/kses.php
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,9 @@ function wp_kses_split( $content, $allowed_html, $allowed_protocols ) {
(<!--.*?(-->|$)) # - Normative HTML comments.
|
</[^a-zA-Z][^>]*> # - Closing tags with invalid tag names.
|
<![^>]*> # - Invalid markup declaration nodes. Not all invalid nodes
# are matched so as to avoid breaking legacy behaviors.
)
|
(<[^>]*(>|$)|>) # Tag-like spans of text.
Expand Down Expand Up @@ -1114,22 +1117,30 @@ function wp_kses_split2( $content, $allowed_html, $allowed_protocols ) {
}

/*
* When a closing tag appears with a name that isn't a valid tag name,
* it must be interpreted as an HTML comment. It extends until the
* first `>` character after the initial opening `</`.
* When certain invalid syntax constructs appear, the HTML parser
* shifts into what's called the "bogus comment state." This is a
* plaintext state that consumes everything until the nearest `>`
* and then transforms the entire span into an HTML comment.
*
* Preserve these comments and do not treat them like tags.
*
* @see https://html.spec.whatwg.org/#bogus-comment-state
*/
if ( 1 === preg_match( '~^</[^a-zA-Z][^>]*>$~', $content ) ) {
$content = substr( $content, 2, -1 );
$transformed = null;
if ( 1 === preg_match( '~^(?:</[^a-zA-Z][^>]*>|<![a-z][^>]*>)$~', $content ) ) {
/**
* Since the pattern matches `</…>` and also `<!…>`, this will
* preserve the type of the cleaned-up token in the output.
*/
$opener = $content[1];
$content = substr( $content, 2, -1 );

while ( $transformed !== $content ) {
$transformed = wp_kses( $content, $allowed_html, $allowed_protocols );
$content = $transformed;
}
do {
$prev = $content;
$content = wp_kses( $content, $allowed_html, $allowed_protocols );
} while ( $prev !== $content );

return "</{$transformed}>";
// Recombine the modified inner content with the original token structure.
return "<{$opener}{$content}>";
}

/*
Expand Down
5 changes: 4 additions & 1 deletion tests/phpunit/tests/kses.php
Original file line number Diff line number Diff line change
Expand Up @@ -1936,11 +1936,13 @@ public function filter_wp_kses_object_added_in_html_filter( $tags, $context ) {
*
* @ticket 61009
*
* @dataProvider data_html_containing_various_kinds_of_html_comments
*
* @param string $html_comment HTML containing a comment; must not be a valid comment
* but must be syntax which a browser interprets as a comment.
* @param string $expected_output How `wp_kses()` ought to transform the comment.
*/
public function wp_kses_preserves_html_comments( $html_comment, $expected_output ) {
public function test_wp_kses_preserves_html_comments( $html_comment, $expected_output ) {
$this->assertSame(
$expected_output,
wp_kses( $html_comment, array() ),
Expand All @@ -1957,6 +1959,7 @@ public static function data_html_containing_various_kinds_of_html_comments() {
return array(
'Normative HTML comment' => array( 'before<!-- this is a comment -->after', 'before<!-- this is a comment -->after' ),
'Closing tag with invalid tag name' => array( 'before<//not a tag>after', 'before<//not a tag>after' ),
'Incorrectly opened comment (Markup declaration)' => array( 'before<!also not a tag>after', 'before<!also not a tag>after' ),
);
}

Expand Down

0 comments on commit 800c254

Please sign in to comment.