Deserialize java.util.UUID encoded as Base64 and base64Url with or without padding
#4393
Conversation
| byte[] stuff = Base64Variants.getDefaultVariant().decode(id); | ||
| byte[] stuff = Base64Variants.getDefaultVariant().decode(id | ||
| //cater for url safe chars | ||
| .replace("-", "+") |
There was a problem hiding this comment.
Isn't this remarkably inefficient way to do this, using dynamically compiled regexp replaced and constructing Strings on the fly? Should be easy enough to change to character-based alternative.
But overall I am not sure how safe it is to start replacing characters here. I'll have to think about this a bit.
There was a problem hiding this comment.
replace(String, String) is actually not regex, and it even delegates to replace(char, char) if both strings are length 1. still should move to the char version though.
There was a problem hiding this comment.
Ah ok. I misremembered that then. Should probably move logic to separate helper method regardless.
There was a problem hiding this comment.
Ok, changed to chars for the replacement now + separated the logic to helper methods.
src/main/java/com/fasterxml/jackson/databind/deser/std/UUIDDeserializer.java
Show resolved
Hide resolved
| class UUIDDeserializerTest { | ||
| @Test | ||
| void testCanDeserializeUUIDFromString() throws IOException { | ||
| final UUID uuid = UUID.randomUUID(); |
There was a problem hiding this comment.
Instead of using new generated random UUIDs, it'd be better to generate one randomly but include as constant.
This to avoid any possibility of having non-reproducible fails (at expense of not covering many combinations).
It is unfortunate JDK does not offer a way to see generator for random-number variant, for testing (but perhaps they were worried about devs using that in production and end up with collisions), as otherwise we'd use that.
JUG of course can do that but perhaps it's better not to add it as a (test) dependency,.
|
@jebl01 Ok, looks good -- I think this makes sense and timing is good as this can make it into 2.17.0-rc1 release, to be released soon (within a week I hope). One thing I'd need before final review & merge: CLA (if we haven't already gotten one -- apologies if I missed earlier one). https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf (there's alternative CCLA too if preferable) and is usually easiest to print, fill & sign, scan/photo, email to Thank you once again for contributing this! |
|
CLA in the mail 📫 |
cowtowncoder
changed the title
java.util.UUID encoded as Base64 and base64Url with or without padding
|
Looks good, made some non-functional changes to help merging to |
Vert.x 4 changed to
Base64UrlEncoderas the default for binary columns in database responses (Vert.x serializes database responses into JSON arrays). This becomes a problem when Jackson tries to deserialize into a field of typeUUID. The workaround is to provide a custom serializer for UUID or to havebyte[]fields in your database entities.This PR ensures all variants of Base64 is supported for UUIDs "out of the box".