diff --git a/.github/workflows/fix_lint.yaml b/.github/workflows/fix_lint.yaml index 3e518c6267..e0817698f4 100644 --- a/.github/workflows/fix_lint.yaml +++ b/.github/workflows/fix_lint.yaml @@ -9,7 +9,9 @@ on: env: # We use nightly so that `fmt` correctly groups together imports, and # clippy correctly fixes up the benchmarks. - RUST_VERSION: nightly-2025-06-24 + # + # Note: This should match the nightly rust version in `tests.yml`. + RUST_VERSION: nightly-2025-03-27 jobs: fixup: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6ecdd88815..aa6250612b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -13,6 +13,16 @@ concurrency: env: RUST_VERSION: 1.87.0 + # This nightly is roughly the last to be branded 1.87.0. + # + # We know this, as 1.87.0 was branched from master on 2025-03-28. Shortly + # afterwards, nightlies were then branded as 1.88.0. See + # https://releases.rs/docs/1.87.0/ for where we get the dates. + # + # Technically the last 1.87.0 nightly was 2025-03-29, but that all depends on + # at which time of day they cut the release and when the nightly is built. + # It's safer for future releases to just do the day before. + RUST_NIGHTLY_VERSION: nightly-2025-03-27 # last nightly before 1.88.0 jobs: # Job to detect what has changed so we don't run e.g. Rust checks on PRs that @@ -240,7 +250,7 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@e97e2d8cc328f1b50210efc529dca0028893a2d9 # master with: - toolchain: nightly-2026-02-01 + toolchain: ${{ env.RUST_NIGHTLY_VERSION }} components: clippy - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 @@ -296,7 +306,7 @@ jobs: with: # We use nightly so that we can use some unstable options that we use in # `.rustfmt.toml`. - toolchain: nightly-2025-04-23 + toolchain: ${{ env.RUST_NIGHTLY_VERSION }} components: rustfmt - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 @@ -677,7 +687,12 @@ jobs: - changes cargo-test: - if: ${{ needs.changes.outputs.rust == 'true' }} + # We need to explicitly specify `!cancelled() && !failure()` as otherwise + # GitHub will implicitly add `success()`, which will be `false` when any job + # in `needs`, or its dependents, is skipped. Most notably, jobs like + # `lint-readme`, a dependency of `linting-done`, are unlikely to run very + # often - and would result in this job being skipped. + if: ${{ !cancelled() && !failure() && needs.changes.outputs.rust == 'true' }} runs-on: ubuntu-latest needs: - linting-done @@ -697,7 +712,8 @@ jobs: # We want to ensure that the cargo benchmarks still compile, which requires a # nightly compiler. cargo-bench: - if: ${{ needs.changes.outputs.rust == 'true' }} + # See `cargo-test` above for why we need to specify `!cancelled() && !failure()`. + if: ${{ !cancelled() && !failure() && needs.changes.outputs.rust == 'true' }} runs-on: ubuntu-latest needs: - linting-done @@ -709,7 +725,7 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@e97e2d8cc328f1b50210efc529dca0028893a2d9 # master with: - toolchain: nightly-2022-12-01 + toolchain: ${{ env.RUST_NIGHTLY_VERSION }} - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 - run: cargo bench --no-run diff --git a/changelog.d/19883.misc b/changelog.d/19883.misc new file mode 100644 index 0000000000..673e8cea1a --- /dev/null +++ b/changelog.d/19883.misc @@ -0,0 +1 @@ +Prevent the `cargo-deny` and `cargo-bench` CI jobs from being skipped, even on PRs that have Rust changes. \ No newline at end of file diff --git a/rust/src/events/formats/v4.rs b/rust/src/events/formats/v4.rs index 22a1a677f2..08e5189275 100644 --- a/rust/src/events/formats/v4.rs +++ b/rust/src/events/formats/v4.rs @@ -37,7 +37,7 @@ use crate::{ json::AllowMissing, }; -/// Version-specific fields for room version 11. +/// Version-specific fields for room version 12+. #[derive(Serialize, Deserialize)] pub struct EventFormatV4 { #[serde( @@ -57,6 +57,8 @@ impl EventFormatV4 { bail!("v4 events must not have an explicit event_id"); } + validate_optional_room_id(self.room_id.as_ref_opt(), common_fields)?; + Ok(()) } diff --git a/rust/src/events/formats/vmsc4242.rs b/rust/src/events/formats/vmsc4242.rs index 45d9a25068..4e13e6f36b 100644 --- a/rust/src/events/formats/vmsc4242.rs +++ b/rust/src/events/formats/vmsc4242.rs @@ -33,7 +33,7 @@ use pyo3::PyResult; use serde::{Deserialize, Serialize}; use crate::events::constants::event_type::M_ROOM_CREATE; -use crate::events::formats::v4::get_room_id_for_optional_room_id; +use crate::events::formats::v4::{get_room_id_for_optional_room_id, validate_optional_room_id}; use crate::events::formats::EventCommonFields; use crate::events::Event; use crate::json::AllowMissing; @@ -62,6 +62,8 @@ impl EventFormatVMSC4242 { bail!("MSC4242 events must not have an explicit event_id"); } + validate_optional_room_id(self.room_id.as_ref_opt(), common_fields)?; + Ok(()) } diff --git a/rust/src/events/utils.rs b/rust/src/events/utils.rs index b58edc9352..668e415d5b 100644 --- a/rust/src/events/utils.rs +++ b/rust/src/events/utils.rs @@ -367,9 +367,14 @@ mod tests { } #[test] - /// Tests to ensure events with overly large values for `depth` are handled appropriately. - /// This was added in room version 6 . - fn test_calculate_event_id_big_int_old_rooms() { + /// This is a bit funky, but we test that relaxed `depth` rules are in + /// place, even in room versions that enforce strict canonical JSON, as we + /// still need to load invalid events from the database even in newer room + /// versions. See https://github.com/element-hq/synapse/pull/19816. + /// + /// We still enforce canonicaljson when creating *new* events (see + /// `EventValidator` on the python side). + fn test_calculate_event_id_big_int_is_relaxed() { let original = json!( { "auth_events":[ @@ -407,12 +412,10 @@ mod tests { ); // These should succeed. - let _event_id = calculate_event_id(&original, &RoomVersion::V3).unwrap(); - let _event_id = calculate_event_id(&original, &RoomVersion::V4).unwrap(); - let _event_id = calculate_event_id(&original, &RoomVersion::V5).unwrap(); - - // These should not succeed. let versions = [ + RoomVersion::V3, + RoomVersion::V4, + RoomVersion::V5, RoomVersion::V6, RoomVersion::V7, RoomVersion::V8, @@ -422,7 +425,7 @@ mod tests { RoomVersion::V12, ]; for version in versions { - let _event_id = calculate_event_id(&original, &version).unwrap_err(); + let _event_id = calculate_event_id(&original, &version).unwrap(); } } diff --git a/rust/src/json.rs b/rust/src/json.rs index 3e833c6707..2a788e5e61 100644 --- a/rust/src/json.rs +++ b/rust/src/json.rs @@ -105,8 +105,6 @@ pub mod allow_missing { #[cfg(test)] mod tests { - use std::assert_matches; - use serde::{Deserialize, Serialize}; use super::*; @@ -126,12 +124,12 @@ mod tests { let json = r#"{"value":42}"#; let deserialized: TestStruct = serde_json::from_str(json).unwrap(); assert!(deserialized.value.is_some()); - assert_matches!(deserialized.value, AllowMissing::Some(42)); + assert!(matches!(deserialized.value, AllowMissing::Some(42))); let json = r#"{}"#; let deserialized: TestStruct = serde_json::from_str(json).unwrap(); assert!(deserialized.value.is_absent()); - assert_matches!(deserialized.value, AllowMissing::Absent); + assert!(matches!(deserialized.value, AllowMissing::Absent)); } #[test] @@ -203,16 +201,16 @@ mod tests { let json = r#"{"value":42}"#; let deserialized: TestStructOption = serde_json::from_str(json).unwrap(); assert!(deserialized.value.is_some()); - assert_matches!(deserialized.value, AllowMissing::Some(Some(42))); + assert!(matches!(deserialized.value, AllowMissing::Some(Some(42)))); let json = r#"{"value":null}"#; let deserialized: TestStructOption = serde_json::from_str(json).unwrap(); assert!(deserialized.value.is_some()); - assert_matches!(deserialized.value, AllowMissing::Some(None)); + assert!(matches!(deserialized.value, AllowMissing::Some(None))); let json = r#"{}"#; let deserialized: TestStructOption = serde_json::from_str(json).unwrap(); assert!(deserialized.value.is_absent()); - assert_matches!(deserialized.value, AllowMissing::Absent); + assert!(matches!(deserialized.value, AllowMissing::Absent)); } } diff --git a/tests/events/test_validator.py b/tests/events/test_validator.py index 082ae04a4c..2b1ee931ba 100644 --- a/tests/events/test_validator.py +++ b/tests/events/test_validator.py @@ -11,7 +11,12 @@ # See the GNU Affero General Public License for more details: # . # -from synapse.api.room_versions import RoomVersions +from synapse.api.errors import Codes, SynapseError +from synapse.api.room_versions import ( + KNOWN_ROOM_VERSIONS, + EventFormatVersions, + RoomVersions, +) from synapse.events import make_event_from_dict from synapse.events.validator import EventValidator @@ -45,3 +50,46 @@ class EventValidatorTestCase(HomeserverTestCase): ) EventValidator().validate_new(event, self.hs.config) + + def test_validate_new_rejects_big_depth_for_strict_canonicaljson_rooms( + self, + ) -> None: + """ + Test that `EventValidator.validate_new` rejects events with integers outside the + canonical JSON range, in room versions which enforce it (v6+). + """ + for room_version in KNOWN_ROOM_VERSIONS.values(): + with self.subTest(room_version=room_version.identifier): + event_dict = { + "room_id": "!room:test", + "type": "m.room.message", + "sender": "@alice:example.com", + "content": { + "msgtype": "m.text", + "body": "hello", + }, + "auth_events": [], + "prev_events": [], + "hashes": {"sha256": "aGVsbG8="}, + "signatures": {}, + "depth": 2**53, + "origin_server_ts": 1000, + } + + if room_version.event_format == EventFormatVersions.ROOM_V1_V2: + event_dict["event_id"] = "$event:test" + + event = make_event_from_dict( + event_dict, + room_version=room_version, + ) + + # Check if this room version enforces strict canonical json. + if room_version.strict_canonicaljson: + with self.assertRaises(SynapseError) as cm: + EventValidator().validate_new(event, self.hs.config) + + self.assertEqual(cm.exception.errcode, Codes.BAD_JSON) + self.assertEqual(cm.exception.msg, "JSON integer out of range") + else: + EventValidator().validate_new(event, self.hs.config)