Automattic / Automattic/jetpack

Forms: esc_shortcode_val() writes decimal entities that unesc_attr() never decodes

Open
#51,300 0 comments 0 reactions 1 assignee Claimed by @enejb View on GitHub
[Feature] Contact Form [Pri] Normal [Type] Janitorial
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.