From 7555e20158df4fa4a724be26eeb5d68ed2ef4508 Mon Sep 17 00:00:00 2001 From: drebs Date: Fri, 29 Sep 2017 18:38:48 -0300 Subject: [bug] check all http response status codes --- src/leap/soledad/client/_db/blobs.py | 55 ++++++++++++++++++------------------ src/leap/soledad/server/_blobs.py | 18 ++++++++++-- tests/blobs/test_fs_backend.py | 31 +++++++++++++++++--- tests/server/test_blobs_server.py | 18 ++++++++++++ 4 files changed, 87 insertions(+), 35 deletions(-) diff --git a/src/leap/soledad/client/_db/blobs.py b/src/leap/soledad/client/_db/blobs.py index 6d47999a..e997c6ff 100644 --- a/src/leap/soledad/client/_db/blobs.py +++ b/src/leap/soledad/client/_db/blobs.py @@ -132,11 +132,13 @@ class ConnectionPool(adbapi.ConnectionPool): return handle -def check_http_status(code): +def check_http_status(code, blob_id=None, flags=None): + if code == 404: + raise BlobNotFoundError(blob_id) if code == 409: - raise BlobAlreadyExistsError() + raise BlobAlreadyExistsError(blob_id) elif code == 406: - raise InvalidFlagsError() + raise InvalidFlagsError((blob_id, flags)) elif code != 200: raise SoledadError("Server Error: %s" % code) @@ -253,8 +255,9 @@ class BlobManager(object): :rtype: twisted.internet.defer.Deferred """ uri = urljoin(self.remote, self.user + '/') - data = yield self._client.get(uri, params=params) - defer.returnValue((yield data.json())) + response = yield self._client.get(uri, params=params) + check_http_status(response.code) + defer.returnValue((yield response.json())) def local_list(self, namespace='', sync_status=None): return self.local.list(namespace, sync_status) @@ -350,15 +353,10 @@ class BlobManager(object): :return: A deferred that fires when the operation finishes. :rtype: twisted.internet.defer.Deferred """ - flags = BytesIO(json.dumps(flags)) + flagsfd = BytesIO(json.dumps(flags)) uri = urljoin(self.remote, self.user + "/" + blob_id) - response = yield self._client.post(uri, data=flags, params=params) - if response.code == 404: - logger.error("Blob not found during set_flags: %s" % blob_id) - msg = "No blob found on server while trying to set flags: %s" - raise SoledadError(msg % (blob_id)) - - check_http_status(response.code) + response = yield self._client.post(uri, data=flagsfd, params=params) + check_http_status(response.code, blob_id=blob_id, flags=flags) @defer.inlineCallbacks def get_flags(self, blob_id, **params): @@ -378,9 +376,7 @@ class BlobManager(object): uri = urljoin(self.remote, self.user + "/" + blob_id) params.update({'only_flags': True}) response = yield self._client.get(uri, params=params) - if response.code == 404: - logger.error("Blob not found in server: %r" % blob_id) - raise BlobNotFoundError(blob_id) + check_http_status(response.code, blob_id=blob_id) defer.returnValue((yield response.json())) @defer.inlineCallbacks @@ -435,7 +431,7 @@ class BlobManager(object): armor=False) fd = yield crypter.encrypt() response = yield self._client.put(uri, data=fd, params=params) - check_http_status(response.code) + check_http_status(response.code, blob_id) logger.info("Finished upload: %s" % (blob_id,)) @defer.inlineCallbacks @@ -444,20 +440,20 @@ class BlobManager(object): # TODO this needs to be connected in a tube uri = urljoin(self.remote, self.user + '/' + blob_id) params = {'namespace': namespace} if namespace else None - data = yield self._client.get(uri, params=params, namespace=namespace) - - if data.code == 404: - logger.warn("Blob not found in server: %s" % blob_id) - defer.returnValue(None) - elif not data.headers.hasHeader('Tag'): - logger.error("Server didn't send a tag header for: %s" % blob_id) - defer.returnValue(None) - tag = data.headers.getRawHeaders('Tag')[0] + response = yield self._client.get(uri, params=params, + namespace=namespace) + check_http_status(response.code, blob_id=blob_id) + + if not response.headers.hasHeader('Tag'): + msg = "Server didn't send a tag header for: %s" % blob_id + logger.error(msg) + raise SoledadError(msg) + tag = response.headers.getRawHeaders('Tag')[0] tag = base64.urlsafe_b64decode(tag) buf = DecrypterBuffer(blob_id, self.secret, tag) # incrementally collect the body of the response - yield treq.collect(data, buf.write) + yield treq.collect(response, buf.write) fd, size = buf.close() logger.info("Finished download: (%s, %d)" % (blob_id, size)) defer.returnValue((fd, size)) @@ -482,10 +478,13 @@ class BlobManager(object): if (yield self.local.exists(blob_id, namespace)): yield self.local.delete(blob_id, namespace) + @defer.inlineCallbacks def _delete_from_remote(self, blob_id, **params): # TODO this needs to be connected in a tube uri = urljoin(self.remote, self.user + '/' + blob_id) - return self._client.delete(uri, params=params) + response = yield self._client.delete(uri, params=params) + check_http_status(response.code, blob_id=blob_id) + defer.returnValue(response) class SQLiteBlobBackend(object): diff --git a/src/leap/soledad/server/_blobs.py b/src/leap/soledad/server/_blobs.py index 790a9b51..cacfe38d 100644 --- a/src/leap/soledad/server/_blobs.py +++ b/src/leap/soledad/server/_blobs.py @@ -124,8 +124,11 @@ class FilesystemBlobsBackend(object): with open(path, 'wb') as blobfile: yield fbp.startProducing(blobfile) - def delete_blob(self, user, blob_id, namespace=''): + def delete_blob(self, user, blob_id, request, namespace=''): blob_path = self._get_path(user, blob_id, namespace) + if not os.path.isfile(blob_path): + request.setResponseCode(404) + return "Blob doesn't exists: %s" % blob_id os.unlink(blob_path) try: os.unlink(blob_path + '.flags') @@ -175,7 +178,12 @@ class FilesystemBlobsBackend(object): return self._get_disk_usage(self._get_path(user)) def add_tag_header(self, user, blob_id, request, namespace=''): - with open(self._get_path(user, blob_id, namespace)) as doc_file: + blob_path = self._get_path(user, blob_id, namespace) + if not os.path.isfile(blob_path): + # 404 - Not Found + request.setResponseCode(404) + return "Blob doesn't exists: %s" % blob_id + with open(blob_path) as doc_file: doc_file.seek(-16, 2) tag = base64.urlsafe_b64encode(doc_file.read()) request.responseHeaders.setRawHeaders('Tag', [tag]) @@ -202,6 +210,10 @@ class FilesystemBlobsBackend(object): raise Exception(err) return desired_path + def exists(self, user, blob_id, namespace): + return os.path.isfile( + self._get_path(user, blob_id=blob_id, namespace=namespace)) + def _get_path(self, user, blob_id='', namespace=''): parts = [user] if blob_id: @@ -262,7 +274,7 @@ class BlobsResource(resource.Resource): def render_DELETE(self, request): logger.info("http put: %s" % request.path) user, blob_id, namespace = self._validate(request) - self._handler.delete_blob(user, blob_id, namespace) + self._handler.delete_blob(user, blob_id, request, namespace=namespace) return '' def render_PUT(self, request): diff --git a/tests/blobs/test_fs_backend.py b/tests/blobs/test_fs_backend.py index 53f3127d..07443b8a 100644 --- a/tests/blobs/test_fs_backend.py +++ b/tests/blobs/test_fs_backend.py @@ -20,6 +20,7 @@ Tests for blobs backend on server side. from twisted.trial import unittest from twisted.internet import defer from twisted.web.test.test_web import DummyRequest +from leap.common.files import mkdir_p from leap.soledad.server import _blobs from io import BytesIO from mock import Mock @@ -38,18 +39,25 @@ class FilesystemBackendTestCase(unittest.TestCase): expected_tag = base64.urlsafe_b64encode('B' * 16) expected_method = Mock() backend = _blobs.FilesystemBlobsBackend() + # write a blob... + path = backend._get_path('user', 'blob_id', '') + mkdir_p(os.path.split(path)[0]) + with open(path, "w") as f: + f.write("bl0b") + # ...and get its tag request = Mock(responseHeaders=Mock(setRawHeaders=expected_method)) backend.add_tag_header('user', 'blob_id', request) expected_method.assert_called_once_with('Tag', [expected_tag]) @mock.patch.object(_blobs.static, 'File') + @mock.patch.object(_blobs.FilesystemBlobsBackend, '_get_path', + Mock(return_value='path')) def test_read_blob(self, file_mock): render_mock = Mock() file_mock.return_value = render_mock backend = _blobs.FilesystemBlobsBackend() request = DummyRequest(['']) - backend._get_path = Mock(return_value='path') backend.read_blob('user', 'blob_id', request) backend._get_path.assert_called_once_with('user', 'blob_id', '') @@ -58,11 +66,12 @@ class FilesystemBackendTestCase(unittest.TestCase): render_mock.render_GET.assert_called_once_with(request) @mock.patch.object(os.path, 'isfile') + @mock.patch.object(_blobs.FilesystemBlobsBackend, '_get_path', + Mock(return_value='path')) @defer.inlineCallbacks def test_cannot_overwrite(self, isfile): isfile.return_value = True backend = _blobs.FilesystemBlobsBackend() - backend._get_path = Mock(return_value='path') request = DummyRequest(['']) yield backend.write_blob('user', 'blob_id', request) self.assertEquals(request.written[0], "Blob already exists: blob_id") @@ -154,7 +163,14 @@ class FilesystemBackendTestCase(unittest.TestCase): @mock.patch('leap.soledad.server._blobs.os.unlink') def test_delete_blob(self, unlink_mock): backend = _blobs.FilesystemBlobsBackend(self.tempdir) - backend.delete_blob('user', 'blob_id') + request = DummyRequest(['']) + # write a blob... + path = backend._get_path('user', 'blob_id', '') + mkdir_p(os.path.split(path)[0]) + with open(path, "w") as f: + f.write("bl0b") + # ...and delete it + backend.delete_blob('user', 'blob_id', request) unlink_mock.assert_any_call(backend._get_path('user', 'blob_id')) unlink_mock.assert_any_call(backend._get_path('user', @@ -164,7 +180,14 @@ class FilesystemBackendTestCase(unittest.TestCase): @mock.patch('leap.soledad.server._blobs.os.unlink') def test_delete_blob_custom_namespace(self, unlink_mock): backend = _blobs.FilesystemBlobsBackend(self.tempdir) - backend.delete_blob('user', 'blob_id', namespace='trash') + request = DummyRequest(['']) + # write a blob... + path = backend._get_path('user', 'blob_id', 'trash') + mkdir_p(os.path.split(path)[0]) + with open(path, "w") as f: + f.write("bl0b") + # ...and delete it + backend.delete_blob('user', 'blob_id', request, namespace='trash') unlink_mock.assert_any_call(backend._get_path('user', 'blob_id', 'trash')) diff --git a/tests/server/test_blobs_server.py b/tests/server/test_blobs_server.py index 2ee6fda2..c628ae78 100644 --- a/tests/server/test_blobs_server.py +++ b/tests/server/test_blobs_server.py @@ -292,3 +292,21 @@ class BlobServerTestCase(unittest.TestCase): yield manager._delete_from_remote('blob_id1', namespace=namespace) blobs_list = yield manager.remote_list(namespace=namespace) self.assertEquals(set(['blob_id2']), set(blobs_list)) + + @defer.inlineCallbacks + @pytest.mark.usefixtures("method_tmpdir") + def test_get_fails_if_no_blob_found(self): + manager = BlobManager(self.tempdir, self.uri, self.secret, + self.secret, uuid4().hex) + self.addCleanup(manager.close) + with pytest.raises(SoledadError): + yield manager.get('missing_id') + + @defer.inlineCallbacks + @pytest.mark.usefixtures("method_tmpdir") + def test_delete_fails_if_no_blob_found(self): + manager = BlobManager(self.tempdir, self.uri, self.secret, + self.secret, uuid4().hex) + self.addCleanup(manager.close) + with pytest.raises(SoledadError): + yield manager.delete('missing_id') -- cgit v1.2.3