Skip to content

base64 encoding does not pad output properly #2

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

Closed
shader opened this issue Mar 6, 2017 · 2 comments
Closed

base64 encoding does not pad output properly #2

shader opened this issue Mar 6, 2017 · 2 comments

Comments

@shader
Copy link

shader commented Mar 6, 2017

Many base64 decoding libraries expect the encoded string to be padded with one or two "=" characters so that the length is a multiple of 4, as given by the following sample code in https://tools.ietf.org/html/draft-ietf-jose-json-web-signature-08#appendix-C

static byte [] base64urldecode(string arg)
{
  string s = arg;
  s = s.Replace('-', '+'); // 62nd char of encoding
  s = s.Replace('_', '/'); // 63rd char of encoding
  switch (s.Length % 4) // Pad with trailing '='s
  {
    case 0: break; // No pad chars in this case
    case 2: s += "=="; break; // Two pad chars
    case 3: s += "="; break; // One pad char
    default: throw new System.Exception(
      "Illegal base64url string!");
  }
  return Convert.FromBase64String(s); // Standard base64 decoder
}

Can we have an option for byte-transforms/encode :base64 to include this padding?

@shader
Copy link
Author

shader commented Mar 6, 2017

After looking through your code to prepare for writing a PR for this issue, I discovered that it is currently encoding in url-safe mode by default, which obviously disables the padding because "=" is reserved for query parameters. I wasn't expecting it to be enabled by default, because the simple example in your readme file explicitly passed the {:url-safe? true} option, indicating that url-safety was not the default behavior.

Updating the readme to indicate this behavior may be the best solution, because it would likely break too many people's code to switch the default behavior. However, I would normally expect options to be disabled by default. url-safe? sounds like a feature to be enabled, rather than a standard behavior to be disabled.

@ztellman
Copy link
Collaborator

ztellman commented Mar 6, 2017

Yeah, this may have not been the best default, but I agree it's not something that can be easily changed at this point. I'll update the documentation to reflect 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