From 09d83f312713c8d2acc0b83f27be2e2b607a2015 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 7 Apr 2026 14:12:01 +0200 Subject: [PATCH] Fix `KNOWN_ROOM_VERSIONS.__contains__` raising TypeError for non-string keys (#19649) The Rust port of `KNOWN_ROOM_VERSIONS` (#19589) made `__contains__` strict about key types, raising `TypeError` when called with `None` instead of returning `False` like a Python dict would. This broke `/sync` for rooms with a NULL `room_version` in the database. ``` File "/home/synapse/src/synapse/handlers/sync.py", line 2628, in _get_room_changes_for_initial_sync if event.room_version_id not in KNOWN_ROOM_VERSIONS: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: argument 'key': 'NoneType' object cannot be cast as 'str' ``` --- changelog.d/19649.bugfix | 1 + rust/src/room_versions.rs | 16 +++++++-- tests/synapse_rust/test_room_versions.py | 45 ++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 changelog.d/19649.bugfix create mode 100644 tests/synapse_rust/test_room_versions.py diff --git a/changelog.d/19649.bugfix b/changelog.d/19649.bugfix new file mode 100644 index 0000000000..948dc4feac --- /dev/null +++ b/changelog.d/19649.bugfix @@ -0,0 +1 @@ +Fix `KNOWN_ROOM_VERSIONS.__contains__` raising `TypeError` for non-string keys, which could cause `/sync` to fail for rooms with a NULL room version in the database. Bug introduced in [#19589](https://github.com/element-hq/synapse/pull/19589). diff --git a/rust/src/room_versions.rs b/rust/src/room_versions.rs index c0b304e18c..fbcc32516a 100644 --- a/rust/src/room_versions.rs +++ b/rust/src/room_versions.rs @@ -600,7 +600,12 @@ impl KnownRoomVersionsMapping { Ok(()) } - fn __getitem__(&self, key: &str) -> PyResult { + fn __getitem__(&self, key: &Bound<'_, PyAny>) -> PyResult { + // We need to accept anything as the key, but we know that only strings + // are valid keys, so if it's not a string we just raise a KeyError. + let Ok(key) = key.extract::<&str>() else { + return Err(PyKeyError::new_err(key.to_string())); + }; let versions = self.versions.read().unwrap(); versions .iter() @@ -609,9 +614,14 @@ impl KnownRoomVersionsMapping { .ok_or_else(|| PyKeyError::new_err(key.to_string())) } - fn __contains__(&self, key: &str) -> PyResult { + fn __contains__(&self, key: &Bound<'_, PyAny>) -> bool { + // We need to accept anything as the key, but we know that only strings + // are valid keys, so if it's not a string we just return false. + let Ok(key) = key.extract::<&str>() else { + return false; + }; let versions = self.versions.read().unwrap(); - Ok(versions.iter().any(|v| v.identifier == key)) + versions.iter().any(|v| v.identifier == key) } fn keys(&self) -> PyResult> { diff --git a/tests/synapse_rust/test_room_versions.py b/tests/synapse_rust/test_room_versions.py new file mode 100644 index 0000000000..aff1ab2783 --- /dev/null +++ b/tests/synapse_rust/test_room_versions.py @@ -0,0 +1,45 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 Element Creations Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . + +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS + +from tests import unittest + + +class KnownRoomVersionsMappingTestCase(unittest.TestCase): + """Tests for the Rust-backed KNOWN_ROOM_VERSIONS mapping.""" + + def test_contains_none(self) -> None: + """Test that `None in KNOWN_ROOM_VERSIONS` returns False + rather than raising a TypeError, matching Python dict semantics.""" + self.assertFalse(None in KNOWN_ROOM_VERSIONS) + + def test_contains_non_string(self) -> None: + """Test that non-string keys return False from __contains__.""" + self.assertFalse(42 in KNOWN_ROOM_VERSIONS) # type: ignore[comparison-overlap] + self.assertFalse(3.14 in KNOWN_ROOM_VERSIONS) # type: ignore[comparison-overlap] + self.assertFalse([] in KNOWN_ROOM_VERSIONS) # type: ignore[comparison-overlap] + + def test_contains_known_version(self) -> None: + self.assertTrue("1" in KNOWN_ROOM_VERSIONS) + + def test_contains_unknown_version(self) -> None: + self.assertFalse("unknown" in KNOWN_ROOM_VERSIONS) + + def test_getitem_non_string_raises_key_error(self) -> None: + """Test that KNOWN_ROOM_VERSIONS[non_string] raises KeyError, + not TypeError, matching Python dict semantics.""" + with self.assertRaises(KeyError): + KNOWN_ROOM_VERSIONS[42] # type: ignore[index] + with self.assertRaises(KeyError): + KNOWN_ROOM_VERSIONS[None] # type: ignore[index]