Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add base45 multibase #291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: add base45 multibase #291

wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jun 6, 2024

@rvagg rvagg requested a review from achingbrain June 6, 2024 10:47
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM

@davidlehn
Copy link

We wrote a base45 implementation too: https://github.com/digitalbazaar/base45. I came here looking for more tests and grabbed the ones I saw.

  • I didn't benchmark it, but the decode indexOf lookup here probably isn't very efficient? You might consider a static or dynamic 256 byte decode table. Makes lookups simple, and all the unused values can use a sentinel value like 0x00 or 0xFF to detect invalid chars, like -1 is checked for here.
  • Are there checks for out of range decoded values? Section 6 of the spec calls out GGW, but anything over 0xFFFF is invalid, as is trailing byte over 0xFF. We test GGW, :::, 000V5, 000::.

@rvagg
Copy link
Member Author

rvagg commented Jun 7, 2024

discussion in here makes me think that I'll make this case insensitive on decode

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants