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.

Sign up for GitHub

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

Jump to bottom

Use StreamConstraintsException in name canonicalizers #948

Merged
merged 3 commits into from Mar 14, 2023
Merged

Use StreamConstraintsException in name canonicalizers #948

merged 3 commits into from Mar 14, 2023

Conversation

pjfanning
Copy link
Member

@pjfanning
Copy link
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
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
Member Author

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

@cowtowncoder
Copy link
Member

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

@pjfanning
Copy link
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

cowtowncoder
cowtowncoder reviewed Mar 14, 2023
View reviewed changes
src/main/java/com/fasterxml/jackson/core/sym/ByteQuadsCanonicalizer.java
{
if (_hashShared) {
// 12-Mar-2021, tatu: prevent modifying of "placeholder" and
// parent tables
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
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
cowtowncoder added a commit that referenced this pull request Mar 14, 2023
@cowtowncoder
Minor tweaks post #948 wrt internal exceptions; update release notes
ff8b1cc
@pjfanning pjfanning deleted the use-ioexception-canonicalizer branch March 14, 2023 23:12
@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 14, 2023 •
edited

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

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
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
Copy link

mail4csm commented Mar 20, 2023 •
edited

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

@cowtowncoder
Copy link
Member

@mail4csm yes that makes sense.

@rdehuyss rdehuyss mentioned this pull request May 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cowtowncoder cowtowncoder cowtowncoder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pjfanning @cowtowncoder @mail4csm