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

PrimitiveArrayDeserializers$ByteDeser.deserialize ignores DeserializationProblemHandler for invalid Base64 content #3784

Closed
or-else opened this issue Feb 11, 2023 · 9 comments
Milestone

Comments

@or-else
Copy link

or-else commented Feb 11, 2023

Describe the bug

PrimitiveArrayDeserializers$ByteDeser.deserialize throws InvalidFormatException instead of calling one of the DeserializationProblemHandler methods, probably handleWeirdStringValue.

InvalidFormatException: Cannot access contents of TextNode as binary due to broken 
Base64 encoding: Illegal character '␡'

Version information

Tested in 2.13.5, but the bug is definitely present in 2.15

To Reproduce

class Test {
   public byte[] data;
}

json:

{"data": "not a valid base64 string"}

Expected behavior

I expect deserializer to call handleWeirdStringValue (or another method of DeserializationProblemHandler, if that's more appropriate) instead of just throwing InvalidFormatException.

@or-else or-else added the to-evaluate Issue that has been received but not yet evaluated label Feb 11, 2023
@or-else
Copy link
Author

or-else commented Feb 11, 2023

The actual bug is in this line:

https://github.com/FasterXML/jackson-databind/blob/2.15/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java#L470

It expects StreamReadException while InvalidFormatException is thrown. It should catch JsonProcessingException instead.

@cowtowncoder cowtowncoder added 2.15 and removed to-evaluate Issue that has been received but not yet evaluated labels Feb 17, 2023
@cowtowncoder
Copy link
Member

Yes, that makes sense. Will need to write a unit test to properly reproduce the problem, ensure handler is called etc.

@or-else
Copy link
Author

or-else commented Feb 21, 2023

Would it be possible to backport it to 2.13? I'm stuck with 2.13 because I need Android API 24. 2.14 requires API 26.

@cowtowncoder
Copy link
Member

@or-else Hmmh. Probably, although I am not sure there will be new releases of 2.13. But fix itself could go there.

@or-else
Copy link
Author

or-else commented Feb 21, 2023

@cowtowncoder Would be nice to have a 2.13.6 with this fix. My workaround is pretty ugly.

@cowtowncoder
Copy link
Member

Alas, it takes multiple hours to publish a full patch release so there typically has to be a few more important fixes.
So it all comes down to availability of time & where it is spent.

@cowtowncoder
Copy link
Member

@or-else Actually, I realized that I am not sure how to reproduce this -- exception message suggests you are not actually reading it directly from JSON (which may actually work as expected) but probably first read content as JsonNode and then try binding? I can try to recreate it but if I can't I may need help here.

@cowtowncoder cowtowncoder changed the title PrimitiveArrayDeserializers$ByteDeser.deserialize ignores DeserializationProblemHandler PrimitiveArrayDeserializers$ByteDeser.deserialize ignores DeserializationProblemHandler for invalid Base64 content Feb 22, 2023
@cowtowncoder
Copy link
Member

Ok yes; regular read actually worked fine. It's only if reading/converting from JsonNode where different exception is thrown. I was able to fix this for 2.13 branch just in case there might be a new release; but will regardless be in next 2.14 and 2.15 releases.

@cowtowncoder cowtowncoder added this to the 2.13.6 milestone Feb 22, 2023
@or-else
Copy link
Author

or-else commented Feb 22, 2023

first read content as JsonNode and then try binding?

Correct.

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

No branches or pull requests

2 participants