-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
SAK-49345 Switch from Base62 to a variant of Base64 #11976
Conversation
basiclti/tsugi-util/src/test/org/tsugi/util/Base64DoubleUrlEncodeSafeTest.java
Outdated
Show resolved
Hide resolved
"Testing encoding and decoding.", | ||
"12345", | ||
"This is a longer string with more characters.", | ||
"A tree 🌲 was here", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't one of these be a really complex url esp one with an underscore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.
/* This code was written by ChatGPT with the following prompt | ||
* Please write a unit test for a Base64DoubleUrlEncodeSafe class | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup.
Base64DoubleUrlEncodeSafe.decode(invalidBase64DoubleUrlEncodeSafeString); | ||
fail("Expected IllegalArgumentException for invalid Base64DoubleUrlEncodeSafe string"); | ||
} catch (Exception e) { | ||
// Expected exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could also assert that an exception was caught, this is a good idea when an exception is expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
(cherry picked from commit 2ea87c8)
Hey @ern and @jonespm - You were right. This commit switches to using Base64 URL Safe Encoding with a small adjustment to make double url encoding not a problem but without building our own Base62 implementation. This in essence reverts the Base62 patch and replace it with an adjusted Base64 encoder.