From c5d0b87c75bab4defc324f2a6b8cfaea5789be5c Mon Sep 17 00:00:00 2001 From: Cory Francis Myers Date: Mon, 9 Dec 2024 17:49:15 -0800 Subject: [PATCH 1/2] fix(_streaming_download): handle retry-level JSON error responses --- client/securedrop_client/sdk/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client/securedrop_client/sdk/__init__.py b/client/securedrop_client/sdk/__init__.py index 810e3d9a0..b11c1bd80 100644 --- a/client/securedrop_client/sdk/__init__.py +++ b/client/securedrop_client/sdk/__init__.py @@ -140,6 +140,7 @@ def _streaming_download( download_finished = False while not download_finished and retry < self.download_retry_limit: + bytes_written_prev = bytes_written logger.debug(f"Streaming download, retry {retry}") try: @@ -213,10 +214,15 @@ def _streaming_download( contents = fobj.read() fobj.close() - # Check for an error response + # Check for an error response first at the request level... if contents[0:1] == b"{": logger.error(f"Retry {retry}, received JSON error response.") return self._handle_json_response(contents) + # ...and then at the retry level. + partial = contents[bytes_written_prev:bytes_written] + if retry > 0 and partial[0:1] == b"{": + logger.error(f"Retry {retry}, received JSON error response.") + return self._handle_json_response(partial) # Get the headers if proc.stderr is not None: From d233eb384e33a8429a32816035ba0add077e0191 Mon Sep 17 00:00:00 2001 From: Cory Francis Myers Date: Tue, 10 Dec 2024 16:38:12 -0800 Subject: [PATCH 2/2] test(test_download_submission_autoresume): break initial download request halfway through actual file-size Now that we can respond correctly to retry-level errors, this change should allow running "make regenerate-sdk-cassettes" on any valid "make dev" instance, not only one with submissions above a hard-coded file-size. --- client/tests/sdk/data/setup_method.yml | 10 +- .../tests/sdk/data/test_auth_badpassword.yml | 2 +- client/tests/sdk/data/test_auth_baduser.yml | 2 +- .../sdk/data/test_delete_conversation.yml | 650 ++++----- client/tests/sdk/data/test_delete_reply.yml | 176 ++- client/tests/sdk/data/test_delete_source.yml | 1272 ++++++++--------- .../tests/sdk/data/test_delete_submission.yml | 572 ++++++-- client/tests/sdk/data/test_download_reply.yml | 144 +- .../sdk/data/test_download_submission.yml | 210 +-- .../test_download_submission_autoresume.yml | 327 ++--- ...st_download_submission_autoresume_fail.yml | 267 ++-- .../test_download_submission_stream_404.yml | 271 ++-- .../sdk/data/test_error_unencrypted_reply.yml | 478 +++---- .../sdk/data/test_failed_single_source.yml | 4 +- client/tests/sdk/data/test_flag_source.yml | 644 ++++----- .../tests/sdk/data/test_get_all_replies.yml | 568 ++++---- .../sdk/data/test_get_all_submissions.yml | 171 ++- .../tests/sdk/data/test_get_current_user.yml | 8 +- .../sdk/data/test_get_replies_from_source.yml | 510 +++---- .../sdk/data/test_get_reply_from_source.yml | 532 +++---- .../tests/sdk/data/test_get_single_source.yml | 638 ++++----- client/tests/sdk/data/test_get_sources.yml | 474 +++--- client/tests/sdk/data/test_get_submission.yml | 420 +++--- .../tests/sdk/data/test_get_submissions.yml | 538 +++---- client/tests/sdk/data/test_get_users.yml | 10 +- .../sdk/data/test_get_wrong_submissions.yml | 478 +++---- client/tests/sdk/data/test_reply_source.yml | 486 +++---- .../sdk/data/test_reply_source_with_uuid.yml | 484 +++---- client/tests/sdk/data/test_seen.yml | 267 ++-- .../tests/sdk/data/test_star_add_remove.yml | 960 ++++++------- client/tests/sdk/data/test_zlogout.yml | 8 +- client/tests/sdk/test_api.py | 24 +- 32 files changed, 5829 insertions(+), 5776 deletions(-) diff --git a/client/tests/sdk/data/setup_method.yml b/client/tests/sdk/data/setup_method.yml index 6aad5450d..8e5b06719 100644 --- a/client/tests/sdk/data/setup_method.yml +++ b/client/tests/sdk/data/setup_method.yml @@ -1,22 +1,22 @@ interactions: - request: body: '{"username": "journalist", "passphrase": "correct horse battery staple - profanity oil chewy", "one_time_code": "112097"}' + profanity oil chewy", "one_time_code": "578466"}' headers: {} method: POST uri: api/v1/token response: !!python/object:securedrop_client.sdk.JSONResponse data: - expiration: '2024-02-29T23:13:01.444028Z' + expiration: '2024-12-11T02:37:22.232423Z' journalist_first_name: null journalist_last_name: null - journalist_uuid: ed64b0fa-0565-4b12-9c48-6cfe27a73fd9 - token: Ilhha2ZHZWpVcFZQVmpKdWJOUzJYRG1YVUc4QXZWS0JiOGVNQXJycE5GUFUi.ZeDzXQ.10ODTj49yF7kxKbeMm9eaayR3Xo + journalist_uuid: e7e047d5-aa5e-4afe-9a4f-c24e5f2cec8d + token: IlAzb1p1MjRubnlLOFo0SDVaajZPc2JPZWRRcjRYRUxLM3RrUmhsUEFWVTQi.Z1jewg.4VNFOk_5HSHXhgn319zzDh9JVyg headers: connection: close content-length: '290' content-type: application/json - date: Thu, 29 Feb 2024 21:13:01 GMT + date: Wed, 11 Dec 2024 00:37:22 GMT server: Werkzeug/2.2.3 Python/3.8.10 status: 200 version: 1 diff --git a/client/tests/sdk/data/test_auth_badpassword.yml b/client/tests/sdk/data/test_auth_badpassword.yml index 7de1a43e3..670f9bb4a 100644 --- a/client/tests/sdk/data/test_auth_badpassword.yml +++ b/client/tests/sdk/data/test_auth_badpassword.yml @@ -1,6 +1,6 @@ interactions: - request: - body: '{"username": "journalist", "passphrase": "no", "one_time_code": "112097"}' + body: '{"username": "journalist", "passphrase": "no", "one_time_code": "578466"}' headers: {} method: POST uri: api/v1/token diff --git a/client/tests/sdk/data/test_auth_baduser.yml b/client/tests/sdk/data/test_auth_baduser.yml index a419a34a4..401d8714c 100644 --- a/client/tests/sdk/data/test_auth_baduser.yml +++ b/client/tests/sdk/data/test_auth_baduser.yml @@ -1,7 +1,7 @@ interactions: - request: body: '{"username": "no", "passphrase": "correct horse battery staple profanity - oil chewy", "one_time_code": "112097"}' + oil chewy", "one_time_code": "578466"}' headers: {} method: POST uri: api/v1/token diff --git a/client/tests/sdk/data/test_delete_conversation.yml b/client/tests/sdk/data/test_delete_conversation.yml index f86f9568f..0fe890b68 100644 --- a/client/tests/sdk/data/test_delete_conversation.yml +++ b/client/tests/sdk/data/test_delete_conversation.yml @@ -5,7 +5,7 @@ interactions: Accept: - application/json Authorization: - - Token Ilhha2ZHZWpVcFZQVmpKdWJOUzJYRG1YVUc4QXZWS0JiOGVNQXJycE5GUFUi.ZeDzXQ.10ODTj49yF7kxKbeMm9eaayR3Xo + - Token IlAzb1p1MjRubnlLOFo0SDVaajZPc2JPZWRRcjRYRUxLM3RrUmhsUEFWVTQi.Z1jewg.4VNFOk_5HSHXhgn319zzDh9JVyg Content-Type: - application/json method: GET @@ -13,491 +13,491 @@ interactions: response: !!python/object:securedrop_client.sdk.JSONResponse data: sources: - - add_star_url: /api/v1/sources/286347a8-7d7b-4ace-8982-d5db927ed246/add_star + - add_star_url: /api/v1/sources/3f85129c-a43e-4ce5-8ec0-ab5c0b3be6bf/add_star interaction_count: 8 is_flagged: false is_starred: false - journalist_designation: conclusive musk + journalist_designation: cocksure incompetent key: - fingerprint: 95557C4B8AE2109AC287A260805532D035EFEC13 + fingerprint: 341E9C1E0468E900C6ADF0FC748BE6E54A2AEDED public: '-----BEGIN PGP PUBLIC KEY BLOCK----- - Comment: 9555 7C4B 8AE2 109A C287 A260 8055 32D0 35EF EC13 + Comment: 341E 9C1E 0468 E900 C6AD F0FC 748B E6E5 4A2A EDED - Comment: Source Key <4WU2GUDMMTNLE5FDHIHZPUSQB47SRKBV2E7QCV4ZHDY + Comment: Source Key = 200: - # Restore the original Popen before raising the exception + if self.counter >= breakpoint: + # Before raising the timeout, restore the original Popen, + # which will handle the retry request untampered. mocker.stopall() raise subprocess.TimeoutExpired( cmd=None, timeout=None, output=None, stderr=None ) chunk = content[self.counter : self.counter + n] self.counter += len(chunk) - return chunk + return chunk[:breakpoint] def close(self): self.closed = True @@ -233,8 +238,7 @@ def fileno(self): return 1 class StubbedStderr: - def __init__(self, content_length): - self.content_length = content_length + def __init__(self): self.closed = False def read(self, n=-1): @@ -244,7 +248,7 @@ def read(self, n=-1): "content-type": "application/pgp-encrypted", "etag": "sha256:3aa5ec3fe60b235a76bfc2c3a5c5d525687f04c1670060632a21b6a77c131f65", # noqa: E501 "content-disposition": "attachment; filename=3-assessable_firmness-doc.gz.gpg", # noqa: E501 - "content-length": self.content_length, + "content-length": breakpoint, "accept-ranges": "bytes", } } @@ -257,7 +261,7 @@ def fileno(self): return 2 class StubbedPopen(subprocess.Popen): - def __init__(self, *args, content="", **kwargs): + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # Only stub if the command contains "securedrop.Proxy" or "securedrop-proxy" @@ -265,7 +269,7 @@ def __init__(self, *args, content="", **kwargs): if "securedrop.Proxy" in cmd or "securedrop-proxy" in cmd: self._is_securedrop_proxy = True self.stdout = StubbedStdout() - self.stderr = StubbedStderr(len(content)) + self.stderr = StubbedStderr() else: self._is_securedrop_proxy = False