Skip to content

Conversation

@Hywan
Copy link
Member

@Hywan Hywan commented Sep 10, 2025

The first patch adds the timestamp field to TimelineEvent. It's a copy of the origin_server_ts value, parsed as an Option<MilliSecondsSinceUnixEpoch>. It's None if the parsing failed, or if the TimelineEvent was deserialised from a version before this new field was added.

A new extract_timestamp function is added for this purpose. It protects against malicious origin_server_ts where the value can be set to year 2100 for example. The only protection we are adding here is to take the min(origin_server_ts, now()), so that the event can never been “in the future”.

It doesn't protect against a malicious value like 0. It's non-trivial to define a minimum timestamp for an event.

When a TimelineEvent is mapped from one kind to another kind, the timestamp is carried over. To achieve that, new to_decrypted and to_utd methods are added.

The second patch updates LatestEventValue::timestamp to use the new TimelineEvent::timestamp method in case of a LatestEventValue::Remote. As such, it fixes the comment in #5617 (comment).


@codspeed-hq
Copy link

codspeed-hq bot commented Sep 10, 2025

CodSpeed Performance Report

Merging #5648 will not alter performance

Comparing Hywan:fix-origin-server-ts (dae438b) with main (4cc1cd1)

Summary

✅ 49 untouched benchmarks

@Hywan Hywan force-pushed the fix-origin-server-ts branch 3 times, most recently from 43029f5 to 31a0943 Compare September 10, 2025 11:37
@codecov
Copy link

codecov bot commented Sep 10, 2025

Codecov Report

❌ Patch coverage is 88.66995% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.36%. Comparing base (2248bbf) to head (31a0943).

Files with missing lines Patch % Lines
...es/matrix-sdk-common/src/deserialized_responses.rs 80.00% 23 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #5648    +/-   ##
========================================
  Coverage   88.35%   88.36%            
========================================
  Files         354      354            
  Lines       97059    97218   +159     
  Branches    97059    97218   +159     
========================================
+ Hits        85752    85902   +150     
- Misses       7261     7269     +8     
- Partials     4046     4047     +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Hywan Hywan force-pushed the fix-origin-server-ts branch from 31a0943 to dae438b Compare September 10, 2025 15:19
This patch adds the `timestamp` field to `TimelineEvent`.
It's a copy of the `origin_server_ts` value, parsed as an
`Option<MilliSecondsSinceUnixEpoch>`. It's `None` if the parsing failed,
or if the `TimelineEvent` was deserialised from a version before this
new field was added.

A new `extract_timestamp` function is added for this purpose. It
protects against malicious `origin_server_ts` where the value can be
set to year 2100 for example. The only protection we are adding here is
to take the `min(origin_server_ts, now())`, so that the event can never
been “in the future”.

It doesn't protect against a malicious value like 0. It's non-trivial to
define a minimum timestamp for an event.

When a `TimelineEvent` is mapped from one kind to another kind, the
`timestamp` is carried over. To achieve that, new `to_decrypted` and
`to_utd` methods are added.

The rest of the code is updated accordingly.
@Hywan Hywan marked this pull request as ready for review September 10, 2025 15:20
@Hywan Hywan requested a review from a team as a code owner September 10, 2025 15:20
@Hywan Hywan requested review from stefanceriu and removed request for a team September 10, 2025 15:20
@Hywan Hywan force-pushed the fix-origin-server-ts branch from dae438b to e89f368 Compare September 10, 2025 15:20
…:timestamp` method.

This patch updates `LatestEventValue::timestamp` to use
the new `TimelineEvent::timestamp` method in case of a
`LatestEventValue::Remote`.
@Hywan Hywan force-pushed the fix-origin-server-ts branch from e89f368 to a4348d4 Compare September 10, 2025 15:27
Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible to me 👍

@Hywan Hywan merged commit b788ba0 into matrix-org:main Sep 12, 2025
53 checks passed
Hywan added a commit to Hywan/matrix-rust-sdk that referenced this pull request Sep 12, 2025
…::timestamp`.

After the merge of
matrix-org#5648, we want
all events to get a `TimelineEvent::timestamp` value (extracted from
`origin_server_ts`).

To accomplish that, we are emptying the event cache. New synced events
will be built correctly, with a valid `TimelineEvent::timestamp`,
allowing a clear, stable situation.
Hywan added a commit to Hywan/matrix-rust-sdk that referenced this pull request Sep 12, 2025
…::timestamp`.

After the merge of
matrix-org#5648, we want
all events to get a `TimelineEvent::timestamp` value (extracted from
`origin_server_ts`).

To accomplish that, we are emptying the event cache. New synced events
will be built correctly, with a valid `TimelineEvent::timestamp`,
allowing a clear, stable situation.
Hywan added a commit that referenced this pull request Sep 12, 2025
…::timestamp`.

After the merge of
#5648, we want
all events to get a `TimelineEvent::timestamp` value (extracted from
`origin_server_ts`).

To accomplish that, we are emptying the event cache. New synced events
will be built correctly, with a valid `TimelineEvent::timestamp`,
allowing a clear, stable situation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants