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

Base 32 Pattern Check #37

Merged
merged 5 commits into from
Dec 5, 2020
Merged

Base 32 Pattern Check #37

merged 5 commits into from
Dec 5, 2020

Conversation

Ursify
Copy link
Contributor

@Ursify Ursify commented Dec 4, 2020

Hi Jason, Algorand Team.

I hope below explanation makes sense.

BASE 32 works in groups of 8 Characters.
Valid Final 8 Characters(with / without padding):
CC======
CC
CCCC====
CCCC
CCCCC===
CCCCC
CCCCCCC=
CCCCCCC
CCCCCCCC
Invalid Final 8 Characters(with / without padding):
C=======
C
CCCCCC==
CCCCCC

Invalid 8 characters group should throw an error, since it won't be possible to uncoded them back.

Cheers,
Ursify

Ursify and others added 2 commits December 4, 2020 13:00
BASE 32 works in groups of 8 Characters.
Valid Final 8 Characters(with / without padding):
CC======
CC
CCCC====
CCCC
CCCCC===
CCCCC
CCCCCCC=
CCCCCCC
CCCCCCCC
Invalid Final 8 Characters(with / without padding):
C=======
C
CCCCCC==
CCCCCC

Invalid 8 characters group should throw an error, since it won't be possible to uncoded them back.
@jasonpaulos
Copy link
Member

Hi @Ursify, thanks for catching this too. I made a small change so that the regex accepts empty strings too and I've updated tests and an example which had invalid base32. Do you see any problems with the updated regex?

jasonpaulos and others added 3 commits December 4, 2020 14:41
Empty string was not raising TealInputError.
Sorry about that, first I understood that the issue was that empty strings were allowed. I rolled it back.
@Ursify
Copy link
Contributor Author

Ursify commented Dec 5, 2020

Hi @Ursify, thanks for catching this too. I made a small change so that the regex accepts empty strings too and I've updated tests and an example which had invalid base32. Do you see any problems with the updated regex?

Hi @jasonpaulos , thanks for reviewing my PR. Your update is working fine. I run some test and all looks good. Cheers,

@jasonpaulos jasonpaulos merged commit eab6d88 into algorand:master Dec 5, 2020
jasonpaulos added a commit that referenced this pull request Mar 1, 2021
* Base 32 Pattern Check

BASE 32 works in groups of 8 Characters.
Valid Final 8 Characters(with / without padding):
CC======
CC
CCCC====
CCCC
CCCCC===
CCCCC
CCCCCCC=
CCCCCCC
CCCCCCCC
Invalid Final 8 Characters(with / without padding):
C=======
C
CCCCCC==
CCCCCC

Invalid 8 characters group should throw an error, since it won't be possible to uncoded them back.

* Allow empty string & update tests

* Add tests for incorrect padding

* Fix on Empty String

Empty string was not raising TealInputError.

* Allowing Empty strings

Sorry about that, first I understood that the issue was that empty strings were allowed. I rolled it back.

Co-authored-by: Jason Paulos <jasonpaulos@users.noreply.github.com>
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.

None yet

2 participants