mirror of
https://github.com/element-hq/synapse.git
synced 2026-04-25 17:32:16 +00:00
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' ```
This commit is contained in:
1
changelog.d/19649.bugfix
Normal file
1
changelog.d/19649.bugfix
Normal file
@@ -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).
|
||||
@@ -600,7 +600,12 @@ impl KnownRoomVersionsMapping {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn __getitem__(&self, key: &str) -> PyResult<RoomVersion> {
|
||||
fn __getitem__(&self, key: &Bound<'_, PyAny>) -> PyResult<RoomVersion> {
|
||||
// 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<bool> {
|
||||
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<Vec<&'static str>> {
|
||||
|
||||
45
tests/synapse_rust/test_room_versions.py
Normal file
45
tests/synapse_rust/test_room_versions.py
Normal file
@@ -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:
|
||||
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
||||
|
||||
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]
|
||||
Reference in New Issue
Block a user