Fix less than character (<) display issue #2243
Conversation
WalkthroughThe changes introduce a new method Changes
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 PHPStan (2.0.3)/bin/bash: line 1: /vendor/bin/phpstan: No such file or directory 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
classes/models/fields/FrmFieldText.php (1)
54-54: Update version tag placeholder.The
@since x.xversion tag needs to be updated with the actual version number.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/models/fields/FrmFieldText.php(1 hunks)classes/models/fields/FrmFieldTextarea.php(1 hunks)
🔇 Additional comments (1)
classes/models/fields/FrmFieldText.php (1)
58-70: Consider edge cases and potential improvements.The method has several areas that could be improved:
- The method is protected but not used within the class.
- Empty string case isn't handled explicitly.
- Array handling assumes comma-separator is always desired.
- Potential double-encoding of entities when
htmlentities()is applied.Consider these improvements:
protected function prepare_esc_value() { $value = $this->field['value']; - if ( is_null( $value ) ) { + if ( is_null( $value ) || '' === $value ) { return ''; } if ( is_array( $value ) ) { - $value = implode( ', ', $value ); + $separator = apply_filters( 'frm_array_value_separator', ', ', array( 'field' => $this->field ) ); + $value = implode( $separator, $value ); } if ( strpos( $value, '<' ) !== false ) { - $value = htmlentities( $value ); + $value = htmlentities( $value, ENT_QUOTES | ENT_HTML5, 'UTF-8', false ); } return str_replace( '&', '&', $value ); }Let's verify if this method is used elsewhere in the codebase:
❌ Incorrect review comment
🏁 Script executed:
#!/bin/bash # Search for method usage rg "prepare_esc_value" -A 5Length of output: 1335
Clarification on Method Usage and Improvement Suggestions
- Our search confirms that
prepare_esc_valueis indeed used—in particular, it’s called from the HTML input generation inclasses/models/fields/FrmFieldType.php. Thus, the review comment’s assertion that the method is “not used within the class” is inaccurate.- The other suggested improvements (explicit handling of empty strings, configurable array separators via a filter, and avoiding potential double-encoding by refining the
htmlentities()call) remain valid considerations for enhancing robustness.Likely an incorrect or invalid review comment.
| @@ -94,7 +94,7 @@ public function front_field_input( $args, $shortcode_atts ) { | |||
|
|
|||
| return '<textarea name="' . esc_attr( $args['field_name'] ) . '" id="' . esc_attr( $args['html_id'] ) . '" ' . | |||
| $rows . $input_html . '>' . | |||
| FrmAppHelper::esc_textarea( $this->field['value'] ) . | |||
| str_replace( '&', '&', FrmAppHelper::esc_textarea( $this->field['value'] ) ) . | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify security implications of entity replacement.
While the change fixes the HTML entities display issue, replacing & with & after escaping might have security implications or affect HTML form validation.
Let's verify how WordPress core handles similar cases:
Consider these alternatives:
- Use WordPress's
wp_specialchars_decode()with controlled flags. - Apply the entity replacement before escaping.
-str_replace( '&', '&', FrmAppHelper::esc_textarea( $this->field['value'] ) ) .
+FrmAppHelper::esc_textarea( wp_specialchars_decode( $this->field['value'], ENT_QUOTES ) ) .🌐 Web query:
How does WordPress handle HTML entities in textarea fields?
💡 Result:
WordPress handles HTML entities in textarea fields through a combination of core sanitization functions and plugin-specific processing layers. Here's the key technical breakdown:
Core Sanitization Process
-
Automatic Entity Conversion:
WordPress automatically converts special characters to HTML entities during database operations using functions likewp_filter_post_kses()[1][5]. For example:& becomes & < becomes <
-
Textarea-Specific Escaping:
- Uses
esc_textarea()for frontend rendering to prevent XSS attacks[5] - Preserves raw input when using
get_post_field('post_content')but applies entity conversion during display[3][7]
- Uses
ACF-Specific Behavior
-
Field Type Differences:
- Text/Textarea Fields: Strip HTML tags by default[3]
- WYSIWYG Fields: Preserve HTML but require unescaped output[3]
-
Common Pitfalls:
// Double-encoding occurs when manually escaping before update_field() update_field('textarea', htmlentities($input)); // Bad practice[1]
Workflow Solutions
-
Safe Input Handling:
// For plain text storage (ACF text/textarea): update_field('field_key', sanitize_textarea_field($_POST['content'])); // For HTML content (ACF WYSIWYG): update_field('field_key', wp_kses_post($_POST['content']));
-
Output Best Practices:
// Text/Textarea (escape output): echo esc_textarea(get_field('text_field')); // WYSIWYG (raw HTML output): echo wp_kses_post(get_field('wysiwyg_field'));
Developers should avoid manual htmlentities() calls when using ACF's API[1][3], as WordPress and ACF already handle encoding through their respective pipelines. The formatting setting in ACF field configurations ("Convert new lines to
") directly affects whether \n characters become <br> tags during rendering[3][9].
Citations:
- 1: https://support.advancedcustomfields.com/forums/topic/update_field-htmlentities-problem/
- 2: https://gigapress.net/how-to-add-html-to-wordpress-text-area-a-step-by-step-guide/
- 3: https://wplake.org/blog/acf-text-fields/
- 4: https://developer.wordpress.org/block-editor/reference-guides/components/textarea-control/
- 5: https://www.youtube.com/watch?v=_yf_CaoLdw4
- 6: HTML tags in Textarea field WordPress/gutenberg#54758
- 7: https://www.webmasterworld.com/php/4673541.htm
- 8: Unexpected encoding of ampersand (&) to & WordPress/gutenberg#11789
- 9: https://wordpress.com/support/wordpress-editor/blocks/custom-html-block/
- 10: https://developer.wordpress.org/block-editor/reference-guides/packages/packages-html-entities/
Security concern: Directly replacing escaped entities may undermine output safety.
- The current code applies
FrmAppHelper::esc_textarea()and then usesstr_replace( '&', '&', … )to alter the escaped output. This can inadvertently reintroduce unescaped ampersands into the rendered textarea. - WordPress’s core approach uses
esc_textarea()to ensure proper escaping. When necessary, decoding should occur before escaping (for example, usingwp_specialchars_decode( $value, ENT_QUOTES )), which avoids bypassing built-in XSS protections. - Given the potential risk of exposing unsanitized characters, consider refactoring the code as suggested:
- str_replace( '&', '&', FrmAppHelper::esc_textarea( $this->field['value'] ) ) . + FrmAppHelper::esc_textarea( wp_specialchars_decode( $this->field['value'], ENT_QUOTES ) ) .
This approach aligns better with WordPress escaping best practices and minimizes potential security vulnerabilities.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| str_replace( '&', '&', FrmAppHelper::esc_textarea( $this->field['value'] ) ) . | |
| FrmAppHelper::esc_textarea( wp_specialchars_decode( $this->field['value'], ENT_QUOTES ) ) . |
AbdiTolesa
changed the title
Fix https://github.com/Strategy11/formidable-pro/issues/5613
Test procedure
<).