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

more relaxed Base64 decoding #89

Closed
wants to merge 1 commit into from
Closed

more relaxed Base64 decoding #89

wants to merge 1 commit into from

Conversation

karel-m
Copy link
Member

@karel-m karel-m commented Jan 2, 2016

Hi,

I would like to propose more relaxed Base64 decoding (the patch does not affect encoding). By more relaxed I mean that we do not insist on a correct number of = at the end of Base64 input.

In fact with this patch we ignore = in a same way as we ignore other unwanted characters.

Without this patch base64url_decode does not follow RFC 4648 section 5 which requires to ignore tailing = (yes it was commited by me, completely my fault :).

--Karel

@sjaeckel sjaeckel mentioned this pull request Jan 5, 2016
@sjaeckel
Copy link
Member

This is clashing with #106 and I prefer having strict checks instead of relaxed ones.
Do you think it'd make sense to integrate these changes in #106 under consideration of the strict flag?

@karel-m
Copy link
Member Author

karel-m commented Jan 26, 2016

When talking about RFC4648 there is also section 5 which defines so called base64 URL Safe variant implemented in base64url_decode (implementation is mine and is buggy, which I was about to fix by this PR).

So if #106 declares it implements strict mode according RFC4648 it should perhaps handle also section 5 not only "standard" base64.

As for the strict/relaxed mode in base64url mode:

  • let us have "standard" base64 string: vuiSPKIl8PiR5O+rC4z9/xTQKZ0=
  • strict URL safe variant looks like: vuiSPKIl8PiR5O-rC4z9_xTQKZ0
  • relaxed variant (with trailing =): vuiSPKIl8PiR5O-rC4z9_xTQKZ0=
  • relaxed variant (extra invalid chars): vuiS*PKIl8P*iR5O-rC4*z9_xTQKZ0
  • relaxed variant (combined): vuiS*PKIl8P*iR5O-rC4*z9_xTQKZ0=

Currently base64url_decode is not able to parse vuiSPKIl8PiR5O-rC4z9_xTQKZ0 neither in strict nor in relaxed mode (with or without #106).

I can fix relax mode on top of #106 so that it accepts missing trailing = as well as it ignores invalid chars. But first we should perhaps fix the strict mode to correctly handle also base64url variant.

@sjaeckel sjaeckel added this to the v2.0.0 milestone Mar 10, 2016
@sjaeckel
Copy link
Member

sjaeckel commented Apr 3, 2016

  • strict URL safe variant looks like: vuiSPKIl8PiR5O-rC4z9_xTQKZ0
  • relaxed variant (with trailing =): vuiSPKIl8PiR5O-rC4z9_xTQKZ0=

Do I understand correctly and it's the inverse?!
The one with trailing = is strict (as of [1]) and the one without = is relaxed (as of [2]), right?

[1] Ch 3.2

Implementations MUST include appropriate pad characters at the end of encoded data unless the specification referring to this document explicitly states otherwise.

[2] Ch 5

...if the data length is known implicitly, this can be avoided by skipping the padding; see section 3.2.

The can makes it optional for me and therefor the version with = becomes the strict one...

The only thing that is clear is that this part is pretty unclear... at least I don't get it...

@sjaeckel
Copy link
Member

Can you please have a look at #106 again and close this PR if you're fine with the solution implemented there?

@sjaeckel
Copy link
Member

sjaeckel commented Feb 5, 2017

*ping* @karel-m

@karel-m
Copy link
Member Author

karel-m commented Feb 21, 2017

I am closing this PR as it is now handled by #106

@karel-m karel-m closed this Feb 21, 2017
@sjaeckel sjaeckel modified the milestone: v2.0.0 Feb 21, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants