HTML API: Double escape character refs when setting attribute values #10143

sirreal · 2025-10-03T15:46:01Z

Attribute values set with the HTML API method set_attribute() are escaped with esc_attr(). That function avoids "double encoding" things that look like HTML character references.

The HTML API should encode whatever it receives, and apply "double encoding." The HTML API expects to receive plain string inputs and manage any necessary encoding itself. The fact that "double encoding" is disabled violates this expectation and makes it difficult correctly to set attribute values that contain sequences that appear to be HTML character references.

By contrast, set_modifiable_text() does not rely on esc_html() and uses htmlspecialchars() directly. It will encode HTML character references as expected.

The text & appears to be an encoded character reference:
$amp_text = '&amp;';
$p = WP_HTML_Processor::create_fragment( '<p>x</p>' );
$p->next_tag();
$p->set_attribute( 'data-attr', $amp_text );
$p->next_token();
$p->set_modifiable_text( $amp_text );
echo $p->get_updated_html();
This prints the following HTML:
<p data-attr="&amp;">&amp;amp;</p>
Notice how the input text is treated differently between an attribute and text. The HTML encoding of the & character is the same in both contexts. The attribute has the value & instead of the expected &. The text node in the P element correctly renders & as expected.

Here's a demo of the difference in behavior between setting attributes and modifiable text.

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

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 · 2025-10-03T15:46:10Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jonsurrell, dmsnell, westonruter.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

sirreal · 2025-10-03T15:54:53Z

src/wp-includes/html-api/class-wp-html-tag-processor.php

-			$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes(), true ) ? esc_url( $value ) : esc_attr( $value );
+			$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes(), true )
+				? esc_url( $value )
+				: htmlspecialchars( $value, ENT_QUOTES | ENT_HTML5 );


This matches how set_modifiable_text() escapes:

wordpress-develop/src/wp-includes/html-api/class-wp-html-tag-processor.php

Lines 3756 to 3761 in abc5dd8

if ( self::STATE_TEXT_NODE === $this->parser_state ) {

$this->lexical_updates['modifiable text'] = new WP_HTML_Text_Replacement(

$this->text_starts_at,

$this->text_length,

htmlspecialchars( $plaintext_content, ENT_QUOTES | ENT_HTML5 )

);

This performs escaping like this:

< => <

> => >

& => &

" => "

' => '

This is more escaping than is strictly necessary because " is always used a few lines later, but it aligns with common practices in WordPress and the other HTML API escaping.

wordpress-develop/src/wp-includes/html-api/class-wp-html-tag-processor.php

Line 3965 in abc5dd8

$updated_attribute = "{$name}=\"{$escaped_new_value}\"";

I don’t trust the PHP man page descriptions: here is what it says in a special called-out note:

Note that this function does not translate anything beyond what is listed above. For full entity translation, see htmlentities()

but it does more

$s = "z\x00\x95\xC2\x96b\xED\xa0\x80"; // letter, null, invalid byte, C1 control, letter, surrogate half var_dump( htmlspecialchars( $s, ENT_QUOTES | ENT_HTML5 ) ); string(0) "" // minimal subpart leaves us with four substitute characters // (what looks like the second substitute character is being display by the browser, not the bytes forming U+FFFD, as it renders a C1 control) var_dump( mb_scrub( $s, 'UTF-8' ) ); string(17) "z��b��" var_dump( htmlspecialchars( $s, ENT_QUOTES | ENT_HTML5 | ENT_SUBSTITUTE ) ); string(11) "z��b�" var_dump( htmlspecialchars( $s, ENT_QUOTES | ENT_HTML5 | ENT_IGNORE ) ); string(5) "z�b" var_dump( htmlspecialchars( $s, ENT_QUOTES | ENT_HTML5 | ENT_DISALLOWED ) ); string(0) ""

given that this is yet-again another PHP function which doesn’t do what it says, and that we generally rely on the browser in the HTML API to handle invalid encodings, maybe we should avoid this here and update set_modifiable_text() to only replace those characters you mention. strtr() and str_replace() should both be fast, I would expect, and possibly faster than htmlspecialchars() anyway, and it doesn’t require deep dives into the PHP source code or creating complex test cases to make the behavior obvious.

I changed both escapes to use strtr() in 1435b6e.

github-actions · 2025-10-03T16:00:37Z

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.

The HTML5 encoding uses ' instead of the numeric character reference.

sirreal · 2025-10-06T10:17:17Z

The "double encode" behavior of esc_attr() (which really comes from htmlspecialchars()) is very strange when considering named character references that appear with and without a trailing ;. They're both character references and will be decoded by the browser (it's slightly more complex) but the double-encoded behavior does not treat them the same. Here's a good demo with some explanation.

westonruter · 2025-10-06T20:51:15Z

src/wp-includes/html-api/class-wp-html-tag-processor.php

 			 */
-			$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes(), true ) ? esc_url( $value ) : esc_attr( $value );
+			$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes(), true )
+				? esc_url( $value )


The esc_url() function also does some stuff with entities, namely:

wordpress-develop/src/wp-includes/formatting.php

Lines 4522 to 4527 in fe2b33f

// Replace ampersands and single quotes only when displaying.

if ( 'display' === $_context ) {

$url = wp_kses_normalize_entities( $url );

$url = str_replace( '&', '&', $url );

$url = str_replace( "'", ''', $url );

}

To avoid that, I think using esc_url_raw() is preferable:

Suggested change

? esc_url( $value )

? htmlspecialchars( esc_url_raw( $value ) )

This line is only a formatting change, I broke the ternary onto three lines.

Can we consider that suggestion in a separate ticket and PR?

Ah sure, but it it is related to the double-escaping issue, because esc_url() does wp_kses_normalize_entities() which includes converting all & to & among other things. For example, these:

var_dump( esc_url( 'http://example.com/?foo&bar' ) ); var_dump( esc_url( 'http://example.com/?foo&bar' ) ); var_dump( esc_url( 'http://example.com/?foo&bar' ) );

All result in:

string(32) "http://example.com/?foo&bar" string(32) "http://example.com/?foo&bar" string(32) "http://example.com/?foo&bar"

I don't really see why a URI attribute should be treated differently at the HTML API layer. So this could instead just be:

$escaped_new_value = strtr( $value, array( '<' => '<', '>' => '>', '&' => '&', '"' => '"', "'" => ''', ) );

You're absolutely right that the double-escaping issue also exists in esc_url(), that's unfortunate. esc_url() does a lot of things, it doesn't seem as easily replaced as esc_attr() or esc_html(). I suspect more discussion will be necessary regarding URL-related changes which is why I'm reluctant to include those changes here.

See r58472 and r58473.

OK, sounds good.

dmsnell

@sirreal why not use & for ampersand to avoid issues in kses? are you not worried about this content then being corrupted on further passes through the sanitizers?

sirreal · 2025-10-08T18:11:40Z

& seemed like the most common, expected, and legible encoding. r60616 addressed the KSES character reference issues I was aware aware of. Do you think we should use the numeric form?

dmsnell · 2025-10-08T19:13:15Z

Do you think we should use the numeric form?

Not particularly; I just wanted your input on whether it would likely be more durable if we made this change here and now.

dmsnell · 2025-10-08T20:11:17Z

src/wp-includes/html-api/class-wp-html-tag-processor.php

 	 *
-	 * For string attributes, the value is escaped using the `esc_attr` function.
+	 * HTML escaping will be performed for string attribute values. The input value should
+	 * not be unescaped.


we have a confusing double-negative here.
can we rearrange this so it’s a positive indication of what to do, rather than a negative impression of what not to do?

* This function handles all HTML escaping: send normal and unescaped string values. * The HTML API will convey input strings such that a browser will read the decoded * string values as verbatim copies to what is provided here. * * Example: * * // Renders “Eggs & Milk” in a browser, encoded as “Eggs & Milk”. * $processor->set_attribute( 'title', 'Eggs & Milk' ); * * // Renders “Eggs & Milk” in a browser, encoded as “Eggs &amp; Milk”. * $processor->set_attribute( 'title', 'Eggs & Milk' );

Thanks, updated the description in 0db1374, happy to make further adjustments.

src/wp-includes/html-api/class-wp-html-tag-processor.php

dmsnell · 2025-10-09T22:53:19Z

I’ve expanded the examples even further to try and make it even more obvious what’s going on when character references are sent into the function. Additionally I have copied this update over to the HTML Processor, as the same changes apply there.

dmsnell · 2025-10-09T23:05:54Z

Also I’ve added a similar note in the docblock for set_modifiable_test(). This method only exists in the Tag Processor so there’s no corresponding update in the HTML Processor.

The HTML API has relied on `esc_attr()` and `esc_html()` when setting string attribute values or the contents of modifiable text. This leads to unexpected behavior when those functions attempt to prevent double-escaping of existing character references, and it can make certain contents impossible to represent. After this change, the HTML API will reliably escape all submitted plaintext such that it appears in the browser the way it was submitted to the HTML API, with all character references escaped. This does not change the behavior of how URL attributes are escaped. Developed in #10143 Discussed in https://core.trac.wordpress.org/ticket/64054 Props dmsnell, jonsurrell, westonruter. Fixes #64054. git-svn-id: https://develop.svn.wordpress.org/trunk@60919 602fd350-edb4-49c9-b593-d223f7449a82

github-actions · 2025-10-09T23:37:14Z

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 60919
GitHub commit: 4892d46

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

The HTML API has relied on `esc_attr()` and `esc_html()` when setting string attribute values or the contents of modifiable text. This leads to unexpected behavior when those functions attempt to prevent double-escaping of existing character references, and it can make certain contents impossible to represent. After this change, the HTML API will reliably escape all submitted plaintext such that it appears in the browser the way it was submitted to the HTML API, with all character references escaped. This does not change the behavior of how URL attributes are escaped. Developed in WordPress/wordpress-develop#10143 Discussed in https://core.trac.wordpress.org/ticket/64054 Props dmsnell, jonsurrell, westonruter. Fixes #64054. Built from https://develop.svn.wordpress.org/trunk@60919 git-svn-id: http://core.svn.wordpress.org/trunk@60255 1a063a9b-81f0-0310-95a4-ce76da25c4cd

The HTML API has relied on `esc_attr()` and `esc_html()` when setting string attribute values or the contents of modifiable text. This leads to unexpected behavior when those functions attempt to prevent double-escaping of existing character references, and it can make certain contents impossible to represent. After this change, the HTML API will reliably escape all submitted plaintext such that it appears in the browser the way it was submitted to the HTML API, with all character references escaped. This does not change the behavior of how URL attributes are escaped. Developed in WordPress/wordpress-develop#10143 Discussed in https://core.trac.wordpress.org/ticket/64054 Props dmsnell, jonsurrell, westonruter. Fixes #64054. Built from https://develop.svn.wordpress.org/trunk@60919 git-svn-id: https://core.svn.wordpress.org/trunk@60255 1a063a9b-81f0-0310-95a4-ce76da25c4cd

sirreal added 2 commits October 3, 2025 17:32

Layout ternary on multiple lines

8d8622e

Escape with htmlspecialchars

8e0d406

sirreal commented Oct 3, 2025

View reviewed changes

sirreal added 5 commits October 3, 2025 18:11

Add test

66af901

Use assertEqualHTML for legible test output

ff62be1

Update two tests for no-double-encode behavior

daef88f

Update tests

c2e2ad4

Update block supports tests

5e5473a

The HTML5 encoding uses ' instead of the numeric character reference.

sirreal requested review from dmsnell and westonruter October 6, 2025 10:17

This comment was marked as resolved.

Sign in to view

westonruter reviewed Oct 6, 2025

View reviewed changes

sirreal added 3 commits October 8, 2025 16:04

Merge branch 'trunk' into trac-64054

40cfce5

Update escaping function to use simple strtr()

1435b6e

Update comment about attribute value escaping

b581113

dmsnell reviewed Oct 8, 2025

View reviewed changes

Improve function description following review suggestion.

0db1374

dmsnell approved these changes Oct 9, 2025

View reviewed changes

src/wp-includes/html-api/class-wp-html-tag-processor.php Outdated Show resolved Hide resolved

Fix typo in comment, expand examples, and copy to HTML Processor.

605b00d

Moar text fixes

00f01bf

westonruter approved these changes Oct 9, 2025

View reviewed changes

github-actions bot closed this Oct 9, 2025

This was referenced Oct 10, 2025

Pattern overrides: Remove old back compat code and update tests WordPress/gutenberg#72231

Merged

Background Block Support: Fix unit test HTML escaping after core change WordPress/gutenberg#72233

Closed

ntsekouras mentioned this pull request Oct 10, 2025

Fix PHP tests WordPress/gutenberg#72236

Merged

sirreal mentioned this pull request Nov 3, 2025

Tests: Ensure block support tests are resilient to equivalent HTML changes WordPress/gutenberg#72934

Closed

sirreal deleted the trac-64054 branch November 4, 2025 17:57

sirreal mentioned this pull request Dec 3, 2025

HTML API: Ensure correct encoding of modified class names #10591

Closed

	if ( self::STATE_TEXT_NODE === $this->parser_state ) {
	$this->lexical_updates['modifiable text'] = new WP_HTML_Text_Replacement(
	$this->text_starts_at,
	$this->text_length,
	htmlspecialchars( $plaintext_content, ENT_QUOTES \| ENT_HTML5 )
	);

	// Replace ampersands and single quotes only when displaying.
	if ( 'display' === $_context ) {
	$url = wp_kses_normalize_entities( $url );
	$url = str_replace( '&', '&', $url );
	$url = str_replace( "'", ''', $url );
	}

	? esc_url( $value )
	? htmlspecialchars( esc_url_raw( $value ) )

HTML API: Double escape character refs when setting attribute values #10143

HTML API: Double escape character refs when setting attribute values #10143

Uh oh!

Conversation

sirreal commented Oct 3, 2025

Uh oh!

github-actions bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sirreal Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

dmsnell Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirreal Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 3, 2025

Test using WordPress Playground

Some things to be aware of

Uh oh!

sirreal commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

westonruter Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirreal Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirreal Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

dmsnell left a comment

Choose a reason for hiding this comment

Uh oh!

sirreal commented Oct 8, 2025

Uh oh!

dmsnell commented Oct 8, 2025

Uh oh!

dmsnell Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

sirreal Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmsnell commented Oct 9, 2025

Uh oh!

dmsnell commented Oct 9, 2025

Uh oh!

github-actions bot commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Oct 3, 2025 •

edited

Loading

dmsnell Oct 6, 2025 •

edited

Loading

sirreal commented Oct 6, 2025 •

edited

Loading

westonruter Oct 6, 2025 •

edited

Loading

westonruter Oct 9, 2025 •

edited

Loading