Skip to content

Fix authentication bug with httpx usage #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Deduplicate error response parsing
  • Loading branch information
c0llab0rat0r committed Jun 30, 2021
commit 567a00ab6f8201c95e440c117c98a2f00751d828
31 changes: 31 additions & 0 deletions ipfshttpclient/http_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,34 @@ def download(
finally:
for closable in closables:
closable.close()

@staticmethod
def _raise_for_response_status(
error: Exception,
status_code: int,
content: ty.List[object]
) -> None:

"""
If we have decoded an error response from the server,
use that as the exception message; otherwise, just pass
the exception on to the caller.
"""

if len(content) == 1:
item = content[0]

if isinstance(item, dict) and "Message" in item:
msg: str = item["Message"]

raise exceptions.ErrorResponse(
status_code=status_code,
message=msg,
original=error
) from error

raise exceptions.StatusError(
status_code=status_code,
message=str(error),
original=error
) from error
27 changes: 5 additions & 22 deletions ipfshttpclient/http_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ def _make_session(self) -> httpx.Client:
base_url = self._session_base,
transport = connection_pool)

@staticmethod
def _do_raise_for_status(response: httpx.Response) -> None:
@classmethod
def _do_raise_for_status(cls, response: httpx.Response) -> None:
try:
response.raise_for_status()
except httpx.HTTPError as error:
Expand All @@ -143,26 +143,9 @@ def _do_raise_for_status(response: httpx.Response) -> None:
content += list(decoder.parse_finalize())
except exceptions.DecodingError:
pass

# If we have decoded an error response from the server,
# use that as the exception message; otherwise, just pass
# the exception on to the caller.
if len(content) == 1 \
and isinstance(content[0], dict) \
and "Message" in content[0]:
msg: str = content[0]["Message"]
raise exceptions.ErrorResponse(
status_code=response.status_code,
message=msg,
original=error
) from error
else:
raise exceptions.StatusError(
status_code=response.status_code,
message=str(error),
original=error
) from error


cls._raise_for_response_status(error, response.status_code, content)

def _request(
self, method: str, path: str, params: ty.Sequence[ty.Tuple[str, str]], *,
auth: auth_t,
Expand Down
25 changes: 4 additions & 21 deletions ipfshttpclient/http_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ def _make_session(self) -> requests.Session: # type: ignore[name-defined]
session.close()
raise

@staticmethod
def _do_raise_for_status(response: requests.Response) -> None: # type: ignore[name-defined]
@classmethod
def _do_raise_for_status(cls, response: requests.Response) -> None: # type: ignore[name-defined]
try:
response.raise_for_status()
except requests.exceptions.HTTPError as error: # type: ignore[attr-defined]
Expand All @@ -132,25 +132,8 @@ def _do_raise_for_status(response: requests.Response) -> None: # type: ignore[n
except exceptions.DecodingError:
pass

# If we have decoded an error response from the server,
# use that as the exception message; otherwise, just pass
# the exception on to the caller.
if len(content) == 1 \
and isinstance(content[0], dict) \
and "Message" in content[0]:
msg: str = content[0]["Message"]
raise exceptions.ErrorResponse(
status_code=response.status_code,
message=msg,
original=error
) from error
else:
raise exceptions.StatusError(
status_code=response.status_code,
message=str(error),
original=error
) from error

cls._raise_for_response_status(error, response.status_code, content)

def _request(
self, method: str, path: str, params: ty.Sequence[ty.Tuple[str, str]], *,
auth: auth_t,
Expand Down
44 changes: 44 additions & 0 deletions test/unit/test_http_common.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@

import pytest

from ipfshttpclient.exceptions import ErrorResponse, StatusError
from ipfshttpclient.http_common import ClientSyncBase


def test_client_sync_base_raises_for_standardized_failure():
original = Exception('foo')

with pytest.raises(ErrorResponse) as failure:
ClientSyncBase._raise_for_response_status(
error=original,
status_code=405,
content=[
{
'Message': 'bar'
}
]
)

assert failure.value.status_code == 405
assert str(failure.value) == 'bar'
assert failure.value.original is original


@pytest.mark.parametrize('content', [
[],
[{'wrong': 'ignored'}],
[{'Message': 'too'}, {'Message': 'many'}]
])
def test_client_sync_base_raises_for_non_standard_failure(content):
original = Exception('qux')

with pytest.raises(StatusError) as failure:
ClientSyncBase._raise_for_response_status(
error=original,
status_code=406,
content=content
)

assert failure.value.status_code == 406
assert str(failure.value) == 'qux'
assert failure.value.original is original