diff --git a/src/api/client/read_marker/mod.rs b/src/api/client/read_marker/mod.rs index 73814157e..4633856c7 100644 --- a/src/api/client/read_marker/mod.rs +++ b/src/api/client/read_marker/mod.rs @@ -1,12 +1,73 @@ mod read_markers; mod receipt; -use ruma::{EventId, MilliSecondsSinceUnixEpoch, RoomId, UserId, events::receipt::ReceiptThread}; +use ruma::{ + EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, RoomId, UserId, + events::{ + RoomAccountDataEventType, + fully_read::{FullyReadEvent, FullyReadEventContent}, + receipt::ReceiptThread, + }, +}; use tuwunel_core::{Err, PduCount, Result, err, utils::result::LogErr}; use tuwunel_service::{Services, rooms::read_receipt::PrivateRead}; pub(crate) use self::{read_markers::set_read_marker_route, receipt::create_receipt_route}; +/// Stores the fully-read marker unless it would move the position backwards: +/// a stale device must not regress the marker another one already advanced. +/// An unresolvable position on either side accepts the write, as for public +/// receipts. +async fn set_fully_read( + services: &Services, + room_id: &RoomId, + user_id: &UserId, + event: &EventId, +) -> Result<()> { + let current: Option = services + .account_data + .get_room::(room_id, user_id, RoomAccountDataEventType::FullyRead) + .await + .ok() + .and_then(|value| { + let event_id = value.pointer("/content/event_id")?.as_str()?; + + EventId::parse(event_id).ok() + }); + + let advances = match current { + | Some(current) => { + let current = services + .timeline + .get_pdu_count(¤t) + .await + .ok(); + let incoming = services.timeline.get_pdu_count(event).await.ok(); + + current + .zip(incoming) + .is_none_or(|(current, incoming)| incoming > current) + }, + | None => true, + }; + + if !advances { + return Ok(()); + } + + services + .account_data + .update( + Some(room_id), + user_id, + RoomAccountDataEventType::FullyRead, + &serde_json::to_value(FullyReadEvent { + content: FullyReadEventContent { event_id: event.to_owned() }, + })?, + ) + .await +} + /// Resolves `event` to its timeline position and stores the private read /// marker for `thread` there. /// diff --git a/src/api/client/read_marker/read_markers.rs b/src/api/client/read_marker/read_markers.rs index f5473c34e..c019f7c3b 100644 --- a/src/api/client/read_marker/read_markers.rs +++ b/src/api/client/read_marker/read_markers.rs @@ -4,16 +4,12 @@ use axum::extract::State; use ruma::{ MilliSecondsSinceUnixEpoch, api::client::read_marker::set_read_marker, - events::{ - RoomAccountDataEventType, - fully_read::{FullyReadEvent, FullyReadEventContent}, - receipt::{Receipt, ReceiptEvent, ReceiptEventContent, ReceiptThread, ReceiptType}, - }, + events::receipt::{Receipt, ReceiptEvent, ReceiptEventContent, ReceiptThread, ReceiptType}, }; use tuwunel_core::Result; use tuwunel_service::presence::Ping; -use super::{reset_and_refresh_badge, set_private_marker}; +use super::{reset_and_refresh_badge, set_fully_read, set_private_marker}; use crate::{ClientIp, Ruma}; /// # `POST /_matrix/client/r0/rooms/{roomId}/read_markers` @@ -31,20 +27,7 @@ pub(crate) async fn set_read_marker_route( let sender_user = body.sender_user(); if let Some(event) = &body.fully_read { - let fully_read_event = FullyReadEvent { - content: FullyReadEventContent { event_id: event.clone() }, - }; - - services - .account_data - .update( - Some(&body.room_id), - sender_user, - RoomAccountDataEventType::FullyRead, - &serde_json::to_value(fully_read_event)?, - ) - .await - .ok(); + set_fully_read(&services, &body.room_id, sender_user, event).await?; } let private_advanced = match &body.private_read_receipt { diff --git a/src/api/client/read_marker/receipt.rs b/src/api/client/read_marker/receipt.rs index 346724050..824710dfd 100644 --- a/src/api/client/read_marker/receipt.rs +++ b/src/api/client/read_marker/receipt.rs @@ -4,16 +4,12 @@ use axum::extract::State; use ruma::{ MilliSecondsSinceUnixEpoch, api::client::receipt::create_receipt::{self, v3::ReceiptType as CreateReceiptType}, - events::{ - RoomAccountDataEventType, - fully_read::{FullyReadEvent, FullyReadEventContent}, - receipt::{Receipt, ReceiptEvent, ReceiptEventContent, ReceiptThread, ReceiptType}, - }, + events::receipt::{Receipt, ReceiptEvent, ReceiptEventContent, ReceiptThread, ReceiptType}, }; use tuwunel_core::{Err, Result}; use tuwunel_service::presence::Ping; -use super::{reset_and_refresh_badge, set_private_marker}; +use super::{reset_and_refresh_badge, set_fully_read, set_private_marker}; use crate::{ClientIp, Ruma}; /// # `POST /_matrix/client/r0/rooms/{roomId}/receipt/{receiptType}/{eventId}` @@ -72,18 +68,7 @@ pub(crate) async fn create_receipt_route( let advanced = match body.receipt_type { | CreateReceiptType::FullyRead => { - let fully_read_event = FullyReadEvent { - content: FullyReadEventContent { event_id: body.event_id.clone() }, - }; - services - .account_data - .update( - Some(&body.room_id), - sender_user, - RoomAccountDataEventType::FullyRead, - &serde_json::to_value(fully_read_event)?, - ) - .await?; + set_fully_read(&services, &body.room_id, sender_user, &body.event_id).await?; false }, diff --git a/src/main/tests/fully_read_monotonic.rs b/src/main/tests/fully_read_monotonic.rs new file mode 100644 index 000000000..8b4b0103f --- /dev/null +++ b/src/main/tests/fully_read_monotonic.rs @@ -0,0 +1,260 @@ +#![cfg(test)] + +use std::{ + env::var, fs::remove_dir_all, net::TcpListener, path::PathBuf, process::id as process_id, + time::Duration, +}; + +use futures::future::join; +use serde_json::{Value, json}; +use tokio::time::{sleep, timeout}; +use tuwunel::{Args, Runtime, Server, async_run, async_start, async_stop}; +use tuwunel_core::{Result, err, ruma::UserId}; +use tuwunel_service::{Services, users::Register}; + +struct DatabasePath(PathBuf); + +impl Drop for DatabasePath { + fn drop(&mut self) { remove_dir_all(&self.0).ok(); } +} + +/// A stale device posting an older `m.fully_read` must not regress the marker +/// that a newer one already advanced. +#[test] +fn fully_read_marker_is_monotonic() -> Result { + let listener = TcpListener::bind(("127.0.0.1", 0))?; + let port = listener.local_addr()?.port(); + + let root = var("TMPDIR").unwrap_or_else(|_| "/nvme/target/tmp".into()); + let db_path = DatabasePath( + PathBuf::from(root).join(format!("tuwunel-fully-read-monotonic-{}", process_id())), + ); + let mut args = Args::default_test(&["fresh", "cleanup"]); + + args.option.extend([ + format!("database_path={:?}", db_path.0), + "address=[\"127.0.0.1\"]".to_owned(), + format!("port={port}"), + "listening=true".to_owned(), + ]); + + let runtime = Runtime::new(Some(&args))?; + let server = Server::new(Some(&args), Some(&runtime))?; + let result = runtime.block_on(async { + let services = async_start(&server).await?; + let base = format!("http://127.0.0.1:{port}"); + + drop(listener); + + let exercise = async { + let outcome = exercise(&services, &base).await; + let shutdown = server.server.shutdown(); + + outcome.and(shutdown) + }; + + let (run_result, outcome) = join(async_run(&server), exercise).await; + + drop(services); + async_stop(&server).await?; + run_result?; + + outcome + }); + + drop(runtime); + result +} + +async fn exercise(services: &Services, base: &str) -> Result { + wait_until_ready(services, base).await?; + + let user_id = UserId::parse_with_server_name("fully-read", services.globals.server_name())?; + let token = "fully-read-monotonic-regression-token"; + + services + .users + .full_register(Register { + user_id: Some(&user_id), + password: Some("fully-read-password"), + ..Default::default() + }) + .await?; + + services + .users + .create_device(&user_id, None, (Some(token), None), None, None, None) + .await?; + + let room = create_room(services, base, token).await?; + let first = send_message(services, base, token, &room, "first", "txn-first").await?; + let second = send_message(services, base, token, &room, "second", "txn-second").await?; + + // A forward write stores. + read_markers(services, base, token, &room, &second).await?; + + assert_eq!(fully_read_event(services, base, token, &user_id, &room).await?, second); + + // A backwards write is accepted but must not move the marker. + read_markers(services, base, token, &room, &first).await?; + + assert_eq!(fully_read_event(services, base, token, &user_id, &room).await?, second); + + // The /receipt endpoint must not move it backwards either. + fully_read_receipt(services, base, token, &room, &first).await?; + + assert_eq!(fully_read_event(services, base, token, &user_id, &room).await?, second); + + Ok(()) +} + +async fn read_markers( + services: &Services, + base: &str, + token: &str, + room: &str, + event: &str, +) -> Result { + let response = services + .client + .clients + .default + .post(format!("{base}/_matrix/client/v3/rooms/{room}/read_markers")) + .bearer_auth(token) + .json(&json!({ + "m.fully_read": event, + "m.read": event, + "m.read.private": event, + })) + .send() + .await?; + + assert_eq!(response.status().as_u16(), 200, "read_markers: {}", response.text().await?); + + Ok(()) +} + +async fn fully_read_receipt( + services: &Services, + base: &str, + token: &str, + room: &str, + event: &str, +) -> Result { + let response = services + .client + .clients + .default + .post(format!("{base}/_matrix/client/v3/rooms/{room}/receipt/m.fully_read/{event}")) + .bearer_auth(token) + .json(&json!({})) + .send() + .await?; + + assert_eq!(response.status().as_u16(), 200, "receipt: {}", response.text().await?); + + Ok(()) +} + +async fn fully_read_event( + services: &Services, + base: &str, + token: &str, + user_id: &UserId, + room: &str, +) -> Result { + let response: Value = services + .client + .clients + .default + .get(format!( + "{base}/_matrix/client/v3/user/{user_id}/rooms/{room}/account_data/m.fully_read" + )) + .bearer_auth(token) + .send() + .await? + .error_for_status()? + .json() + .await?; + + response + .get("event_id") + .and_then(Value::as_str) + .map(str::to_owned) + .ok_or_else(|| err!("m.fully_read response omitted event_id")) +} + +async fn send_message( + services: &Services, + base: &str, + token: &str, + room: &str, + body: &str, + txn: &str, +) -> Result { + let response: Value = services + .client + .clients + .default + .put(format!("{base}/_matrix/client/v3/rooms/{room}/send/m.room.message/{txn}")) + .bearer_auth(token) + .json(&json!({"msgtype": "m.text", "body": body})) + .send() + .await? + .error_for_status()? + .json() + .await?; + + response + .get("event_id") + .and_then(Value::as_str) + .map(str::to_owned) + .ok_or_else(|| err!("send response omitted event_id")) +} + +async fn create_room(services: &Services, base: &str, token: &str) -> Result { + let response: Value = services + .client + .clients + .default + .post(format!("{base}/_matrix/client/v3/createRoom")) + .bearer_auth(token) + .json(&json!({})) + .send() + .await? + .error_for_status()? + .json() + .await?; + + response + .get("room_id") + .and_then(Value::as_str) + .map(str::to_owned) + .ok_or_else(|| err!("createRoom response omitted room_id")) +} + +async fn wait_until_ready(services: &Services, base: &str) -> Result { + let url = format!("{base}/_matrix/client/versions"); + + timeout(Duration::from_secs(10), async { + loop { + if services + .client + .clients + .default + .get(&url) + .send() + .await + .is_ok() + { + break; + } + + sleep(Duration::from_millis(20)).await; + } + }) + .await + .map_err(|_| err!("server listener did not become ready"))?; + + Ok(()) +} diff --git a/src/main/tests/receipt_replay.rs b/src/main/tests/receipt_replay.rs index 9aad698e4..19ec17dbd 100644 --- a/src/main/tests/receipt_replay.rs +++ b/src/main/tests/receipt_replay.rs @@ -27,7 +27,7 @@ impl Drop for DatabasePath { } #[test] -fn replayed_receipts_hold_their_stream_position() -> Result { +fn replayed_receipts_reannounce() -> Result { let root = var("TMPDIR").unwrap_or_else(|_| "/nvme/target/tmp".into()); let db_path = DatabasePath( PathBuf::from(root).join(format!("tuwunel-receipt-replay-{}", process_id())), @@ -86,11 +86,11 @@ async fn exercise(services: &Services) -> Result { return Err!("first receipt did not store exactly one row"); } - if replayed || after_replay != after_store { - return Err!("replayed receipt moved the receipt stream"); + if replayed || after_replay.len() != 1 || after_replay[0] <= after_store[0] { + return Err!("replayed receipt did not re-announce at a fresh position"); } - if !advanced || after_advance.len() != 1 || after_advance[0] <= after_store[0] { + if !advanced || after_advance.len() != 1 || after_advance[0] <= after_replay[0] { return Err!("receipt naming another event did not take a new position"); } @@ -132,23 +132,34 @@ async fn private_read_replay(services: &Services, room: &RoomId, user: &UserId) let stored = set_private_read(services, room, user, 5, true).await; let after_store = token().await; let replayed = set_private_read(services, room, user, 5, true).await; - let earlier = set_private_read(services, room, user, 4, true).await; let after_replay = token().await; + let earlier = set_private_read(services, room, user, 4, true).await; + let after_earlier = token().await; let advanced = set_private_read(services, room, user, 6, true).await; let after_advance = token().await; + let silent = set_private_read(services, room, user, 6, false).await; + let after_silent = token().await; if !stored || !advanced { return Err!("private marker rejected an advancing position"); } - if replayed || earlier || after_replay != after_store { - return Err!("private marker accepted a position it already held"); + if replayed || after_replay <= after_store { + return Err!("identical private marker did not re-announce"); + } + + if earlier || after_earlier != after_replay { + return Err!("older private marker was not rejected"); } - if after_advance <= after_store { + if after_advance <= after_replay { return Err!("advancing private marker did not move the update token"); } + if silent || after_silent != after_advance { + return Err!("unannounced identical private marker was not a no-op"); + } + private_read_unannounced(services, room, user).await } diff --git a/src/service/rooms/read_receipt/data.rs b/src/service/rooms/read_receipt/data.rs index cf0056aaa..d1021cedc 100644 --- a/src/service/rooms/read_receipt/data.rs +++ b/src/service/rooms/read_receipt/data.rs @@ -66,9 +66,9 @@ impl Data { /// Stores `event` as the user's receipt for its thread context, reporting /// whether it advanced. /// - /// A receipt naming the stored event, or an earlier one, is rejected - /// without allocating a stream position or writing anything. An accepted - /// receipt replaces every superseded row in one transaction. + /// Earlier events are rejected without writing. The stored event is + /// re-announced under a fresh stream position so clients which missed the + /// receipt can repair their unread state, but is not an advance. #[inline] pub(super) async fn readreceipt_update( &self, @@ -124,9 +124,13 @@ impl Data { }) .await; - if !self - .receipt_advanced(current.as_deref(), event_id) - .await + // Identical receipts re-announce but never advance. + let reannounce = current.as_deref() == Some(event_id); + + if !reannounce + && !self + .receipt_advanced(current.as_deref(), event_id) + .await { return false; } @@ -144,7 +148,7 @@ impl Data { txn.put(&self.readreceiptid_readreceipt, latest_id, Json(event)); txn.execute(); - true + !reannounce } /// Whether a receipt for `incoming` supersedes the stored one at @@ -211,6 +215,9 @@ impl Data { /// Sets the private read marker for `(room, user, thread)`, reporting /// whether it advanced. /// + /// Strictly older markers are rejected without writing. An identical + /// marker is re-announced (the sync gate bumps) but is not an advance. + /// /// Unthreaded writes use the legacy 2-tuple `(room, user)` key shape /// and sweep any pre-existing per-thread rows so the room-wide receipt /// supersedes prior thread state. Threaded writes (Main, Thread, custom) @@ -234,11 +241,21 @@ impl Data { ) -> bool { let thread_kind = thread.as_str().unwrap_or_default(); - if self + let stored = self .private_read_position(room_id, user_id, thread_kind) .await - .is_ok_and(|(stored, _)| count <= stored) - { + .ok() + .map(|(stored, _)| stored); + + // Strictly older markers are rejected. + if stored.is_some_and(|stored| count < stored) { + return false; + } + + // Identical markers re-announce but never advance. + let reannounce = stored.is_some_and(|stored| count == stored); + + if reannounce && !announce { return false; } @@ -309,7 +326,7 @@ impl Data { txn.execute(); - true + !reannounce } /// Private read position for an exact `(room, user, thread)` context. diff --git a/tests/probe-read-state.sh b/tests/probe-read-state.sh new file mode 100755 index 000000000..82e77a78b --- /dev/null +++ b/tests/probe-read-state.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# probe-read-state.sh — Diagnose a stuck read marker for a given user+room. +# Run with YOUR OWN credentials: +# RU_USER=ruka RU_PASS=... ./tests/probe-read-state.sh '!room1:server' '!room2:server' +# +# Per room it dumps the three signals a client derives its unread state from +# (notification counts, own receipts, m.fully_read), then posts /read_markers +# at the latest message (exactly what FluffyChat sends) and dumps them again. +set -euo pipefail + +BASE="${BASE_URL:-https://matrix.agiadn.org}" +USER="${RU_USER:?set RU_USER}" +PASS="${RU_PASS:?set RU_PASS}" + +LOGIN=$(curl -sS -X POST -H 'Content-Type: application/json' \ + -d "$(jq -nc --arg u "$USER" --arg p "$PASS" \ + '{type:"m.login.password", identifier:{type:"m.id.user",user:$u}, password:$p}')" \ + "$BASE/_matrix/client/v3/login") +TOKEN=$(jq -r '.access_token' <<<"$LOGIN") +MXID=$(jq -r '.user_id' <<<"$LOGIN") +[[ "$TOKEN" != "null" && -n "$TOKEN" ]] || { echo "login failed" >&2; exit 1; } + +# dump_state ROOM — print every server-side signal a client derives unread +# state from: stored m.fully_read, unread notification counts, the user's own +# latest receipts as served in /sync, and the room's actual latest event. +dump_state() { # dump_state ROOM + local room="$1" + + local fr + fr=$(curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/user/$MXID/rooms/$room/account_data/m.fully_read") + echo "m.fully_read: $fr" + + local mu + mu=$(curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/user/$MXID/rooms/$room/account_data/m.marked_unread") + echo "m.marked_unread: $mu" + mu=$(curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/user/$MXID/rooms/$room/account_data/com.famedly.marked_unread") + echo "com.famedly.marked_unread: $mu" + + local sync + sync=$(curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/sync?timeout=0") + + jq -r --arg r "$room" --arg u "$MXID" ' + .rooms.join[$r] as $j | + "unread_notifications: \($j.unread_notifications // "absent")", + ([$j.ephemeral.events[]? | select(.type=="m.receipt") | .content + | to_entries[] | .key as $ev | .value + | ((.["m.read"][$u].ts // null) as $r2 | + (.["m.read.private"][$u].ts // null) as $p | + select($r2 != null or $p != null) | + "own receipt: \($ev) m.read.ts=\($r2) m.read.private.ts=\($p)")] + | if length == 0 then "own receipt: NONE in sync" else .[] end) + ' <<<"$sync" + + curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/rooms/$room/messages?dir=b&limit=3" | + jq -r '.chunk[] | "latest events: \(.event_id) \(.type) \(.sender) ts=\(.origin_server_ts)"' +} + +for ROOM in "$@"; do + echo + echo "=== $ROOM (before) ===" + dump_state "$ROOM" + + LATEST=$(curl -sS -H "Authorization: Bearer $TOKEN" \ + "$BASE/_matrix/client/v3/rooms/$ROOM/messages?dir=b&limit=10" | + jq -r '[.chunk[] | select(.type=="m.room.message")][0].event_id // empty') + [[ -n "$LATEST" ]] || continue + + echo + echo "POST /read_markers at $LATEST ->" + curl -sS -w '\nHTTP %{http_code}\n' -X POST -H "Authorization: Bearer $TOKEN" \ + -H 'Content-Type: application/json' \ + -d "$(jq -nc --arg e "$LATEST" \ + '{"m.fully_read":$e, "m.read":$e, "m.read.private":$e}')" \ + "$BASE/_matrix/client/v3/rooms/$ROOM/read_markers" + + echo + echo "=== $ROOM (after) ===" + dump_state "$ROOM" +done diff --git a/tests/repro-fully-read-regression.sh b/tests/repro-fully-read-regression.sh new file mode 100755 index 000000000..d990a8f1a --- /dev/null +++ b/tests/repro-fully-read-regression.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# repro-fully-read-regression.sh — Checks whether the server lets a stale +# device move the m.fully_read marker BACKWARDS. +# +# Unfixed server: a read_markers/receipt request naming an older event moves +# the marker back (exit 0). Fixed server: the backwards write is ignored and +# the marker stays (exit 1). +# +# Usage: +# ALICE_PASS=... BOB_PASS=... ./tests/repro-fully-read-regression.sh +# Optional env: BASE_URL (default https://matrix.agiadn.org), +# ALICE_USER (default kimi, the reader), +# BOB_USER (default imik, the sender) +set -euo pipefail + +BASE="${BASE_URL:-https://matrix.agiadn.org}" +ALICE_USER="${ALICE_USER:-kimi}" +ALICE_PASS="${ALICE_PASS:?set ALICE_PASS}" +BOB_USER="${BOB_USER:-imik}" +BOB_PASS="${BOB_PASS:?set BOB_PASS}" + +api() { # api METHOD PATH [TOKEN] [JSON] + local method="$1" path="$2" token="${3:-}" body="${4:-}" + local args=(-sS -X "$method" -H 'Content-Type: application/json') + [[ -n "$token" ]] && args+=(-H "Authorization: Bearer $token") + [[ -n "$body" ]] && args+=(-d "$body") + curl "${args[@]}" "$BASE/_matrix/client/v3$path" +} + +login() { # login USER PASS -> "token user_id" + local resp + resp=$(api POST /login '' "$(jq -nc --arg u "$1" --arg p "$2" \ + '{type:"m.login.password", identifier:{type:"m.id.user",user:$u}, password:$p}')") + jq -e -r '.access_token' <<<"$resp" >/dev/null + echo "$(jq -r '.access_token' <<<"$resp") $(jq -r '.user_id' <<<"$resp")" +} + +read -r ALICE_TOKEN ALICE_ID <<<"$(login "$ALICE_USER" "$ALICE_PASS")" +read -r BOB_TOKEN BOB_ID <<<"$(login "$BOB_USER" "$BOB_PASS")" + +ROOM=$(api POST /createRoom "$BOB_TOKEN" \ + "$(jq -nc --arg a "$ALICE_ID" '{preset:"private_chat", invite:[$a]}')" | jq -r '.room_id') +api POST "/rooms/$ROOM/join" "$ALICE_TOKEN" >/dev/null +echo "room: $ROOM" + +send() { # send TOKEN BODY -> event_id + api PUT "/rooms/$ROOM/send/m.room.message/cli$(date +%s%N)$RANDOM" "$1" \ + "$(jq -nc --arg b "$2" '{msgtype:"m.text", body:$b}')" | jq -r '.event_id' +} + +fully_read() { # -> stored m.fully_read event id + api GET "/user/$ALICE_ID/rooms/$ROOM/account_data/m.fully_read" "$ALICE_TOKEN" | + jq -r '.event_id // empty' +} + +mark() { # mark EVENT — post read_markers like FluffyChat does + api POST "/rooms/$ROOM/read_markers" "$ALICE_TOKEN" \ + "$(jq -nc --arg e "$1" '{"m.fully_read":$e, "m.read":$e, "m.read.private":$e}')" >/dev/null +} + +FIRST=$(send "$BOB_TOKEN" "first") +SECOND=$(send "$BOB_TOKEN" "second") + +# Forward write: marker must land on the newer event. +mark "$SECOND" +STORED=$(fully_read) +echo "after forward write: $STORED" +[[ "$STORED" == "$SECOND" ]] || { echo "FAIL: forward write did not store" >&2; exit 1; } + +# Backwards write, as a stale device would send it. +mark "$FIRST" +STORED=$(fully_read) +echo "after backward write: $STORED" + +# Backwards write via the /receipt endpoint too. +api POST "/rooms/$ROOM/receipt/m.fully_read/$FIRST" "$ALICE_TOKEN" '{}' >/dev/null +STORED=$(fully_read) +echo "after /receipt write: $STORED" + +if [[ "$STORED" == "$FIRST" ]]; then + echo + echo "*** BUG REPRODUCED: m.fully_read moved backwards. ***" + exit 0 +elif [[ "$STORED" == "$SECOND" ]]; then + echo + echo "GUARD PRESENT: m.fully_read stayed at the newer event." + exit 1 +else + echo "UNEXPECTED: marker is at neither event: $STORED" >&2 + exit 2 +fi diff --git a/tests/repro-reread-noedu.sh b/tests/repro-reread-noedu.sh new file mode 100755 index 000000000..e1a5a781d --- /dev/null +++ b/tests/repro-reread-noedu.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash +# repro-reread-noedu.sh — Verify that re-reading an already-read position +# re-announces the receipt instead of going silent. +# +# A client which missed the original receipt EDU derives its unread state +# from the latest receipt it has seen; re-reading the same position is its +# only way to ask the server for the receipt again. If the duplicate receipt +# produces no EDU, the client stays stuck in a quiet room forever. +# +# Device A reads at an event; device B (same user, own session) consumes +# that receipt, then re-reads the same event. On a fixed server B's next +# sync carries the receipt EDU again (exit 0). On a buggy server the +# duplicate receipt produces no sync traffic at all (exit 1). +# +# Usage: +# ALICE_PASS=... BOB_PASS=... ./tests/repro-reread-noedu.sh +# Optional env: BASE_URL (default https://matrix.agiadn.org), +# ALICE_USER (default kimi, the reader), +# BOB_USER (default imik, the sender) + +set -euo pipefail + +BASE="${BASE_URL:-https://matrix.agiadn.org}" +ALICE_USER="${ALICE_USER:-kimi}" +ALICE_PASS="${ALICE_PASS:?set ALICE_PASS}" +BOB_USER="${BOB_USER:-imik}" +BOB_PASS="${BOB_PASS:?set BOB_PASS}" + +api() { # api METHOD PATH [TOKEN] [JSON] + local method="$1" path="$2" token="${3:-}" body="${4:-}" + local args=(-sS -X "$method" -H 'Content-Type: application/json') + [[ -n "$token" ]] && args+=(-H "Authorization: Bearer $token") + [[ -n "$body" ]] && args+=(-d "$body") + curl "${args[@]}" "$BASE/_matrix/client$path" +} + +login() { # login USER PASS [DEVICE] -> token + local token + token=$(api POST /v3/login '' "$(jq -nc --arg u "$1" --arg p "$2" --arg d "${3:-}" \ + '{type:"m.login.password", identifier:{type:"m.id.user",user:$u}, password:$p} + + (if $d == "" then {} else {device_id: $d} end)')" | jq -r '.access_token') + [[ "$token" != "null" && -n "$token" ]] || { echo "login failed for $1" >&2; exit 1; } + echo "$token" +} + +sync() { # sync TOKEN [SINCE] -> sync response JSON (timeout=0) + local url="/v3/sync?timeout=0" + [[ -n "${2:-}" ]] && url+="&since=$2" + api GET "$url" "$1" +} + +sss() { # sss TOKEN [POS] BODY -> sliding sync response JSON + local url="/unstable/org.matrix.simplified_msc3575/sync" + [[ -n "${2:-}" ]] && url+="?pos=$2" + api POST "$url" "$1" "$3" +} + +echo "== Logging in $ALICE_USER (devices A and B) and $BOB_USER on $BASE" +A_TOKEN="$(login "$ALICE_USER" "$ALICE_PASS" REREADA)" +B_TOKEN="$(login "$ALICE_USER" "$ALICE_PASS" REREADB)" +BOB_TOKEN="$(login "$BOB_USER" "$BOB_PASS")" +ALICE_ID="$(api GET /v3/account/whoami "$A_TOKEN" | jq -r '.user_id')" + +room=$(api POST /v3/createRoom "$BOB_TOKEN" \ + "$(jq -nc --arg a "$ALICE_ID" '{preset:"private_chat", invite:[$a]}')" | jq -r '.room_id') +[[ "$room" != "null" && -n "$room" ]] || { echo "createRoom failed" >&2; exit 1; } +api POST "/v3/rooms/$room/join" "$A_TOKEN" >/dev/null +echo "room: $room" + +msg=$(api PUT "/v3/rooms/$room/send/m.room.message/cli$(date +%s%N)$RANDOM" "$BOB_TOKEN" \ + '{"msgtype":"m.text","body":"re-read repro"}' | jq -r '.event_id') +[[ "$msg" != "null" && -n "$msg" ]] || { echo "send failed" >&2; exit 1; } +echo "message: $msg" + +# B learns the message. +s0=$(sync "$B_TOKEN" | jq -r '.next_batch') +s0=$(sync "$B_TOKEN" "$s0" | jq -r '.next_batch') + +# A reads at the message; B consumes the original receipt EDU. +api POST "/v3/rooms/$room/read_markers" "$A_TOKEN" \ + "$(jq -nc --arg e "$msg" '{"m.read":$e}')" >/dev/null +resp=$(sync "$B_TOKEN" "$s0") +s1=$(jq -r '.next_batch' <<<"$resp") +echo "original receipt EDU delivered to B: $(jq --arg r "$room" \ + '[.rooms.join[$r].ephemeral.events[]? | select(.type=="m.receipt")] | length' <<<"$resp")" + +# B re-reads the same position: identical to the stored receipt. +api POST "/v3/rooms/$room/read_markers" "$B_TOKEN" \ + "$(jq -nc --arg e "$msg" '{"m.read":$e}')" >/dev/null + +# v3: the duplicate receipt must re-announce as a fresh EDU. +resp=$(sync "$B_TOKEN" "$s1") +v3_edus=$(jq --arg r "$room" \ + '[.rooms.join[$r].ephemeral.events[]? | select(.type=="m.receipt")] | length' <<<"$resp") +echo "v3 receipt EDUs after duplicate re-read: $v3_edus" + +# MSC4186: same assertion on the receipts extension, the path sliding-sync +# clients (matrix-rust-sdk) consume. +p0=$(sss "$B_TOKEN" "" "$(jq -nc --arg r "$room" '{ + lists: {main: {ranges: [[0,19]], required_state: [], timeline_limit: 1}}, + room_subscriptions: {($r): {required_state: [], timeline_limit: 1}}, + extensions: {receipts: {enabled: true}} +}')" | jq -r '.pos') + +api POST "/v3/rooms/$room/read_markers" "$B_TOKEN" \ + "$(jq -nc --arg e "$msg" '{"m.read":$e}')" >/dev/null + +resp=$(sss "$B_TOKEN" "$p0" "$(jq -nc --arg r "$room" '{ + lists: {main: {ranges: [[0,19]], required_state: [], timeline_limit: 1}}, + room_subscriptions: {($r): {required_state: [], timeline_limit: 1}}, + extensions: {receipts: {enabled: true}} +}')") +v5_edus=$(jq --arg r "$room" '.extensions.receipts.rooms[$r] | length' <<<"$resp") +echo "v5 receipts-extension entries after duplicate re-read: $v5_edus" + +if [[ "$v3_edus" -ge 1 && "$v5_edus" -ge 1 ]]; then + echo "OK: duplicate re-reads re-announce the receipt; stuck clients can self-heal." + exit 0 +fi + +echo "BUG: duplicate re-read produced no receipt EDU (v3=$v3_edus v5=$v5_edus);" >&2 +echo "a client which missed the original receipt stays unread forever." >&2 +exit 1