Opened 6 years ago
Last modified 6 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: |
|
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
@
6 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
@
6 years ago
Replying to TimothyBlynJacobs:
Why
If-Unmodified-Sinceinstead 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_modifiedlike metadata, terms, and any other custom fields.
I suppose if we wanted to get similar benefits, we could force apost_modifiedupdate 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_modifiedwould only track changes to the post resource a benefit?
The post_modified would probably be an 80% solution.
If the
If-Unmodified-Sinceis used I think we should definitely sendLast-Modifiedas a header, and ideally supportIf-Modified-Sinceas well.Last-Modifiedcan 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_itemmethod would need to compute theETagfrom serializing the response data. - The
update_itemmethod would need to callget_itemif the request has anIf-Matchheader, and then compare theget_itemmethod's responseETagwith thePUTrequest'sIf-Matchheader 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' => 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