Skip to content

bpo-29427: allow unpadded input and ouput in base64 module#7072

Closed
guillp wants to merge 8 commits intopython:mainfrom
guillp:bpo-29427
Closed

bpo-29427: allow unpadded input and ouput in base64 module#7072
guillp wants to merge 8 commits intopython:mainfrom
guillp:bpo-29427

Conversation

@guillp
Copy link
Copy Markdown

@guillp guillp commented May 23, 2018

Allow base64.b64decode() to accept unpadded input instead of throwing a binascii.Error. Padding check is switchable using the new padded boolean argument, and is enabled by default to maintain backward compatibility.
In a symmetric fashion, allow base64.b64encode() to produce unpadded output, using the padded argument. Default output will include padding for backward compatibility.

Updated test_base64 unittests to include new tests for optional padding.

https://bugs.python.org/issue29427

guillp added 2 commits May 23, 2018 17:10
Allow base64.b64decode() to accept unpadded input instead of throwing a binascii.Error. Padding check is switchable using the new `padded` boolean argument, and is enabled by default to maintain backward compatibility.
In a symmetric fashion, allow base64.b64encode() to produce unpadded output, using the `padded` argument. Default output will include padding for backward compatibility.
Added some test cases for base64 module, to test optional padding in b64decode() and b64encode(), urlsafe_b64encode() and urlsafe_b64decode().
@the-knights-who-say-ni
Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

Thanks again to your contribution and we look forward to looking at it!

@guillp
Copy link
Copy Markdown
Author

guillp commented May 24, 2018

Good catch, this indeed breaks when the string to decode contains non-base64 characters, which would be ignored by binascii.a2b_decode().
I don't think we should reject strings that are incompletely padded, we should ignore the adding instead (just like it was a non-base64 character); unless validate=True, in that case we should reject strings that contain padding.

If you agree, I'll push a fix including all that.

BTW, I signed the CLA.

Fixes b64decode on non padded input if that input contains invalid characters.
Implements validate=True for non padded input.
@FranklinYu
Copy link
Copy Markdown

Any news?

@djc
Copy link
Copy Markdown
Contributor

djc commented Sep 3, 2019

@guillp there seem to be some whitespace issues (according to Travis). Could you fix those up? I'd love to have this feature!

Copy link
Copy Markdown
Contributor

@sir-sigurd sir-sigurd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that documentation should be updated.

@bitti
Copy link
Copy Markdown

bitti commented Oct 6, 2019

What's the holdup? Can't the maintainer edit the PR if @guillp seems to have forgotten about it?

guillp and others added 2 commits October 8, 2019 14:57
updated commit description

Co-Authored-By: Sergey Fedoseev <fedoseev.sergey@gmail.com>
Co-Authored-By: Sergey Fedoseev <fedoseev.sergey@gmail.com>
@guillp
Copy link
Copy Markdown
Author

guillp commented Oct 8, 2019

Very sorry about that. git push to github.com is not allowed from my workplace, so I add to commit manually from github.com UI which is a pain. Thanks to @sir-sigurd for the review.

@dfrankow
Copy link
Copy Markdown

I hope this is not dead, it would've been useful for me.

if not padded:
if validate and not re.match(b'^[A-Za-z0-9+/]*$', s):
raise binascii.Error('Padding found in supposedly non-padded input')
s += b'=='
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why two ==s is always the right padding. I may be missing something.

Copy link
Copy Markdown

@bitti bitti Feb 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably depends on the base64 flavor of what is accepted as "right" padding, but as far as I understand the a2b_base64 implementation extra padding characters are just ignored, so always appending == should be safe in this context.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. Maybe worth putting that in a comment in the code.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using util from django.utils.http.urlsafe_base64_encode

https://docs.djangoproject.com/en/3.1/ref/utils/#django.utils.http.urlsafe_base64_encode

_urlsafe_decode_translation = bytes.maketrans(b'-_', b'+/')

def urlsafe_b64encode(s):
def urlsafe_b64encode(s, validate=False, padded=True):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validate arg isn't used. And if it's going to be added it should be added to urlsafe_b64decode instead.

@Elkasitu
Copy link
Copy Markdown

Is there anything that can be done to push this change forward? Should a new PR be submitted if the original contributor has lost interest?

@guillp guillp closed this by deleting the head repository May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.