From 563f9b61b1439e7a6a8a250fd068659092583dbf Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 29 Oct 2018 21:01:22 +0000 Subject: [PATCH 1/9] Make e2e backup versions numeric in the DB We were doing max(version) which does not do what we wanted on a column of type TEXT. --- synapse/handlers/e2e_room_keys.py | 2 +- synapse/storage/prepare_database.py | 2 +- .../storage/schema/delta/52/e2e_room_keys.sql | 53 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 synapse/storage/schema/delta/52/e2e_room_keys.sql diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 5edb3cfe0445..b6d302cd6b4d 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -138,7 +138,7 @@ def upload_room_keys(self, user_id, version, room_keys): else: raise - if version_info['version'] != version: + if str(version_info['version']) != version: # Check that the version we're trying to upload actually exists try: version_info = yield self.store.get_e2e_room_keys_version_info( diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index b364719312d6..bd740e1e4553 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -25,7 +25,7 @@ # Remember to update this number every time a change is made to database # schema files, so the users will be informed on server restarts. -SCHEMA_VERSION = 51 +SCHEMA_VERSION = 52 dir_path = os.path.abspath(os.path.dirname(__file__)) diff --git a/synapse/storage/schema/delta/52/e2e_room_keys.sql b/synapse/storage/schema/delta/52/e2e_room_keys.sql new file mode 100644 index 000000000000..754985187e1d --- /dev/null +++ b/synapse/storage/schema/delta/52/e2e_room_keys.sql @@ -0,0 +1,53 @@ +/* Copyright 2018 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* Change version column to an integer so we can do MAX() sensibly + */ +CREATE TABLE e2e_room_keys_versions_new ( + user_id TEXT NOT NULL, + version BIGINT NOT NULL, + algorithm TEXT NOT NULL, + auth_data TEXT NOT NULL, + deleted SMALLINT DEFAULT 0 NOT NULL +); + +INSERT INTO e2e_room_keys_versions_new + SELECT user_id, version, algorithm, auth_data, deleted FROM e2e_room_keys_versions; + +DROP TABLE e2e_room_keys_versions; +ALTER TABLE e2e_room_keys_versions_new RENAME TO e2e_room_keys_versions; + +CREATE UNIQUE INDEX e2e_room_keys_versions_idx ON e2e_room_keys_versions(user_id, version); + +/* Change e2e_rooms_keys to match + */ +CREATE TABLE e2e_room_keys_new ( + user_id TEXT NOT NULL, + room_id TEXT NOT NULL, + session_id TEXT NOT NULL, + version BIGINT NOT NULL, + first_message_index INT, + forwarded_count INT, + is_verified BOOLEAN, + session_data TEXT NOT NULL +); + +INSERT INTO e2e_room_keys_new + SELECT user_id, room_id, session_id, version, first_message_index, forwarded_count, is_verified, session_data FROM e2e_room_keys; + +DROP TABLE e2e_room_keys; +ALTER TABLE e2e_room_keys_new RENAME TO e2e_room_keys; + +CREATE UNIQUE INDEX e2e_room_keys_idx ON e2e_room_keys(user_id, room_id, session_id); From 64fa557f80edc99318aa6152c3b84c76dd455c8a Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Oct 2018 09:51:04 +0000 Subject: [PATCH 2/9] Try & make it work on postgres --- synapse/storage/schema/delta/52/e2e_room_keys.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/schema/delta/52/e2e_room_keys.sql b/synapse/storage/schema/delta/52/e2e_room_keys.sql index 754985187e1d..db687cccae88 100644 --- a/synapse/storage/schema/delta/52/e2e_room_keys.sql +++ b/synapse/storage/schema/delta/52/e2e_room_keys.sql @@ -24,7 +24,7 @@ CREATE TABLE e2e_room_keys_versions_new ( ); INSERT INTO e2e_room_keys_versions_new - SELECT user_id, version, algorithm, auth_data, deleted FROM e2e_room_keys_versions; + SELECT user_id, CAST(version as BIGINT), algorithm, auth_data, deleted FROM e2e_room_keys_versions; DROP TABLE e2e_room_keys_versions; ALTER TABLE e2e_room_keys_versions_new RENAME TO e2e_room_keys_versions; @@ -45,7 +45,7 @@ CREATE TABLE e2e_room_keys_new ( ); INSERT INTO e2e_room_keys_new - SELECT user_id, room_id, session_id, version, first_message_index, forwarded_count, is_verified, session_data FROM e2e_room_keys; + SELECT user_id, room_id, session_id, CAST(version as BIGINT), first_message_index, forwarded_count, is_verified, session_data FROM e2e_room_keys; DROP TABLE e2e_room_keys; ALTER TABLE e2e_room_keys_new RENAME TO e2e_room_keys; From 4eacf0f2008573ae96c2c693427f3098730691b7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Oct 2018 10:05:51 +0000 Subject: [PATCH 3/9] news fragment --- changelog.d/4113.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4113.bugfix diff --git a/changelog.d/4113.bugfix b/changelog.d/4113.bugfix new file mode 100644 index 000000000000..7f9d8ee0326a --- /dev/null +++ b/changelog.d/4113.bugfix @@ -0,0 +1 @@ +Fix e2e key backup with more than 9 backup versions From 2f0f911c52f6de48e47fbb5624a3e3beee0dd6a4 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Oct 2018 10:35:18 +0000 Subject: [PATCH 4/9] Convert version back to a string --- synapse/storage/e2e_room_keys.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index f25ded229520..9f826b292c04 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -236,6 +236,7 @@ def _get_e2e_room_keys_version_info_txn(txn): ), ) result["auth_data"] = json.loads(result["auth_data"]) + result["version"] = str(result["version"]) return result return self.runInteraction( From 12941f5f8b1f38f273e301104203149b10e9e214 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Oct 2018 11:01:07 +0000 Subject: [PATCH 5/9] Cast bacjup version to int when querying --- synapse/storage/e2e_room_keys.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 9f826b292c04..2a012e9487c6 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -219,7 +219,12 @@ def _get_e2e_room_keys_version_info_txn(txn): if version is None: this_version = self._get_current_version(txn, user_id) else: - this_version = version + try: + this_version = int(version) + except ValueError: + # Our versions are all ints so if we can't convert it to an integer, + # it isn't there. + raise StoreError(404, "No row found") result = self._simple_select_one_txn( txn, From e0934acdbbd497ee1330efe42feccb11382639ec Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Oct 2018 11:12:23 +0000 Subject: [PATCH 6/9] Cast to int here too --- synapse/storage/e2e_room_keys.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 2a012e9487c6..a1d203fd61ae 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -118,6 +118,11 @@ def get_e2e_room_keys( these room keys. """ + try: + version = int(version) + except ValueError: + defer.returnValue({'rooms': {}}) + keyvalues = { "user_id": user_id, "version": version, From d3fa6194f7cce262b39ab27523565ca0e6880263 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 9 Nov 2018 11:11:31 +0000 Subject: [PATCH 7/9] Remove unnecessary str() --- synapse/handlers/e2e_room_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index b6d302cd6b4d..5edb3cfe0445 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -138,7 +138,7 @@ def upload_room_keys(self, user_id, version, room_keys): else: raise - if str(version_info['version']) != version: + if version_info['version'] != version: # Check that the version we're trying to upload actually exists try: version_info = yield self.store.get_e2e_room_keys_version_info( From 4f93abd62d6800c00c9e943f75e19e09c98c8f30 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 9 Nov 2018 13:25:38 +0000 Subject: [PATCH 8/9] add docs --- synapse/storage/e2e_room_keys.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index a1d203fd61ae..5d83707fa09b 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -217,7 +217,10 @@ def get_e2e_room_keys_version_info(self, user_id, version=None): Raises: StoreError: with code 404 if there are no e2e_room_keys_versions present Returns: - A deferred dict giving the info metadata for this backup version + A deferred dict giving the info metadata for this backup version, with fields including: + version(str) + algorithm(str) + auth_data(object): opaque dict supplied by the client """ def _get_e2e_room_keys_version_info_txn(txn): From d44dea0223c16f86a57647e670b8a7651b93aa2a Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 9 Nov 2018 14:38:31 +0000 Subject: [PATCH 9/9] pep8 --- synapse/storage/e2e_room_keys.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 5d83707fa09b..16b7f005aa9a 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -217,7 +217,8 @@ def get_e2e_room_keys_version_info(self, user_id, version=None): Raises: StoreError: with code 404 if there are no e2e_room_keys_versions present Returns: - A deferred dict giving the info metadata for this backup version, with fields including: + A deferred dict giving the info metadata for this backup version, with + fields including: version(str) algorithm(str) auth_data(object): opaque dict supplied by the client