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

Added option to use a lowercase alphabet in base 16 and base 32 encode #2143

Closed
wants to merge 2 commits into from

Conversation

pma
Copy link
Contributor

@pma pma commented Mar 28, 2014

and decode functions.

Options are passed as Keyword list.
Example Base.encode16("Hello World, lowercase: true)

Following the RFC security considerations chapter, the decode functions are not case insensitive. The lowercase option will just select an alternative alphabet (with lowercase letters).

@josevalim
Copy link
Member

Thanks! Given that those affect only encoding, since decode is not case insensitive, I would like to wait a bit before this change.

Alternatively, we can simply ask people to use String.downcase/1 and String.upcase/1 at the end. In the case this can be an issue, another alternative is to provide a Base.downcase/1 and Base.upcase/1 functions that only works on the ascii subset.

@pma
Copy link
Contributor Author

pma commented Mar 29, 2014

@josevalim Yes, getting feedback from actual users will be best before considering changes / new features.

But it's worth pointing out that in the PR the decoders (base16 and base32) also accept the lowercase option, not just the encoders. They are not case insensitive because they will reject input in mixed-case. The lowercase option toggles between the uppercase and lowercase alphabets.

For the encoders I agree that we could just pipe the output through String.downcase/1 if needed. For the decoders thought piping through String.upcase/1 first would mean we would be potentially accepting input in mixed-case, which according to the RFC can have security implications.

If there is a need to make the codecs more configurable, some different approaches could result in a cleaner API:

  1. One encode and one decode function, with arguments to select base and additional options.
    Examples:
    -> Base.encode(data, :hex, lowercase: true)
    -> Base.decode(data, :base64)
    -> Base.encode(data, :base64url, padding: false)

2) Record to setup the codec configuration:
Examples:
-> Base.64().padding(false).encode(data)
-> Base.64().decode(data)
-> Base.hex().lowercase().encode(data)

Edit: Option #2 wouldn't actually work exactly like this. Damn OO mind-set

@alco alco added App:Mix and removed App:Mix labels Apr 2, 2014
@josevalim
Copy link
Member

@pma I will close this based on the idea we will wait for users feedback. IN case they desire this functionality, we can always reopen this! Thank you!

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

Successfully merging this pull request may close these issues.

3 participants