From b0857d021d64c7eff0eaef2dad48e4bac16f9804 Mon Sep 17 00:00:00 2001 From: sklump Date: Fri, 13 Mar 2020 19:02:41 +0000 Subject: [PATCH 1/2] use requests for now to fetch tails file reliably Signed-off-by: sklump --- .../revocation/models/revocation_registry.py | 23 +++--- .../tests/test_issuer_revocation_record.py | 10 ++- .../models/tests/test_revocation_registry.py | 82 +++++++++++++++---- requirements.txt | 1 + 4 files changed, 86 insertions(+), 30 deletions(-) diff --git a/aries_cloudagent/revocation/models/revocation_registry.py b/aries_cloudagent/revocation/models/revocation_registry.py index 4a85bcfcfd..ebf315d3bd 100644 --- a/aries_cloudagent/revocation/models/revocation_registry.py +++ b/aries_cloudagent/revocation/models/revocation_registry.py @@ -6,8 +6,10 @@ from pathlib import Path from tempfile import gettempdir +from requests import Session +from requests.exceptions import RequestException + from ...config.injection_context import InjectionContext -from ...utils.http import FetchError, fetch_stream from ...utils.temp import get_temp_dir from ..error import RevocationError @@ -163,11 +165,6 @@ async def retrieve_tails(self, context: InjectionContext): self.registry_id, ) - try: - tails_stream = await fetch_stream(self._tails_public_uri) - except FetchError as e: - raise RevocationError("Error retrieving tails file") from e - tails_file_path = Path(self.get_receiving_tails_local_path(context)) tails_file_dir = tails_file_path.parent if not tails_file_dir.exists(): @@ -176,11 +173,15 @@ async def retrieve_tails(self, context: InjectionContext): buffer_size = 65536 # should be multiple of 32 bytes for sha256 with open(tails_file_path, "wb", buffer_size) as tails_file: file_hasher = hashlib.sha256() - buf = await tails_stream.read(buffer_size) - while len(buf) > 0: - file_hasher.update(buf) - tails_file.write(buf) - buf = await tails_stream.read(buffer_size) + + with Session() as req_session: + try: + resp = req_session.get(self._tails_public_uri, stream=True) + for buf in resp.iter_content(chunk_size=buffer_size): + tails_file.write(buf) + file_hasher.update(buf) + except RequestException as rx: + raise RevocationError(f"Error retrieving tails file: {rx}") download_tails_hash = base58.b58encode(file_hasher.digest()).decode("utf-8") if download_tails_hash != self.tails_hash: diff --git a/aries_cloudagent/revocation/models/tests/test_issuer_revocation_record.py b/aries_cloudagent/revocation/models/tests/test_issuer_revocation_record.py index 1d0d186e78..e400839978 100644 --- a/aries_cloudagent/revocation/models/tests/test_issuer_revocation_record.py +++ b/aries_cloudagent/revocation/models/tests/test_issuer_revocation_record.py @@ -8,7 +8,7 @@ from asynctest import TestCase as AsyncTestCase, mock as async_mock from ....config.injection_context import InjectionContext -from ....issuer.base import BaseIssuer +from ....issuer.base import BaseIssuer, IssuerError from ....issuer.indy import IndyIssuer from ....ledger.base import BaseLedger from ....storage.base import BaseStorage @@ -50,6 +50,14 @@ async def test_generate_registry_etc(self): issuer = async_mock.MagicMock(BaseIssuer) self.context.injector.bind_instance(BaseIssuer, issuer) + with async_mock.patch.object( + issuer, "create_and_store_revocation_registry", async_mock.CoroutineMock() + ) as mock_create_store_rr: + mock_create_store_rr.side_effect = IssuerError("Not this time") + + with self.assertRaises(RevocationError): + await rec.generate_registry(self.context, None) + issuer.create_and_store_revocation_registry.return_value = ( REV_REG_ID, json.dumps( diff --git a/aries_cloudagent/revocation/models/tests/test_revocation_registry.py b/aries_cloudagent/revocation/models/tests/test_revocation_registry.py index a0fdb035df..70ad0329e1 100644 --- a/aries_cloudagent/revocation/models/tests/test_revocation_registry.py +++ b/aries_cloudagent/revocation/models/tests/test_revocation_registry.py @@ -90,13 +90,29 @@ async def test_properties(self): assert rev_reg.tails_local_path == "dummy" rev_reg.tails_public_uri = "dummy" assert rev_reg.tails_public_uri == "dummy" + return rev_reg.reg_def == REV_REG_DEF async def test_tails_local_path(self): rr_def_public = deepcopy(REV_REG_DEF) rr_def_public["value"]["tailsLocation"] = "http://sample.ca:8088/path" - rev_reg = RevocationRegistry.from_definition(rr_def_public, public_def=True) + rev_reg_pub = RevocationRegistry.from_definition(rr_def_public, public_def=True) + + assert rev_reg_pub.get_receiving_tails_local_path(self.context) == TAILS_LOCAL + + rev_reg_loc = RevocationRegistry.from_definition(REV_REG_DEF, public_def=False) + assert rev_reg_loc.get_receiving_tails_local_path(self.context) == TAILS_LOCAL + + with async_mock.patch.object( + Path, "is_file", autospec=True + ) as mock_is_file: + mock_is_file.return_value = True - assert rev_reg.get_receiving_tails_local_path(self.context) == TAILS_LOCAL + assert await rev_reg_loc.get_or_fetch_local_tails_path( + self.context + ) == TAILS_LOCAL + + rmtree(TAILS_DIR, ignore_errors=True) + assert not rev_reg_loc.has_local_tails_file(self.context) async def test_retrieve_tails(self): rev_reg = RevocationRegistry.from_definition(REV_REG_DEF, public_def=False) @@ -108,39 +124,69 @@ async def test_retrieve_tails(self): rr_def_public["value"]["tailsLocation"] = "http://sample.ca:8088/path" rev_reg = RevocationRegistry.from_definition(rr_def_public, public_def=True) + more_magic = async_mock.MagicMock() with async_mock.patch.object( - test_module, "fetch_stream", async_mock.CoroutineMock() - ) as mock_fetch: - mock_fetch.side_effect = test_module.FetchError() + test_module, "Session", autospec=True + ) as mock_session: + mock_session.return_value.__enter__=async_mock.MagicMock( + return_value=more_magic + ) + more_magic.get=async_mock.MagicMock( + side_effect=test_module.RequestException('Not this time') + ) with self.assertRaises(RevocationError) as x_retrieve: await rev_reg.retrieve_tails(self.context) assert x_retrieve.message.contains("Error retrieving tails file") - rmtree(TAILS_DIR, ignore_errors=True) + rmtree(TAILS_DIR, ignore_errors=True) + more_magic = async_mock.MagicMock() with async_mock.patch.object( - test_module, "fetch_stream", async_mock.CoroutineMock() - ) as mock_fetch: - mock_fetch.return_value = async_mock.CoroutineMock( - read=async_mock.CoroutineMock(side_effect=[b"abcd1234", b""]) + test_module, "Session", autospec=True + ) as mock_session: + mock_session.return_value.__enter__=async_mock.MagicMock( + return_value=more_magic ) + more_magic.get=async_mock.MagicMock( + return_value=async_mock.MagicMock( + iter_content=async_mock.MagicMock( + side_effect=[(b"abcd1234",), (b"", )] + ) + ) + ) + with self.assertRaises(RevocationError) as x_retrieve: await rev_reg.retrieve_tails(self.context) assert x_retrieve.message.contains( - "The has of the downloaded tails file does not match." + "The hash of the downloaded tails file does not match." ) + rmtree(TAILS_DIR, ignore_errors=True) + + more_magic = async_mock.MagicMock() with async_mock.patch.object( - test_module, "fetch_stream", async_mock.CoroutineMock() - ) as mock_fetch, async_mock.patch.object( + test_module, "Session", autospec=True + ) as mock_session, async_mock.patch.object( base58, "b58encode", async_mock.MagicMock() - ) as mock_b58enc: - mock_fetch.return_value = async_mock.CoroutineMock( - read=async_mock.CoroutineMock(side_effect=[b"abcd1234", b""]) + ) as mock_b58enc, async_mock.patch.object( + Path, "is_file", autospec=True + ) as mock_is_file: + mock_session.return_value.__enter__=async_mock.MagicMock( + return_value=more_magic ) + more_magic.get=async_mock.MagicMock( + return_value=async_mock.MagicMock( + iter_content=async_mock.MagicMock( + side_effect=[(b"abcd1234",), (b"", )] + ) + ) + ) + mock_is_file.return_value = False + mock_b58enc.return_value = async_mock.MagicMock( decode=async_mock.MagicMock(return_value=TAILS_HASH) ) - await rev_reg.retrieve_tails(self.context) - assert Path(TAILS_LOCAL).is_file() + await rev_reg.get_or_fetch_local_tails_path(self.context) + + rmtree(TAILS_DIR, ignore_errors=True) diff --git a/requirements.txt b/requirements.txt index 5eb28cb875..4a2eda93bc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,3 +9,4 @@ marshmallow==3.0.0 msgpack~=0.6.1 prompt_toolkit~=2.0.9 pynacl~=1.3.0 +requests~=2.23.0 From 4b73b55e7904f8bbef79216ed845975d01435c85 Mon Sep 17 00:00:00 2001 From: sklump Date: Mon, 16 Mar 2020 10:40:58 +0000 Subject: [PATCH 2/2] Touch up NOTICES for python requests Signed-off-by: sklump --- NOTICES | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/NOTICES b/NOTICES index cb4b8c9736..a24b3a4eee 100644 --- a/NOTICES +++ b/NOTICES @@ -374,6 +374,25 @@ Copyright (C) 2008-2011 INADA Naoki ----------------------------------------------------------------- ----------------------------------------------------------------- +=> requests + +Copyright (C) 2019 Kenneith Reitz + + 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. + +----------------------------------------------------------------- +----------------------------------------------------------------- + => prompt_toolkit Copyright (c) 2014, Jonathan Slenders