WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 16 months ago

#21613 new defect (bug)

format_to_edit runs esc_textarea if $richedit param is set to false, not true

Reported by: vhauri Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version:
Component: Formatting Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Currently, the docs on format_to_edit() indicate that it runs the content through esc_textarea (which in turn runs htmlspecialchars() ) if the $richedit param is set to true. The code, however, runs the filter if the param is not set (or passed as false, see line 1270).

1255 /**
1256  * Acts on text which is about to be edited.
1257  *
1258  * The $content is run through esc_textarea(), which uses htmlspecialchars(
1259  * to convert special characters to HTML entities. If $richedit is set to t
1260  * it is simply a holder for the 'format_to_edit' filter.
1261  *
1262  * @since 0.71
1263  *
1264  * @param string $content The text about to be edited.
1265  * @param bool $richedit Whether the $content should not pass through htmls
1266  * @return string The text after the filter (and possibly htmlspecialchars(
1267  */
1268 function format_to_edit( $content, $richedit = false ) {
1269   $content = apply_filters( 'format_to_edit', $content );
1270   if ( ! $richedit )
1271     $content = esc_textarea( $content );
1272   return $content;
1273 }
1274 

My thought is the if statement should evaluate whether $richedit is true, rather than false, and therefore apply the esc_textarea function only when explicitly passed as a param. This would, however, result in unexpected behavior for anyone currently passing only the default $content param and getting sanitized output.

Attachments (1)

21613.diff (441 bytes) - added by vhauri 3 years ago.
patch changes when esc_textarea is called on the content (i.e. when $richedit is true rather than false)

Download all attachments as: .zip

Change History (6)

@vhauri3 years ago

patch changes when esc_textarea is called on the content (i.e. when $richedit is true rather than false)

comment:1 @vhauri3 years ago

  • Keywords has-patch added

comment:2 follow-up: @SergeyBiryukov3 years ago

Currently, the docs on format_to_edit() indicate that it runs the content through esc_textarea (which in turn runs htmlspecialchars() ) if the $richedit param is set to true.

The docs actually say that htmlspecialchars() will be applied if $richedit is false:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/formatting.php#L1256

Current behaviour appears to be proper.

comment:3 in reply to: ↑ 2 ; follow-up: @vhauri3 years ago

Replying to SergeyBiryukov:

Currently, the docs on format_to_edit() indicate that it runs the content through esc_textarea (which in turn runs htmlspecialchars() ) if the $richedit param is set to true.

The docs actually say that htmlspecialchars() will be applied if $richedit is false:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/formatting.php#L1256

Current behaviour appears to be proper.

Well, the code comment was recently updated to reflect the fact that the behavior is backwards( 'Whether the $content should pass through htmlspecialchars().' has become 'Whether the $content should NOT pass through htmlspecialchars().'). I'm not sure the variable name $richedit makes sense anymore...perhaps $dont_encode = false would make more sense. Either way, having a variable that a) isn't really descriptive and b) written as a double negative is probably not ideal for clarity.

Version 1, edited 3 years ago by vhauri (previous) (next) (diff)

comment:4 in reply to: ↑ 3 @SergeyBiryukov3 years ago

Replying to vhauri:

Well, the code comment was recently updated (between v 3.3.3 and 3.4)

Related: [19982]

comment:5 @nacin16 months ago

  • Component changed from General to Formatting
  • Keywords 2nd-opinion added
Note: See TracTickets for help on using tickets.