Accept non-string scalars in the esc_*() escaping functions - #13044
Accept non-string scalars in the esc_*() escaping functions#13044westonruter wants to merge 1 commit into
Conversation
esc_attr(), esc_html(), esc_js(), esc_textarea(), and esc_xml() have always accepted int and float at runtime. Both wp_check_invalid_utf8() and _wp_specialchars() open by casting to string, so the value is coerced long before it reaches htmlspecialchars(). Only the @PARAM annotations claimed otherwise, which turned every such call site into a static analysis error. Document the parameter as string|int|float and cast at the top of each function. That cast is not redundant with the ones downstream: it is what makes the $text argument handed to the attribute_escape, esc_html, and js_escape filters actually match its documented string type. Those filters previously received the raw int or float. bool is deliberately excluded. Casting true yields '1' while false yields '', and an empty string is indistinguishable from a missing value in any output context, so passing a bool is nearly always either a latent bug or a sentinel convention that reads better written out. The two call sites in core that pass one stay baselined rather than being silently permitted by the signature. Regenerating the baselines drops 54 entries covering 101 errors from tests/phpstan/baselines/argument.type.neon. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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. |
The `esc_attr()`, `esc_html()`, `esc_js()`, `esc_textarea()`, and `esc_xml()` functions were each annotated `@param string $text`, but none of them has ever required a string at runtime. Both `wp_check_invalid_utf8()` and `_wp_specialchars()` open by casting to string, and that coercion dates to r10298 and r11380, the same year `esc_attr()` and `esc_html()` were introduced in 2.8.0. Only the annotation ever claimed otherwise, which is why passing an integer, overwhelmingly a post ID, a term ID, or a count, has always worked while still registering as a static analysis error. Widen the annotation to `string|int|float` and cast at the top of each function. That cast is not redundant with the ones downstream: it is what makes the `$text` argument handed to the `attribute_escape`, `esc_html`, and `js_escape` filters match its documented `string` type, where callbacks previously received the raw integer or float. For `esc_textarea()` it also affects the output, since that function passed `$text` straight to `htmlspecialchars()`, so `esc_textarea( null )` emitted a deprecation notice on PHP 8.1 and later. The `bool` type is deliberately excluded. Casting `true` yields `'1'` while `false` yields the empty string, and an empty string is indistinguishable from a missing value in any output context. The two call sites in core which pass one stay baselined rather than being permitted by the signature; one of the two is an a11y defect being addressed separately. Regenerating the baselines drops 54 entries covering 101 errors from `tests/phpstan/baselines/argument.type.neon`, where `esc_attr()` was the largest single cluster. Developed in #13044. Follow-up to r11380, r63024. See #65817. git-svn-id: https://develop.svn.wordpress.org/trunk@63296 602fd350-edb4-49c9-b593-d223f7449a82
The `esc_attr()`, `esc_html()`, `esc_js()`, `esc_textarea()`, and `esc_xml()` functions were each annotated `@param string $text`, but none of them has ever required a string at runtime. Both `wp_check_invalid_utf8()` and `_wp_specialchars()` open by casting to string, and that coercion dates to r10298 and r11380, the same year `esc_attr()` and `esc_html()` were introduced in 2.8.0. Only the annotation ever claimed otherwise, which is why passing an integer, overwhelmingly a post ID, a term ID, or a count, has always worked while still registering as a static analysis error. Widen the annotation to `string|int|float` and cast at the top of each function. That cast is not redundant with the ones downstream: it is what makes the `$text` argument handed to the `attribute_escape`, `esc_html`, and `js_escape` filters match its documented `string` type, where callbacks previously received the raw integer or float. For `esc_textarea()` it also affects the output, since that function passed `$text` straight to `htmlspecialchars()`, so `esc_textarea( null )` emitted a deprecation notice on PHP 8.1 and later. The `bool` type is deliberately excluded. Casting `true` yields `'1'` while `false` yields the empty string, and an empty string is indistinguishable from a missing value in any output context. The two call sites in core which pass one stay baselined rather than being permitted by the signature; one of the two is an a11y defect being addressed separately. Regenerating the baselines drops 54 entries covering 101 errors from `tests/phpstan/baselines/argument.type.neon`, where `esc_attr()` was the largest single cluster. Developed in WordPress/wordpress-develop#13044. Follow-up to r11380, r63024. See #65817. Built from https://develop.svn.wordpress.org/trunk@63296 git-svn-id: http://core.svn.wordpress.org/trunk@62489 1a063a9b-81f0-0310-95a4-ce76da25c4cd
✅ Committed in r63296 (abc50fe).
Widens the
@paramannotation on theesc_*()escaping functions fromstringtostring|int|float, and adds an explicit(string)cast to each.This clears 54 baseline entries covering 101 errors from
tests/phpstan/baselines/argument.type.neon.esc_attr()alone accounted for 94 of them — the largest single cluster in that file.Background
esc_attr(),esc_html(),esc_js(),esc_textarea(), andesc_xml()were all annotated@param string $text, but none of them has ever required a string at runtime. The two helpers they delegate to each open by coercing:_wp_specialchars()—$text = (string) $text;wp_check_invalid_utf8()— the sameTracing that cast back through r11380 (which deprecated
wp_specialchars()in favour ofesc_html()) to r10298 puts it in 2009. Sinceesc_attr()andesc_html()are@since 2.8.0, also 2009, the coercion is as old as the functions themselves. Only the annotation ever claimed otherwise — which is why passing anint, overwhelmingly a post ID, a term ID, or a count, has always worked while still registering as a static analysis error.Confirmed against the current codebase (PHP 8.5):
esc_attr()esc_html()esc_js()esc_textarea()42'42''42''42''42'-7'-7''-7''-7''-7'1.5'1.5''1.5''1.5''1.5'true'1''1''1''1'false''''''''Why the cast is not redundant
For
esc_attr(),esc_html(), andesc_js()the value is already coerced downstream, so the new cast does not change what gets escaped. What it does change is the second argument handed to theattribute_escape,esc_html, andjs_escapefilters. Each is documented as@param string $text The text prior to being escaped, but callbacks previously received the rawintorfloat. The cast makes that documented contract true.esc_textarea()is the one case where the cast also affects escaping: it passed$textstraight tohtmlspecialchars(), soesc_textarea( null )emitted a PHP 8.1+ deprecation. That no longer happens.Why
boolis excludedboolis accepted at runtime, and the cast handles it, but it is deliberately left out of the annotation. Castingtrueyields'1'whilefalseyields'', and an empty string is indistinguishable from a missing value, an empty option, or a failed lookup in any output context. There is no output context where''is a meaningful rendering of false.Core has exactly two call sites passing a
bool, and they split evenly:wp-admin/options-general.php:193—data-state="<?php echo esc_attr( has_site_icon() ); ?>". This works, becausesite-icon.jscompares against the literal'1'and writes back'1'/''. The convention is real but implicit.wp-admin/customize.php:291—aria-pressed="<?php echo esc_attr( $active ); ?>". This rendersaria-pressed="1"for the desktop button andaria-pressed=""for tablet and mobile. Valid ARIA tristate values are only"true","false","mixed", and"undefined", so all three buttons are exposed as non-toggles untilcontrols.js(which correctly writes"true"/"false") runs.Admitting
boolto the signature would permanently silence the second one. Both stay baselined instead, so they remain visible as outstanding work. The accessibility fix is intentionally not in this PR and will be handled separately.Scope
esc_textarea()andesc_xml()had no baselined errors and are included only to keep the family consistent — five functions with identical semantics should not carry three different parameter annotations.Call sites are not touched. Nothing here changes escaping behaviour for any input that was already a string.
Trac ticket: https://core.trac.wordpress.org/ticket/65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Investigating the runtime coercion behaviour and its history, the docblock and cast changes, regenerating the baselines, and drafting this description. The decision to exclude
booland to defer the accessibility fix was mine, as was review of the result.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.