Skip to content

Commit 0d77c08

Browse files
committed
Collaboration: Deduplicate awareness query for client_id ownership check
check_permissions() called get_awareness_state() to validate client_id ownership, then process_awareness_update() called the exact same query to build the response. Both used the same 30s timeout. Moves the ownership check into process_awareness_update() so a single get_awareness_state() call serves both purposes. The method now returns WP_Error on ownership violations, checked in handle_request(). The current client's awareness state is run through wp_json_encode / json_decode to match the round-trip path other clients' states take through the DB, preventing inconsistencies from encoding normalization. Note: ownership validation now runs per-room inside handle_request() rather than for all rooms upfront in check_permissions(). In a multi-room request where room 2 has a conflict, room 1's awareness upsert will have already completed. This is acceptable because awareness upserts are idempotent heartbeats. Saves 1 query per room per poll.
1 parent 8ce0822 commit 0d77c08

1 file changed

Lines changed: 38 additions & 21 deletions

File tree

src/wp-includes/collaboration/class-wp-http-polling-collaboration-server.php

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -200,24 +200,10 @@ public function check_permissions( WP_REST_Request $request ) {
200200
);
201201
}
202202

203-
$rooms = $request['rooms'];
204-
$wp_user_id = get_current_user_id();
203+
$rooms = $request['rooms'];
205204

206205
foreach ( $rooms as $room ) {
207-
$client_id = $room['client_id'];
208-
$room = $room['room'];
209-
210-
// Check that the client_id is not already owned by another user.
211-
$existing_awareness = $this->storage->get_awareness_state( $room );
212-
foreach ( $existing_awareness as $entry ) {
213-
if ( $client_id === $entry['client_id'] && $wp_user_id !== $entry['wp_user_id'] ) {
214-
return new WP_Error(
215-
'rest_cannot_edit',
216-
__( 'Client ID is already in use by another user.' ),
217-
array( 'status' => rest_authorization_required_code() )
218-
);
219-
}
220-
}
206+
$room = $room['room'];
221207

222208
$type_parts = explode( '/', $room, 2 );
223209
$object_parts = explode( ':', $type_parts[1] ?? '', 2 );
@@ -263,9 +249,13 @@ public function handle_request( WP_REST_Request $request ) {
263249
$cursor = $room_request['after'];
264250
$room = $room_request['room'];
265251

266-
// Merge awareness state.
252+
// Merge awareness state (also validates client_id ownership).
267253
$merged_awareness = $this->process_awareness_update( $room, $client_id, $awareness );
268254

255+
if ( is_wp_error( $merged_awareness ) ) {
256+
return $merged_awareness;
257+
}
258+
269259
// The lowest client ID is nominated to perform compaction when needed.
270260
$is_compactor = false;
271261
if ( count( $merged_awareness ) > 0 ) {
@@ -371,24 +361,51 @@ private function can_user_collaborate_on_entity_type( string $entity_kind, strin
371361
/**
372362
* Processes and stores an awareness update from a client.
373363
*
364+
* Also validates that the client_id is not already owned by another user.
365+
* This check uses the same get_awareness_state() query that builds the
366+
* response, eliminating a duplicate query that was previously performed
367+
* in check_permissions().
368+
*
374369
* @since 7.0.0
375370
*
376371
* @param string $room Room identifier.
377372
* @param int $client_id Client identifier.
378373
* @param array<string, mixed>|null $awareness_update Awareness state sent by the client.
379-
* @return array<int, array<string, mixed>> Map of client ID to awareness state.
374+
* @return array<int, array<string, mixed>>|WP_Error Map of client ID to awareness state, or WP_Error if client_id is owned by another user.
380375
*/
381-
private function process_awareness_update( string $room, int $client_id, ?array $awareness_update ): array {
376+
private function process_awareness_update( string $room, int $client_id, ?array $awareness_update ) {
377+
$wp_user_id = get_current_user_id();
378+
379+
// Check ownership before upserting so a hijacked client_id is rejected.
380+
$entries = $this->storage->get_awareness_state( $room, self::AWARENESS_TIMEOUT );
381+
382+
foreach ( $entries as $entry ) {
383+
if ( $client_id === $entry['client_id'] && $wp_user_id !== $entry['wp_user_id'] ) {
384+
return new WP_Error(
385+
'rest_cannot_edit',
386+
__( 'Client ID is already in use by another user.' ),
387+
array( 'status' => rest_authorization_required_code() )
388+
);
389+
}
390+
}
391+
382392
if ( null !== $awareness_update ) {
383-
$this->storage->set_awareness_state( $room, $client_id, $awareness_update, get_current_user_id() );
393+
$this->storage->set_awareness_state( $room, $client_id, $awareness_update, $wp_user_id );
384394
}
385395

386-
$entries = $this->storage->get_awareness_state( $room, self::AWARENESS_TIMEOUT );
387396
$response = array();
388397
foreach ( $entries as $entry ) {
389398
$response[ $entry['client_id'] ] = $entry['state'];
390399
}
391400

401+
// Other clients' states were decoded from the DB. Run the current
402+
// client's state through the same encode/decode path so the response
403+
// is consistent — wp_json_encode may normalize values (e.g. strip
404+
// invalid UTF-8) that would otherwise differ on the next poll.
405+
if ( null !== $awareness_update ) {
406+
$response[ $client_id ] = json_decode( wp_json_encode( $awareness_update ), true );
407+
}
408+
392409
return $response;
393410
}
394411

0 commit comments

Comments
 (0)