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

Fix (un)marshal bug #1135 #1294

Merged
merged 2 commits into from
Nov 28, 2019
Merged

Fix (un)marshal bug #1135 #1294

merged 2 commits into from
Nov 28, 2019

Conversation

gaol
Copy link
Contributor

@gaol gaol commented Feb 18, 2019

Fix Issue: #1135

Original PR from: javaee/jaxb-v2#1155

Signed-off-by: Lin Gao lgao@redhat.com

@eclipsewebmaster
Copy link

Issue tracker reference:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=1135

@gaol
Copy link
Contributor Author

gaol commented Feb 18, 2019

@eclipsewebmaster I have signed ECA, how to refresh the ip-validation check?

@vrlgohel
Copy link

Can someone check this please ?

@loser168
Copy link

loser168 commented May 24, 2019

This fix causes a regression where in the class is abstract and has subclasses along with a IDREF defined in it. With the new fix, While validating the generated xml against the schema it gives the following Exception.

Exception in thread "main" javax.xml.bind.MarshalException
 - with linked exception:
[org.xml.sax.SAXParseException: cvc-elt.4.3: Type 'xs:string' is not validly derived from the type definition, 'IDREF', of element 'deployment'.]
	at com.sun.xml.internal.bind.v2.runtime.MarshallerImpl.write(MarshallerImpl.java:323)
	at com.sun.xml.internal.bind.v2.runtime.MarshallerImpl.marshal(MarshallerImpl.java:248)
	at javax.xml.bind.helpers.AbstractMarshallerImpl.marshal(AbstractMarshallerImpl.java:106)
	at com.ibm.test.jaxb.MainTest.main(MainTest.java:39)
Caused by: org.xml.sax.SAXParseException: cvc-elt.4.3: Type 'xs:string' is not validly derived from the type definition, 'IDREF', of element 'deployment'.
	at org.apache.xerces.util.ErrorHandlerWrapper.createSAXParseException(Unknown Source)
	at org.apache.xerces.util.ErrorHandlerWrapper.error(Unknown Source)
	at org.apache.xerces.impl.XMLErrorReporter.reportError(Unknown Source)
	at org.apache.xerces.impl.XMLErrorReporter.reportError(Unknown Source)
	at org.apache.xerces.impl.xs.XMLSchemaValidator$XSIErrorReporter.reportError(Unknown Source)
	at org.apache.xerces.impl.xs.XMLSchemaValidator.reportSchemaError(Unknown Source)
	at org.apache.xerces.impl.xs.XMLSchemaValidator.getAndCheckXsiType(Unknown Source)
	at org.apache.xerces.impl.xs.XMLSchemaValidator.handleStartElement(Unknown Source)
	at org.apache.xerces.impl.xs.XMLSchemaValidator.startElement(Unknown Source)
	at org.apache.xerces.jaxp.validation.ValidatorHandlerImpl.startElement(Unknown Source)
	at org.xml.sax.helpers.XMLFilterImpl.startElement(Unknown Source)
	at com.sun.xml.internal.bind.v2.runtime.output.SAXOutput.endStartTag(SAXOutput.java:125)
	at com.sun.xml.internal.bind.v2.runtime.output.ForkXmlOutput.endStartTag(ForkXmlOutput.java:103)
	at com.sun.xml.internal.bind.v2.runtime.XMLSerializer.endAttributes(XMLSerializer.java:304)
	at com.sun.xml.internal.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:696)
	at com.sun.xml.internal.bind.v2.runtime.property.ArrayElementNodeProperty.serializeItem(ArrayElementNodeProperty.java:66)
	at com.sun.xml.internal.bind.v2.runtime.property.ArrayElementProperty.serializeListBody(ArrayElementProperty.java:169)
	at com.sun.xml.internal.bind.v2.runtime.property.ArrayERProperty.serializeBody(ArrayERProperty.java:156)
	at com.sun.xml.internal.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:357)
	at com.sun.xml.internal.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:348)
	at com.sun.xml.internal.bind.v2.runtime.XMLSerializer.childAsXsiType(XMLSerializer.java:697)
	at com.sun.xml.internal.bind.v2.runtime.property.SingleElementNodeProperty.serializeBody(SingleElementNodeProperty.java:157)
	at com.sun.xml.internal.bind.v2.runtime.ClassBeanInfoImpl.serializeBody(ClassBeanInfoImpl.java:357)
	at com.sun.xml.internal.bind.v2.runtime.XMLSerializer.childAsSoleContent(XMLSerializer.java:590)
	at com.sun.xml.internal.bind.v2.runtime.ClassBeanInfoImpl.serializeRoot(ClassBeanInfoImpl.java:338)
	at com.sun.xml.internal.bind.v2.runtime.XMLSerializer.childAsRoot(XMLSerializer.java:491)
	at com.sun.xml.internal.bind.v2.runtime.MarshallerImpl.write(MarshallerImpl.java:320)
	... 3 more

It doesnt check the IDREF in the isLeaf() method of com/sun/xml/internal/bind/v2/runtime/property/PropertyFactory.java/PropertyFactory.java , since it returns false from the new code now.

The reproducer project where the JaxbContainer class you can see it is abstract, has subclasses and an IDREF defined in it.

We get a wrong xml with IDREF type treated as a discrete type without any connection to ID.

Expected xml

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<environmentModel>
    <container xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="concreteContainerType">
        <deployments>
            <deployment>Context-Root</deployment>
        </deployments>
    </container>
    <distribution>
        <deployments>
            <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="concreteDeploymentType">
                <contextRoot>Context-Root</contextRoot>
            </deployment>
        </deployments>
    </distribution>
</environmentModel>

Actual xml

<environmentModel>
    <container xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="concreteContainerType">
        <deployments>
            <deployment xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Context-Root</deployment>
        </deployments>
    </container>
    <distribution>
        <deployments>
            <deployment xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="concreteDeploymentType">
                <contextRoot>Context-Root</contextRoot>
            </deployment>
        </deployments>
    </distribution>
</environmentModel>

I think this fix needs restructuring or additional checks.

@loser168
Copy link

loser168 commented Jun 3, 2019

@gaol Can you please comment on my previous update?

@gaol
Copy link
Contributor Author

gaol commented Jun 4, 2019

@loser168 Hi, I am very sorry that the message was drowned that I did not notice. The PR comes from the old repository by another guy. So I need to investigate it before I can give any comments :)

I am currently working on another issue, and I hope I can be back to this issue this week. :) Thanks again. :)

@loser168
Copy link

loser168 commented Jun 6, 2019

Thanks @gaol. Will wait for your inputs :)

@gaol
Copy link
Contributor Author

gaol commented Jun 10, 2019

@loser168 Hi, I just updated the PR by moving the subclass checks down after IDREF checks. It passed tests in my local environment(JDK11), and it passed the regression you found as well. Would you please confirm? Thank you very much!

@gaol
Copy link
Contributor Author

gaol commented Jul 2, 2019

@loser168 I also included your regression tests into the PR, would you please review? Can anyone move this issue on? Thanks.

@gaol
Copy link
Contributor Author

gaol commented Nov 14, 2019

Can anyone review? This is just one line change, and it has been pending for several months.

Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

new files are missing license headers

Original PR: javaee/jaxb-v2#1155

Move the original fix down after IDREF check

Signed-off-by: Lin Gao <lgao@redhat.com>
Refer to: https://github.com/NiasSt90/jaxb-bug1135

Signed-off-by: Lin Gao <lgao@redhat.com>
@gaol
Copy link
Contributor Author

gaol commented Nov 28, 2019

@lukasj Thank you for the review, I added the missing license headers to the new files. Would you please review again? Thanks :)

Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Just for the record new files created in 2019 should say just 2019 and not 1997.

@lukasj lukasj merged commit d134fd3 into eclipse-ee4j:master Nov 28, 2019
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.

5 participants