Skip to content
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

Macaroon.deserialize raises very general exceptions #50

Open
cjwatson opened this issue Apr 23, 2019 · 2 comments
Open

Macaroon.deserialize raises very general exceptions #50

cjwatson opened this issue Apr 23, 2019 · 2 comments

Comments

@cjwatson
Copy link

Macaroon.deserialize can raise at least:

  • MacaroonInitException (if the input is None or empty)
  • various subclasses of ValueError (various causes depending on the serializer: if the input is a non-UTF-8 byte string or invalid JSON, or if a packet header isn't valid hex)
  • TypeError (if base64 padding is wrong, although admittedly this can only happen if there's a programming error in the padding-restoration functions)
  • struct.error (if encountering a short packet)
  • MacaroonDeserializationException (various causes)
  • perhaps other possibilities that I missed

This puts calling code in the position of having to catch a lot of exceptions, and given that the range of possibilities is not documented it often ends up being best to catch Exception, but that means that it's very easy to accidentally mask programming errors, which is unsatisfactory.

Ideally it should be guaranteed that Macaroon.deserialize only ever raises MacaroonDeserializationException, or at least some subclass of MacaroonException, so that calling code can catch anything raised by pymacaroons while being confident that it won't accidentally catch exceptions raised by other code. And, in any event, the possible exceptions should be documented.

@di
Copy link

di commented May 6, 2020

Macaroon.deserialize can also raise an IndexError:

>>> import pymacaroons
>>> pymacaroons.Macaroon.deserialize(' ')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/macaroon.py", line 47, in deserialize
    return serializer.deserialize(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 90, in deserialize
    return self.deserialize_raw(decoded)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 96, in deserialize_raw
    first = six.byte2int(serialized[:1])
IndexError: index out of range

@di
Copy link

di commented May 18, 2020

Macaroon.deserialize can also raise a UnicodeDecodeError:

>>> pymacaroons.Macaroon.deserialize(b'h\xcf')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/macaroon.py", line 47, in deserialize
    return serializer.deserialize(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 88, in deserialize
    serialized = convert_to_string(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/utils.py", line 26, in convert_to_string
    return string_or_bytes.decode('utf-8')
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xcf in position 1: unexpected end of data

As well as a generic Exception:

>>> pymacaroons.Macaroon.deserialize('Age')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/macaroon.py", line 47, in deserialize
    return serializer.deserialize(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 90, in deserialize
    return self.deserialize_raw(decoded)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 98, in deserialize_raw
    return self._deserialize_v2(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 159, in _deserialize_v2
    serialized, section = self._parse_section_v2(serialized)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 265, in _parse_section_v2
    rest, packet = self._parse_packet_v2(data)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 293, in _parse_packet_v2
    payload_len, n = _decode_uvarint(data)
  File "/private/tmp/env/lib/python3.8/site-packages/pymacaroons/serializers/binary_serializer.py", line 348, in _decode_uvarint
    raise Exception('cannot read uvarint from buffer')
Exception: cannot read uvarint from buffer

The latter is probably a bug, IMO this library should never produce a generic Exception like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants