Unslash post content before parsing during save #3455
Conversation
lib/blocks.php
Outdated
| // WP_REST_Posts_Controller slashes post data before inserting/updating | ||
| // a post. This data gets unslashed by `wp_insert_post` right before | ||
| // saving to the DB. The PEG parser needs unslashed input in order to | ||
| // properly parse JSON attributes. |
There was a problem hiding this comment.
Does this mean we need to "slash" data back after our custom behavior?
There was a problem hiding this comment.
Ah, yes, I left that line out. Amended. (Initially, not slashing back didn't break anything for me, but obviously any other piece consuming the hook should expect slashed data, as the signature for the hook states.)
Codecov Report
@@ Coverage Diff @@
## master #3455 +/- ##
==========================================
+ Coverage 33.85% 34.42% +0.56%
==========================================
Files 252 252
Lines 6731 6922 +191
Branches 1221 1284 +63
==========================================
+ Hits 2279 2383 +104
- Misses 3755 3824 +69
- Partials 697 715 +18
Continue to review full report at Codecov.
|
mcsf
force-pushed
the
fix/unslash-wp-insert-post-data
branch
from
8ac56b6
to
b013025
Compare
lib/blocks.php
Outdated
| $data['post_content'] = gutenberg_wpautop_block_content( $data['post_content'] ); | ||
| $data['post_content'] = wp_slash( $data['post_content'] ); |
There was a problem hiding this comment.
Minor but It might be better to avoid storing any unslashed data in $data[ 'post_content' ] and use a temporary variable.
mcsf
force-pushed
the
fix/unslash-wp-insert-post-data
branch
from
b013025
to
adf94b4
Compare
| @@ -220,7 +220,15 @@ function gutenberg_wpautop_block_content( $content ) { | |||
| */ | |||
| function gutenberg_wpautop_insert_post_data( $data ) { | |||
| if ( ! empty( $data['post_content'] ) && gutenberg_content_has_blocks( $data['post_content'] ) ) { | |||
| $data['post_content'] = gutenberg_wpautop_block_content( $data['post_content'] ); | |||
| // WP_REST_Posts_Controller slashes post data before inserting/updating | |||
There was a problem hiding this comment.
We can expect this filter (wp_insert_post_data ) will be called not in the context of a REST API save, but generally any save for post content. Do we need to account for this? Is it wasteful (or worse, breaking) for us to be unslashing in these other contexts?
There was a problem hiding this comment.
Good questions.
Is it breaking? My reasoning:
- A comment in
wp_insert_post(), upon creating$datafrom a bunch of fields, reads Expected slashed, referring to said fields. - At the end,
wp_insert_post()blindly unslashes all of$data, with no prior check for certain kinds of formatting in$data, which I took as another sign that it is expected thatwp_insert_postbe passed slashed data. - It would be odd if saving via the REST API would silently differ in such a crucial aspect.
- Assuming that all other actors (incl. third parties) play well and provide slashed data, there shouldn't be anything breaking about this, as that would imply that
$x !== wp_slash( wp_unslash( $x ) ).
Is it wasteful? Plausibly; we'd have to measure this. We could bypass this for non-REST-API contexts, but from my first paragraph it seems like the better default to leave it on.
Fixes #3426
See #2806 for context on wpautop filter.
Description
Per the comment in the fix, WP_REST_Posts_Controller slashes post data before inserting/updating a post. This data gets unslashed by
wp_insert_postright before saving to the DB. The PEG parser expects unslashed input in order to properly parse JSON attributes.How Has This Been Tested?
Try reproducing the parent issue.
Types of changes
Bug fix.
Checklist: