handle empty backups according to latest spec proposal (#4123)

fixes #4056
This commit is contained in:
Hubert Chathi 2018-11-05 17:59:29 -05:00 committed by GitHub
parent efdcbbe46b
commit f1087106cf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 71 additions and 52 deletions

1
changelog.d/4123.bugfix Normal file
View file

@ -0,0 +1 @@
fix return code of empty key backups

View file

@ -19,7 +19,7 @@ from six import iteritems
from twisted.internet import defer
from synapse.api.errors import RoomKeysVersionError, StoreError, SynapseError
from synapse.api.errors import NotFoundError, RoomKeysVersionError, StoreError
from synapse.util.async_helpers import Linearizer
logger = logging.getLogger(__name__)
@ -55,6 +55,8 @@ class E2eRoomKeysHandler(object):
room_id(string): room ID to get keys for, for None to get keys for all rooms
session_id(string): session ID to get keys for, for None to get keys for all
sessions
Raises:
NotFoundError: if the backup version does not exist
Returns:
A deferred list of dicts giving the session_data and message metadata for
these room keys.
@ -63,13 +65,19 @@ class E2eRoomKeysHandler(object):
# we deliberately take the lock to get keys so that changing the version
# works atomically
with (yield self._upload_linearizer.queue(user_id)):
# make sure the backup version exists
try:
yield self.store.get_e2e_room_keys_version_info(user_id, version)
except StoreError as e:
if e.code == 404:
raise NotFoundError("Unknown backup version")
else:
raise
results = yield self.store.get_e2e_room_keys(
user_id, version, room_id, session_id
)
if results['rooms'] == {}:
raise SynapseError(404, "No room_keys found")
defer.returnValue(results)
@defer.inlineCallbacks
@ -120,7 +128,7 @@ class E2eRoomKeysHandler(object):
}
Raises:
SynapseError: with code 404 if there are no versions defined
NotFoundError: if there are no versions defined
RoomKeysVersionError: if the uploaded version is not the current version
"""
@ -134,7 +142,7 @@ class E2eRoomKeysHandler(object):
version_info = yield self.store.get_e2e_room_keys_version_info(user_id)
except StoreError as e:
if e.code == 404:
raise SynapseError(404, "Version '%s' not found" % (version,))
raise NotFoundError("Version '%s' not found" % (version,))
else:
raise
@ -148,7 +156,7 @@ class E2eRoomKeysHandler(object):
raise RoomKeysVersionError(current_version=version_info['version'])
except StoreError as e:
if e.code == 404:
raise SynapseError(404, "Version '%s' not found" % (version,))
raise NotFoundError("Version '%s' not found" % (version,))
else:
raise

View file

@ -17,7 +17,7 @@ import logging
from twisted.internet import defer
from synapse.api.errors import Codes, SynapseError
from synapse.api.errors import Codes, NotFoundError, SynapseError
from synapse.http.servlet import (
RestServlet,
parse_json_object_from_request,
@ -208,9 +208,24 @@ class RoomKeysServlet(RestServlet):
user_id, version, room_id, session_id
)
# Convert room_keys to the right format to return.
if session_id:
# If the client requests a specific session, but that session was
# not backed up, then return an M_NOT_FOUND.
if room_keys['rooms'] == {}:
raise NotFoundError("No room_keys found")
else:
room_keys = room_keys['rooms'][room_id]['sessions'][session_id]
elif room_id:
# If the client requests all sessions from a room, but no sessions
# are found, then return an empty result rather than an error, so
# that clients don't have to handle an error condition, and an
# empty result is valid. (Similarly if the client requests all
# sessions from the backup, but in that case, room_keys is already
# in the right format, so we don't need to do anything about it.)
if room_keys['rooms'] == {}:
room_keys = {'sessions': {}}
else:
room_keys = room_keys['rooms'][room_id]
defer.returnValue((200, room_keys))

View file

@ -169,8 +169,8 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase):
self.assertEqual(res, 404)
@defer.inlineCallbacks
def test_get_missing_room_keys(self):
"""Check that we get a 404 on querying missing room_keys
def test_get_missing_backup(self):
"""Check that we get a 404 on querying missing backup
"""
res = None
try:
@ -179,19 +179,20 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase):
res = e.code
self.assertEqual(res, 404)
# check we also get a 404 even if the version is valid
@defer.inlineCallbacks
def test_get_missing_room_keys(self):
"""Check we get an empty response from an empty backup
"""
version = yield self.handler.create_version(self.local_user, {
"algorithm": "m.megolm_backup.v1",
"auth_data": "first_version_auth_data",
})
self.assertEqual(version, "1")
res = None
try:
yield self.handler.get_room_keys(self.local_user, version)
except errors.SynapseError as e:
res = e.code
self.assertEqual(res, 404)
res = yield self.handler.get_room_keys(self.local_user, version)
self.assertDictEqual(res, {
"rooms": {}
})
# TODO: test the locking semantics when uploading room_keys,
# although this is probably best done in sytest
@ -345,17 +346,15 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase):
# check for bulk-delete
yield self.handler.upload_room_keys(self.local_user, version, room_keys)
yield self.handler.delete_room_keys(self.local_user, version)
res = None
try:
yield self.handler.get_room_keys(
res = yield self.handler.get_room_keys(
self.local_user,
version,
room_id="!abc:matrix.org",
session_id="c0ff33",
)
except errors.SynapseError as e:
res = e.code
self.assertEqual(res, 404)
self.assertDictEqual(res, {
"rooms": {}
})
# check for bulk-delete per room
yield self.handler.upload_room_keys(self.local_user, version, room_keys)
@ -364,17 +363,15 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase):
version,
room_id="!abc:matrix.org",
)
res = None
try:
yield self.handler.get_room_keys(
res = yield self.handler.get_room_keys(
self.local_user,
version,
room_id="!abc:matrix.org",
session_id="c0ff33",
)
except errors.SynapseError as e:
res = e.code
self.assertEqual(res, 404)
self.assertDictEqual(res, {
"rooms": {}
})
# check for bulk-delete per session
yield self.handler.upload_room_keys(self.local_user, version, room_keys)
@ -384,14 +381,12 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase):
room_id="!abc:matrix.org",
session_id="c0ff33",
)
res = None
try:
yield self.handler.get_room_keys(
res = yield self.handler.get_room_keys(
self.local_user,
version,
room_id="!abc:matrix.org",
session_id="c0ff33",
)
except errors.SynapseError as e:
res = e.code
self.assertEqual(res, 404)
self.assertDictEqual(res, {
"rooms": {}
})