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

Exception when parsing Lists of mixed content (since 2.13.0) #509

Closed
cornwe19 opened this issue Jan 6, 2022 · 12 comments
Closed

Exception when parsing Lists of mixed content (since 2.13.0) #509

cornwe19 opened this issue Jan 6, 2022 · 12 comments
Labels
2.16 For issues planned for 2.16
Milestone

Comments

@cornwe19
Copy link

cornwe19 commented Jan 6, 2022

In Jackson 2.12 the following junit example test (written in Kotlin) works as I'd expect it to

@JsonIgnoreProperties(ignoreUnknown = true)
data class Tspan(
    @field:JacksonXmlText
    var content: String? = null
)

@JsonIgnoreProperties(ignoreUnknown = true)
class Text(
    @field:JacksonXmlElementWrapper(useWrapping = false)
    @field:JacksonXmlProperty(localName = "tspan")
    var spans: List<Tspan>? = null
)

class ExampleTest {
    @Test
    fun svgExample() {
        val text = XmlMapper().readValue("<text>Mixed <tspan>content</tspan> should<tspan> work</tspan></text>", Text::class.java)

        assertThat(text.spans, hasSize(2))
    }
}

But when I upgrade to 2.13, I get the following error

Unexpected non-whitespace text (' should' in Array context: should not occur (or should be handled)
 at [Source: (StringReader); line: 1, column: 49] (through reference chain: com.visualdx.api.kmax.graphql.Text["tspan"])
com.fasterxml.jackson.databind.JsonMappingException: Unexpected non-whitespace text (' should' in Array context: should not occur (or should be handled)
 at [Source: (StringReader); line: 1, column: 49] (through reference chain: com.visualdx.api.kmax.graphql.Text["tspan"])

This feels like a bug, but I'm not sure if there's some workaround or setting I'm missing somewhere.

@ptziegler
Copy link
Contributor

I ran into the same issue earlier today. This problem seems to have been introduced with 6c37f48

// 29-Mar-2021, tatu: This seems like an error condition...
// How should we indicate it? As of 2.13, report as unexpected state
 throw _constructError("Unexpected non-whitespace text ('"+_currText+"' in Array context: should not occur (or should be handled)"

For example, following entry is triggering this "error state". I think throwing an error is the wrong course of action here, so I suggest removing this error check again.

<metaData>
	<data key="MadeWith">Created with XXX</data>
	<data key="Version">12345</data>
</metaData>

Is there a chance this can be fixed? Because right now, it's blocking us from updating to a newer version.

@cowtowncoder
Copy link
Member

cowtowncoder commented May 24, 2023

Sounds like something to address, yes.

I know there is a test but it'd be great to get a simpler one if possible: also, plan Java as I assume this is not Kotlin-specific.
Put another way: a unit test reproduction would make it easier for me to see what exactly is going on. Use of @JsonIgnoreProperties(ignoreUnknown = true), for example, tends to hide all kinds of real issues.

@cowtowncoder
Copy link
Member

@ptziegler Any chance you could provide full reproduction; or at least POJOs being used? Your case is somewhat different as it only contains (ignorable) white-space, not non-empty content, so it might be supportable.

The original case isn't possible to map to POJOs (can be mapped to JsonNode, however, or as java.lang.Object).

ptziegler added a commit to ptziegler/jackson-dataformat-xml that referenced this issue Jul 28, 2023
This adds two additional test cases. Those test attempt to deserialize
the two classes Data and MetaData.

The MetaData class simply contains a list of Data objects. The Data
class contains a "key", which is derived from the attribute of the XML
node, and a "content" list, which is the arbitrary data stored inside
that node.
ptziegler added a commit to ptziegler/jackson-dataformat-xml that referenced this issue Jul 28, 2023
This adds two additional test cases. Those tests attempt to deserialize
the two classes Data and MetaData.

The MetaData class simply contains a list of Data objects. The Data
class contains a "key", which is derived from the attribute of the XML
node, and a "content" list, which is the arbitrary data stored inside
that node.
ptziegler added a commit to ptziegler/jackson-dataformat-xml that referenced this issue Jul 28, 2023
This adds two additional test cases. Those tests attempt to deserialize
the two classes Data and MetaData.

The MetaData class simply contains a list of Data objects. The Data
class contains a "key", which is derived from the attribute of the XML
node, and a "content" list, which is the arbitrary data stored inside
that node.
@ptziegler
Copy link
Contributor

Any chance you could provide full reproduction; or at least POJOs being used?

@cowtowncoder I've created #604

ptziegler added a commit to ptziegler/jackson-dataformat-xml that referenced this issue Jul 28, 2023
This adds two additional test cases. Those tests attempt to deserialize
the two classes Data and MetaData.

The MetaData class simply contains a list of Data objects. The Data
class contains a "key", which is derived from the attribute of the XML
node, and a "content" list, which is the arbitrary data stored inside
that node.
@cowtowncoder
Copy link
Member

Thank you @ptziegler !

@wickstopher
Copy link

Hello! I'm wondering if there is any recent movement on this issue? Spring Framework 6.1 was released last November, raising the minimum requirement for Jackson to 2.14 (https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#upgrading-to-version-61).

Our project relies on being able to parse mixed content, and this issue is blocking us from upgrading to the latest version of Spring, as we need to pin 2.12 to avoid this bug. I'd be happy to look into contributing back to the project in order to expedite this fix if possible.

@cowtowncoder
Copy link
Member

Unfortunately I haven't had time to look into this @wickstopher. There is a failing test under src/test/java/com/fasterxml/jackson/dataformat/xml/failing/UnexpectedNonWhitespaceText509Test.java for reproduction; all help would be appreciated.

Fix would need to go in 2.16 branch or later (2.18 is the next minor version to add but unless fix requires API change, can be merged to latest open branch which I think is 2.16).

@cowtowncoder
Copy link
Member

Also verified that the problem exists in 2.18 branch and not been fixed by some other changes.

@wickstopher
Copy link

wickstopher commented May 10, 2024

@cowtowncoder Thanks for the reply. I also have somewhat limited time, but in looking into it yesterday evening it looks like a (potentially incorrect?) assumption was made when the following Exception case was added in FromXmlParser.java:

// 29-Mar-2021, tatu: This seems like an error condition...
//   How should we indicate it? As of 2.13, report as unexpected state
throw _constructError(
"Unexpected non-whitespace text ('"+_currText+"' in Array context: should not occur (or should be handled)"
);

2.13 is the version in which this functionality broke, and removing this throw resolves the issue without breaking any additional tests. I understand there may be a more robust fix on the table as alluded to here (#604 (comment)), but in the meantime removing this throw doesn't appear to break anything, and restores functionality that was lost with the 2.13 release.

Would you be willing to consider a stopgap PR in which this was addressed in such a manner? If so I'd be happy to put one together.

@cowtowncoder
Copy link
Member

@wickstopher Yes, that sounds reasonable given the lack of time for finding proper fix to handling, so PR for 2.16 branch would be welcome (with an additional comment pointing to this issue as follow-up)

wickstopher added a commit to VisualDx/jackson-dataformat-xml that referenced this issue May 10, 2024
wickstopher added a commit to VisualDx/jackson-dataformat-xml that referenced this issue May 10, 2024
@wickstopher
Copy link

@cowtowncoder PR is up here: #651

Thanks very much for your consideration!

@cowtowncoder cowtowncoder changed the title Jackson 2.13 throws when parsing lists of mixed content Exception when parsing Lists of mixed content (since 2.13.0) May 10, 2024
@cowtowncoder
Copy link
Member

Temporary fix included in:

  • 2.16.3
  • 2.17.2
  • 2.18.0

whenever those are released (may be a while as 2.17.2 was just released; 2.16 has only this fix in branch)

@cowtowncoder cowtowncoder added 2.16 For issues planned for 2.16 and removed test-needed labels May 10, 2024
@cowtowncoder cowtowncoder added this to the 2.16.3 milestone May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16 For issues planned for 2.16
Projects
None yet
Development

No branches or pull requests

4 participants