WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 15 months ago

Last modified 14 months ago

#42810 closed defect (bug) (duplicate)

Updating metadata via the REST API fails when sanitized values match existing values

Reported by: bradyvercher Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:
PR Number:

Description

WP_REST_Meta_Fields::update_meta_value() checks incoming values to make sure they don't match the existing value, otherwise update_metadata() will return false to prevent extra SQL queries.

However, the check in update_metadata() sanitizes values before doing the comparison, while the method in the REST controller does not, which causes the API request to fail when a meta sanitization callback alters the incoming value and causes it to match the existing value.

Attachments (1)

42810.diff (2.6 KB) - added by TimothyBlynJacobs 2 years ago.

Download all attachments as: .zip

Change History (15)

#1 @TimothyBlynJacobs
2 years ago

  • Keywords has-patch has-unit-tests added

This ticket was mentioned in Slack in #core-editor by bradyvercher. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-editor by jaswrks. View the logs.


23 months ago

#4 @mfolkeseth
23 months ago

I have a use case which I would like to add to this ticket. If the incoming meta value is a stringified javascript object (JSON), this if check will fail because the incoming meta value is wp_slash-ed and old value is not

This ticket was mentioned in Slack in #core-restapi by mfolkeseth. View the logs.


23 months ago

This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.


23 months ago

This ticket was mentioned in Slack in #core by mfolkeseth. View the logs.


23 months ago

#8 @dcavins
22 months ago

Note that #42069 is related.

#9 @MattGeri
19 months ago

Ticket #42069 solves the additional use-case reported by @mfolkeseth. I'd suggest that becomes the ticket to get merged.

#10 follow-up: @boonebgorges
15 months ago

  • Focuses rest-api added
  • Milestone changed from Awaiting Review to 5.0

I'd like to flag this as a Gutenberg blocker, because of the following situation:

  1. You have a post with more than one block that uses post meta attributes
  2. One or more of those blocks save their data as a JSON-encoded string, as recommended in the documentation https://wordpress.org/gutenberg/handbook/block-api/attributes/#considerations
  3. You save the post without modifying the data in one or more of the blocks

Because the REST API uses *slashed* data to compare with the existing values, but get_metadata() returns *unslashed* data, most JSON-encoded strings will look "new" to the REST API https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php?marks=299,301#L287 But update_metadata() correctly compares unslashed-to-unslashed values, so returns false. Not only does this cause an "Updating failed" notice in Gutenberg, but because of the way that the update_value() method returns early out of the foreach() loop, blocks further down the page are not processed. https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php?marks=131,163-165#L129

This ticket was mentioned in Slack in #core-editor by boone. View the logs.


15 months ago

#12 in reply to: ↑ 10 @dcavins
15 months ago

Replying to boonebgorges:
Boone, I'd like to add a test case to the tests in #42069 to cover this case. I added a case using the value

json_encode ( array(
  "one" => "Someone's Text/Value",
  "two" => 2,
  "three" => array( 1, 1, 2, 3, 5 ),
) );

but it appears to pass without the suggested fix. Can you suggest a string or encoded item that currently fails to update via the REST API that I can add to the test cases?

Thanks!

#13 @TimothyBlynJacobs
15 months ago

  • Resolution set to duplicate
  • Status changed from new to closed

Closing as a duplicate of #42069 to consolidate discussion on that ticket.

#14 @netweb
14 months ago

  • Milestone 5.0 deleted
Note: See TracTickets for help on using tickets.