Skip to content

Use StreamConstraintsException in name canonicalizers - #948

Merged
cowtowncoder merged 3 commits into
FasterXML:2.15from
pjfanning:use-ioexception-canonicalizer
Mar 14, 2023
Merged

Use StreamConstraintsException in name canonicalizers#948
cowtowncoder merged 3 commits into
FasterXML:2.15from
pjfanning:use-ioexception-canonicalizer

Conversation

@pjfanning

Copy link
Copy Markdown
Member

@pjfanning

Copy link
Copy Markdown
Member Author

If this change is ok for 2.15 and 2.15 already has some small exception throwing changes to the APIs - then maybe a similar change shoul go into the byte canonicalizer.

@cowtowncoder

Copy link
Copy Markdown
Member

Definitely should behave the same for byte-backed symbol table too.

One thing to verify: that all 2.15 format backends are aligned -- I think this may be source incompatibility but bytecode compatible (since exceptions are not part of method signature).

One question/thought -- is there any Jackson IOException subtype we could/should use?
Perhaps this could be constraints failure (not per-document, globally)?

@pjfanning

Copy link
Copy Markdown
Member Author

StreamConstraintsException? Or a new class with similar definition to that?

@cowtowncoder

Copy link
Copy Markdown
Member

@pjfanning Was thinking of StreamConstraintsException. While not 100% accurate, has sort of similar domain.

@pjfanning

Copy link
Copy Markdown
Member Author

@pjfanning Was thinking of StreamConstraintsException. While not 100% accurate, has sort of similar domain.

Change made. Also applied similar changes in the ByteQuadsCanonicalizer

if (_parent == null) {
if (_count == 0) { // root
throw new IllegalStateException("Cannot add names to Root symbol table");
throw new StreamConstraintsException("Cannot add names to Root symbol table");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, I'll change this back, actually, since it's internal sanity check (not something users can trigger). Same for next one.

@cowtowncoder
cowtowncoder merged commit 25313f1 into FasterXML:2.15 Mar 14, 2023
@cowtowncoder cowtowncoder changed the title use checked exception in canonicalizer Use StreamConstraintsException in name canonicalizers Mar 14, 2023
@pjfanning
pjfanning deleted the use-ioexception-canonicalizer branch March 14, 2023 23:12
@cowtowncoder

cowtowncoder commented Mar 14, 2023

Copy link
Copy Markdown
Member

Thank you @pjfanning! I think this will now resolve one last OSS-Fuzz issue that I had not tackled in 2.x (only 3.0).

EDIT: had to make minor changes to CBOR, Smile codecs to expose IOException, now all compile again (and I don't think it is bytecode-incompatible across versions despite being source-compatible)

@mail4csm

Copy link
Copy Markdown

After trying out "2.15.0-rc1" error pops up

java.lang.NoClassDefFoundError: com/fasterxml/jackson/core/exc/StreamConstraintsException

at com.fasterxml.jackson.dataformat.xml.XmlFactory._createParser(XmlFactory.java:670)
at com.fasterxml.jackson.dataformat.xml.XmlFactory._createParser(XmlFactory.java:30)
at com.fasterxml.jackson.core.JsonFactory.createParser(JsonFactory.java:1123)
at com.fasterxml.jackson.databind.ObjectMapper.readTree(ObjectMapper.java:3138)

@cowtowncoder

Copy link
Copy Markdown
Member

@mail4csm You have a version mismatch somewhere: somehow you have older jackson-core than jackson-dataformat-xml -- minor versions need to match (specifically you need 2.15.0-rc1 of jackson-core, not just jackson-dataformat-xml).

@mail4csm

mail4csm commented Mar 20, 2023

Copy link
Copy Markdown

@cowtowncoder thanks, found the issue - indirect dependency was with older version of jackson-core. Thanks!

@cowtowncoder

Copy link
Copy Markdown
Member

@mail4csm yes that makes sense.

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

Successfully merging this pull request may close these issues.

3 participants