Skip to content

Commit 3122867

Browse files
committed
feat(sqlite): Detect cycles when loading last chunk of LinkedChunk.
This patch updates `SqliteEventCacheStore::load_last_chunk` to detect cycle for the last chunk only.
1 parent a45e058 commit 3122867

File tree

1 file changed

+61
-14
lines changed

1 file changed

+61
-14
lines changed

crates/matrix-sdk-sqlite/src/event_cache_store.rs

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -625,24 +625,26 @@ impl EventCacheStore for SqliteEventCacheStore {
625625
.acquire()
626626
.await?
627627
.with_transaction(move |txn| -> Result<_> {
628-
// Find the latest chunk identifier to generate a `ChunkIdentifierGenerator`.
629-
let chunk_identifier_generator = match txn
628+
// Find the latest chunk identifier to generate a `ChunkIdentifierGenerator`, and count the number of chunks.
629+
let (chunk_identifier_generator, number_of_chunks) = txn
630630
.prepare(
631-
"SELECT MAX(id) FROM linked_chunks WHERE room_id = ?"
631+
"SELECT MAX(id), COUNT(*) FROM linked_chunks WHERE room_id = ?"
632632
)?
633633
.query_row(
634634
(&hashed_room_id,),
635635
|row| {
636-
// Read the `MAX(id)` as an `Option<u64>` instead
637-
// of `u64` in case the `SELECT` returns nothing.
638-
// Indeed, if it returns no line, the `MAX(id)` is
639-
// set to `Null`.
640-
row.get::<_, Option<u64>>(0)
636+
Ok((
637+
// Read the `MAX(id)` as an `Option<u64>` instead
638+
// of `u64` in case the `SELECT` returns nothing.
639+
// Indeed, if it returns no line, the `MAX(id)` is
640+
// set to `Null`.
641+
row.get::<_, Option<u64>>(0)?,
642+
row.get::<_, u64>(1)?,
643+
))
641644
}
642-
)
643-
.optional()?
644-
.flatten()
645-
{
645+
)?;
646+
647+
let chunk_identifier_generator = match chunk_identifier_generator {
646648
Some(last_chunk_identifier) => {
647649
ChunkIdentifierGenerator::new_from_previous_chunk_identifier(
648650
ChunkIdentifier::new(last_chunk_identifier)
@@ -668,8 +670,23 @@ impl EventCacheStore for SqliteEventCacheStore {
668670
)
669671
.optional()?
670672
else {
671-
// Chunk is not found.
672-
return Ok((None, chunk_identifier_generator));
673+
// Chunk is not found and there is zero chunk for this room, this is consistent, all
674+
// good.
675+
if number_of_chunks == 0 {
676+
return Ok((None, chunk_identifier_generator));
677+
}
678+
// Chunk is not found **but** there are chunks for this room, this is inconsistent. The
679+
// linked chunk is malformed.
680+
//
681+
// Returning `Ok((None, _))` would be invalid here: we must return an error.
682+
else {
683+
return Err(Error::InvalidData {
684+
details:
685+
"last chunk is not found but chunks exist: the linked chunk contains a cycle"
686+
.to_owned()
687+
}
688+
)
689+
}
673690
};
674691

675692
// Build the chunk.
@@ -2040,6 +2057,36 @@ mod tests {
20402057
}
20412058
}
20422059

2060+
#[async_test]
2061+
async fn test_load_last_chunk_with_a_cycle() {
2062+
let room_id = room_id!("!r0:matrix.org");
2063+
let store = get_event_cache_store().await.expect("creating cache store failed");
2064+
2065+
store
2066+
.handle_linked_chunk_updates(
2067+
room_id,
2068+
vec![
2069+
Update::NewItemsChunk {
2070+
previous: None,
2071+
new: ChunkIdentifier::new(0),
2072+
next: None,
2073+
},
2074+
Update::NewItemsChunk {
2075+
// Because `previous` connects to chunk #0, it will create a cycle.
2076+
// Chunk #0 will have a `next` set to chunk #1! Consequently, the last chunk
2077+
// **does not exist**. We have to detect this cycle.
2078+
previous: Some(ChunkIdentifier::new(0)),
2079+
new: ChunkIdentifier::new(1),
2080+
next: Some(ChunkIdentifier::new(0)),
2081+
},
2082+
],
2083+
)
2084+
.await
2085+
.unwrap();
2086+
2087+
store.load_last_chunk(room_id).await.unwrap_err();
2088+
}
2089+
20432090
#[async_test]
20442091
async fn test_load_previous_chunk() {
20452092
let room_id = room_id!("!r0:matrix.org");

0 commit comments

Comments
 (0)