Automattic / Automattic/jetpack
Forms: esc_shortcode_val() writes decimal entities that unesc_attr() never decodes
- Dominant language
- PHP
- Stars
- 1.8k
- Forks
- 898
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 774
Description
### Summary
`Contact_Form::esc_shortcode_val()` encodes four characters as **decimal** HTML entities so they survive the shortcode parser, but `Contact_Form_Shortcode::unesc_attr()` only decodes the **hex** forms. Anything that round-trips through the block → shortcode → render path keeps a raw entity in its value.
### Detail
Encoding side — `projects/packages/forms/src/contact-form/class-contact-form.php` (in `esc_shortcode_val()`):
```php
'[' => '[',
']' => ']',
'\\' => '\',
',' => ',',
```
Decoding side — `projects/packages/forms/src/contact-form/class-contact-form-shortcode.php` (in `unesc_attr()`):
```php
$value = preg_replace( array( '/�*22;/i', '/�*27;/i', '/�*26;/i', '/�*2c;/i' ), array( '"', "'", '&', ',' ), $value );
$value = htmlspecialchars_decode( $value, ENT_QUOTES );
```
`�*2c;` is the **hex** comma. The decimal `,` that `esc_shortcode_val()` actually writes matches neither that pattern nor `htmlspecialchars_decode()`, which doesn't handle numeric entities at all. Same for `[`, `\`, `]` — no decoder exists for those in any form.
### Why nobody has noticed
Every existing consumer of these attributes emits either through `wp_kses_post()` or into an HTML attribute value, so the browser decodes the leftover entity on the way to the screen.
It only becomes visible when a value is emitted through `esc_html()` into a **text node**: the `&` gets escaped again and the visitor sees the raw code. Field help text (#51122) was the first such consumer — author types `Enter your name, then your email.`, visitor sees `Enter your name, then your email.`
### Current state
#51122 works around it locally in `Contact_Form_Field::get_help_text()`:
```php
return html_entity_decode( trim( $help_text ), ENT_QUOTES );
```
That was deliberately scoped to the one affected consumer, since widening `unesc_attr()` changes what every attribute emits and nothing else is affected today. Per review discussion, the underlying asymmetry is worth fixing properly rather than papering over per-consumer.
### Suggested fix
Teach `unesc_attr()` to decode the decimal forms `esc_shortcode_val()` actually writes (`,` `[` `\` `]`), then drop the local `html_entity_decode()` from `get_help_text()`.
Needs a careful pass over existing consumers first — some may be relying on the browser decoding the leftover entity, and decoding earlier could double-decode author content that legitimately contains `,` as literal text.
### Context
Found during review of #51122. See https://github.com/Automattic/jetpack/pull/51122#discussion_r3770245003.
Contributor guide
Assessment
This issue has not been assessed yet.