Opened 5 years ago
Last modified 5 years ago
#47676 new defect (bug)
Add support for If-Unmodified-Since header to make a conditional request when updating posts via REST API (for offline and collaborative editing)
Reported by: | westonruter | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Editor | Keywords: | has-patch has-unit-tests |
Focuses: | rest-api | Cc: |
Description
In order to guard against accidentally overwriting someone else's changes when updating a post via the REST API, there should be support added for Conditional Requests. If a post update request includes an If-Modified-Since header and the supplied date is before the post's post_modified_gmt
then the request should fail with a 412 Precondition Failed response, with no change to the post being made.
It seems generally useful to include this If-Unmodified-Since
header when making requests to update posts in the editor as additional protection on top of post locking.
Additionally, this header will be important when adding support for offline editing, in particular when a service worker (see PWA feature plugin) is sending queued REST API requests which were made while the user was offline (via background sync). Since it could be several hours or days between when a post was edited and the user goes back online, there needs to be protection against overwriting the post on the server if it has been modified in the mean time.
When the service worker uses background sync to re-send the request and it fails with a 412 Precondition Failed response, this would then be a signal for the service worker to re-submit the changes as an autosave revision which could then be shown to the user via some conflict resolution UI the next time they access the editor.
As such, this is also important for the collaborative editing in a future phase.
See also comments at https://github.com/WordPress/gutenberg/issues/6322#issuecomment-410578818
Attachments (1)
Change History (6)
#2
follow-up:
↓ 3
@
5 years ago
Why If-Unmodified-Since
instead of using ETag's? An ETag could be built by serializing the item response. That would handle updates to the entity that don't trigger post_modified
like metadata, terms, and any other custom fields.
Or is the fact that post_modified
would only track changes to the post resource a benefit?
One downside to an ETag is it'd be more intensive to calculate. I suppose if we wanted to get similar benefits, we could force a post_modified
update whenever a post entity is updated through the posts controller.
If the If-Unmodified-Since
is used I think we should definitely send Last-Modified
as a header, and ideally support If-Modified-Since
as well. Last-Modified
can also be sent as a targetHint so it is accessible in the collection route.
#3
in reply to:
↑ 2
@
5 years ago
Replying to TimothyBlynJacobs:
Why
If-Unmodified-Since
instead of using ETag's? An ETag could be built by serializing the item response.
Good question. I didn't think about that. One issue with using ETag generated from serializing the response is that it would require always querying data for all fields, even if they are not requested by the user. For example, if someone had obtained the entity originally via /wp-json/wp/v2/posts/2553?_fields=id,title,link
and then they modify the title
and PUT
back the modified post, then the ETag would not have been generated to include the content
. Or perhaps this doesn't matter because you only requested the title
in the first place?
That would handle updates to the entity that don't trigger
post_modified
like metadata, terms, and any other custom fields.
I suppose if we wanted to get similar benefits, we could force apost_modified
update whenever a post entity is updated through the posts controller.
That is a very good point. For custom fields, the only thing to protect against that would be to store some additional last_modified
in postmeta and bump it whenever postmeta is changed or term associations are changed, such as via the posts controller. But that's not ideal.
Or is the fact that
post_modified
would only track changes to the post resource a benefit?
The post_modified
would probably be an 80% solution.
If the
If-Unmodified-Since
is used I think we should definitely sendLast-Modified
as a header, and ideally supportIf-Modified-Since
as well.Last-Modified
can also be sent as a targetHint so it is accessible in the collection route.
Another great point. Yes, sending the Last-Modified
header as derived from post_modified_gmt
is something I missed. A client should not be trying to convert the post_modified_gmt
into a value to for the If-Unmodified-Since
header.
One downside to an ETag is it'd be more intensive to calculate.
I see it being more intensive in two ways:
- The
get_item
method would need to compute theETag
from serializing the response data. - The
update_item
method would need to callget_item
if the request has anIf-Match
header, and then compare theget_item
method's responseETag
with thePUT
request'sIf-Match
header to see if there is a match.
In other words:
-
src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php
a b class WP_REST_Posts_Controller extends WP_REST_Controller { 477 477 $response->link_header( 'alternate', get_permalink( $post->ID ), array( 'type' => 'text/html' ) ); 478 478 } 479 479 480 $response->set_headers( 481 array( 482 'ETag' => sprintf( '"%s"', md5( serialize( $response->get_data() ) ) ), 483 ) 484 ); 485 480 486 return $response; 481 487 } 482 488 … … class WP_REST_Posts_Controller extends WP_REST_Controller { 685 691 return $valid_check; 686 692 } 687 693 694 // Handle conditional request. 695 if ( $request->get_header( 'If-Match' ) ) { 696 $get_request = clone $request; 697 $get_request->set_method( 'GET' ); 698 $get_request->set_body( null ); 699 $etag = md5( serialize( $this->get_item( $get_request )->get_data() ) ); 700 if ( $etag !== trim( $request->get_header( 'If-Match' ), '"' ) ) { 701 return new WP_Error( 'rest_precondition_failed', __( 'Post has been changed on server. Please resolve conflicts and try again' ), array( 'status' => 412 ) ); 702 } 703 } 704 688 705 $post = $this->prepare_item_for_database( $request ); 689 706 690 707 if ( is_wp_error( $post ) ) {
https://github.com/westonruter/wordpress-develop/pull/3