Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/wp-includes/html-api/class-wp-html-processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -1511,9 +1511,6 @@ public function serialize_token(): string {
case 'IFRAME':
case 'NOEMBED':
case 'NOFRAMES':
$text = '';
break;

case 'SCRIPT':
case 'STYLE':
case 'XMP':
Expand Down
30 changes: 28 additions & 2 deletions src/wp-includes/html-api/class-wp-html-tag-processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@
* closing the SCRIPT from inside a JavaScript string. E.g. `console.log( '</script>' )`.
* - `TITLE` and `TEXTAREA` whose contents are treated as plaintext and then any
* character references are decoded. E.g. `1 &lt; 2 < 3` becomes `1 < 2 < 3`.
* - `IFRAME`, `NOEMBED`, `NOFRAMES`, `STYLE` whose contents are treated as
* - `IFRAME`, `NOEMBED`, `NOFRAMES`, `STYLE`, `XMP` whose contents are treated as
* raw plaintext and left as-is. E.g. `1 &lt; 2 < 3` remains `1 &lt; 2 < 3`.
*
* #### Other tokens with modifiable text.
Expand Down Expand Up @@ -4075,6 +4075,32 @@ public function set_modifiable_text( string $plaintext_content ): bool {
);
return true;

case 'IFRAME':
case 'NOEMBED':
case 'NOFRAMES':
case 'XMP':
$tag_name = $this->get_tag();
if ( false !== stripos( $plaintext_content, "</{$tag_name}" ) ) {
Comment thread
sirreal marked this conversation as resolved.
_doing_it_wrong(
__METHOD__,
sprintf(
/* translators: %s: HTML tag name. */
__( '%s text cannot contain its own closing tag.' ),
$tag_name
),
'7.1.0'
);
return false;
}

$this->lexical_updates['modifiable text'] = new WP_HTML_Text_Replacement(
$this->text_starts_at,
$this->text_length,
$plaintext_content
);

return true;

case 'STYLE':
$plaintext_content = preg_replace_callback(
'~</(?P<TAG_NAME>style)~i',
Expand Down Expand Up @@ -4132,7 +4158,7 @@ static function ( $tag_match ) {

_doing_it_wrong(
__METHOD__,
__( 'Only the SCRIPT, STYLE, TEXTAREA, and TITLE tags support setting modifiable text.' ),
__( 'This tag does not support setting modifiable text.' ),
Comment thread
sirreal marked this conversation as resolved.
'7.1.0'
);
return false;
Expand Down
54 changes: 36 additions & 18 deletions tests/phpunit/tests/html-api/wpHtmlProcessor-serialize.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,22 +270,6 @@ public function test_style_contents_are_not_escaped() {
);
}

/**
* XMP contents are parsed using the generic raw text element parsing algorithm.
* Their contents should not be escaped with HTML character references on normalization.
*
* @ticket 65372
*/
public function test_xmp_contents_are_not_escaped() {
$normalized = WP_HTML_Processor::normalize( "<xmp> < > & \" ' \x00 </xmp>" );

$this->assertSame(
"<xmp> < > & \" ' \u{FFFD} </xmp>",
$normalized,
'Should have preserved text inside an XMP element, except for replacing NULL bytes.'
);
}

public function test_unexpected_closing_tags_are_removed() {
$this->assertSame(
WP_HTML_Processor::normalize( 'one</div>two</span>three' ),
Expand Down Expand Up @@ -481,9 +465,9 @@ public function test_replaces_null_bytes_appropriately( string $html_with_nulls,
/**
* Data provider.
*
* @return array[]
* @return array<string, array{string, string}>
*/
public static function data_tokens_with_null_bytes() {
public static function data_tokens_with_null_bytes(): array {
return array(
'Tag name' => array( "<img\x00id=5>", "<img\u{FFFD}id=5></img\u{FFFD}id=5>" ),
'Attribute name' => array( "<img/\x00id=5>", "<img \u{FFFD}id=\"5\">" ),
Expand All @@ -492,11 +476,45 @@ public static function data_tokens_with_null_bytes() {
'Foreign content text' => array( "<svg>one\x00two</svg>", "<svg>one\u{FFFD}two</svg>" ),
'SCRIPT content' => array( "<script>alert(\x00)</script>", "<script>alert(\u{FFFD})</script>" ),
'STYLE content' => array( "<style>\x00 {}</style>", "<style>\u{FFFD} {}</style>" ),
'IFRAME content' => array( "<iframe>a\x00b</iframe>", "<iframe>a\u{FFFD}b</iframe>" ),
'NOEMBED content' => array( "<noembed>a\x00b</noembed>", "<noembed>a\u{FFFD}b</noembed>" ),
'NOFRAMES content' => array( "<noframes>a\x00b</noframes>", "<noframes>a\u{FFFD}b</noframes>" ),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don’t currently allow setting modifiable text in these, so it may be appropriate to toggle that in this same change, and follow what we do with SCRIPT and TITLE etc... and start rejecting updates which otherwise would contain closing tags for these.

although they are special atomic elements, we can find isolated closing tags. to make it easier we could reject strings containing </iframe, for example, which might be good enough for this upcoming release.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 7a5300d by enabling set_modifiable_text() for IFRAME, NOEMBED, and NOFRAMES, and rejecting updates containing their own closing tags such as </iframe.

Verified with PHPCS, the focused modifiable-text tests, the HTML Processor serialization class, and the full html-api group.

'XMP content' => array( "<xmp>a\x00b</xmp>", "<xmp>a\u{FFFD}b</xmp>" ),
'Comment text' => array( "<!-- \x00 -->", "<!-- \u{FFFD} -->" ),
);
}

/**
* Ensures that contents of rawtext elements are preserved when serializing.
*
* @ticket 65372
*
* @dataProvider data_rawtext_elements_with_html_syntax_character_contents
*
* @param string $html Normalized HTML containing a rawtext element with contents.
*/
public function test_rawtext_element_contents_are_preserved_when_normalizing( string $html ): void {
$this->assertSame(
$html,
WP_HTML_Processor::normalize( $html ),
'Should have preserved the rawtext element contents.'
);
}

/**
* Data provider.
*
* @return array<string, array{string}>
*/
public static function data_rawtext_elements_with_html_syntax_character_contents(): array {
return array(
'IFRAME' => array( 'before<iframe> < > &amp; " \' </iframe>after' ),
'NOEMBED' => array( 'before<noembed> < > &amp; " \' </noembed>after' ),
'NOFRAMES' => array( 'before<noframes> < > &amp; " \' </noframes>after' ),
'XMP' => array( 'before<xmp> < > &amp; " \' </xmp>after' ),
);
}

/**
* @ticket 62396
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,10 @@ public static function data_tokens_with_basic_modifiable_text_updates() {
'PI node (no separator)' => array( '<?wp-bit?>', 1, '{"just": "kidding"}', '<?wp-bit {"just": "kidding"}?>' ),
'Text node (end)' => array( '<img>of a dog', 2, 'of a cat', '<img>of a cat' ),
'Encoded text node' => array( '<figcaption>birds and dogs</figcaption>', 2, '<birds> & <dogs>', '<figcaption>&lt;birds&gt; &amp; &lt;dogs&gt;</figcaption>' ),
'IFRAME tag' => array( 'before<iframe>old content</iframe>after', 2, '<p>raw & text</p>', 'before<iframe><p>raw & text</p></iframe>after' ),
'NOEMBED tag' => array( 'before<noembed>old content</noembed>after', 2, '<p>raw & text</p>', 'before<noembed><p>raw & text</p></noembed>after' ),
'NOFRAMES tag' => array( 'before<noframes>old content</noframes>after', 2, '<p>raw & text</p>', 'before<noframes><p>raw & text</p></noframes>after' ),
'XMP tag' => array( 'before<xmp>old content</xmp>after', 2, '<p>raw & text</p>', 'before<xmp><p>raw & text</p></xmp>after' ),
'SCRIPT tag' => array( 'before<script></script>after', 2, 'const img = "<img> & <br>";', 'before<script>const img = "<img> & <br>";</script>after' ),
'STYLE tag' => array( '<style></style>', 1, 'p::before { content: "<img> & </style>"; }', '<style>p::before { content: "<img> & \3c\2fstyle>"; }</style>' ),
'TEXTAREA tag' => array( 'a<textarea>has no need to escape</textarea>b', 2, "so it <doesn't>", "a<textarea>so it <doesn't></textarea>b" ),
Expand Down Expand Up @@ -624,6 +628,10 @@ public static function data_unallowed_modifiable_text_updates() {
return array(
'Comment with -->' => array( '<!-- this is a comment -->', 'Comments end in -->' ),
'Comment with --!>' => array( '<!-- this is a comment -->', 'Invalid but legitimate comments end in --!>' ),
'IFRAME with </iframe>' => array( '<iframe>Replace me</iframe>', 'Just a </iframe>' ),
'NOEMBED with </NOEMBED>' => array( '<noembed>Replace me</noembed>', 'Just a </NOEMBED>' ),
'NOFRAMES with </noframes attributes>' => array( '<noframes>Replace me</noframes>', 'before</noframes sneaky>after' ),
'XMP with </xmp>' => array( '<xmp>Replace me</xmp>', 'Just a </xmp>' ),
'PI with >' => array( '<?wp-bit some data?>', 'Processing instructions end at the first >' ),
'PI with leading space' => array( '<?wp-bit some data?>', ' leading whitespace is skipped after the target' ),
'PI with leading tab' => array( '<?wp-bit some data?>', "\tleading whitespace is skipped after the target" ),
Expand Down
Loading