Skip to content

Support string input #21

Description

@ecederstrand

I have an str input stream that I would like to decode. base64.b64decode() handles str just fine, so I would like to avoid ASCII encoding the input, which would consume the entire stream unless I'm very clever. I tried this:

>>> from base64 import b64decode
>>> from io import StringIO
>>> b64decode('SGVsbG8gZnJvbSB1bmljb2RlIMOmw7jDpQ==')
b'Hello from unicode \xc3\xa6\xc3\xb8\xc3\xa5'
>>> Base64IO(StringIO('SGVsbG8gZnJvbSB1bmljb2RlIMOmw7jDpQ==')).read()
  File "/usr/lib/python3.5/site-packages/base64io/__init__.py", line 276, in read
    if any([char.encode("utf-8") in data for char in string.whitespace]):
  File "/usr/lib/python3.5/site-packages/base64io/__init__.py", line 276, in <listcomp>
    if any([char.encode("utf-8") in data for char in string.whitespace]):
TypeError: 'in <string>' requires string as left operand, not bytes

which fails in https://github.com/aws/base64io-python/blob/master/src/base64io/__init__.py#L276 because the code assumes data is a bytes instance.

  1. It strikes me as a quite heavy operation to pass over each piece of data 5 times (the length of string.whitespace) just to test for possible whitespace. Maybe handling whitespace should be configurable?
  2. If I change the line to if any([char in data for char in string.whitespace]): then the example code works fine. So we could test the type of data and then run the version that applies.
  3. A similar, small patch is required in _read_additional_data_removing_whitespace()

Any comments?

Activity

  1. changed the title [-]Support `str` input[/-] [+]Support string input[/+] on Oct 25, 2018
  2. ecederstrand commented on Oct 25, 2018

    @ecederstrand
    ContributorAuthor

    Also, it seems base64.b64decode() already handles whitespaces out of the box, at least with Python 3.5:

    >>> from base64 import b64decode
    >>> import string
    >>> b64decode('SGVsbG8gZn    JvbSB1bmljb2RlIMOmw7jDpQ==')
    b'Hello from unicode \xc3\xa6\xc3\xb8\xc3\xa5'
    >>> b64decode('SGVsbG8gZn    JvbSB1b    \n    \t   mljb2RlIMOmw7jDpQ==  ')
    b'Hello from unicode \xc3\xa6\xc3\xb8\xc3\xa5'
    >>> b64decode('SGVsbG8gZnJvbSB' + string.whitespace + '1bmljb2RlIMOmw7jDpQ==')
    b'Hello from unicode \xc3\xa6\xc3\xb8\xc3\xa5'

    EDIT: This assumes, of course, that b64decode has access to the full stream.

  3. mattsb42-aws commented on Oct 30, 2018

    @mattsb42-aws
    Contributor

    This is a good catch. b64decode does in fact accept str as input in 2 and since 3.3, so we should as well. Fortunately, we duck-type everywhere except for these encoding steps, so as you've seen, there's not a lot that needs to change.

    I don't think that making the whitespace removal optional is the right approach. I would rather we find a more efficient way of performing it if this is an issue. Have you seen a performance issue with the whitespace removal? I'm wondering if just always passing through _read_additional_data_removing_whitespace might be the right approach rather than passing through all members of string.whitespace to check first.

  4. ecederstrand commented on Nov 1, 2018

    @ecederstrand
    ContributorAuthor

    Thanks for the feedback! The ignore_whitespace patch was just to show what I meant. I have benchmarked the whitespace removal code with a 10MB random str and it's almost instant, so I agree there's no obvious performance penalty there.

    I'll implement the suggestions in the code review and drop the other PR.

  5. mattsb42-aws commented on Dec 10, 2018

    @mattsb42-aws
    Contributor

    This is released in 1.0.3 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions