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

CXF-8966 : Validation of nillable tags fails #1558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

f2par0
Copy link

@f2par0 f2par0 commented Dec 6, 2023

Change xml validation strategy to use the Woodstox validation when reading instead of having a reader and a writer to a nulloutput because this method is not able to handle the xsi:nil=true attributes.

Previous Algorithm :

  • Transform the input to a DOMSource Object
  • create a XMLReader from this new object and a XMLWriter to a nulloutput
  • setup the wookdstock validation on the writer if possible
  • Create a filtered Reader to remove external references
  • Copy the filterReader to the Null Writer
  • if setup fails, use XOP validation

Suggested algorithm :

  • create a filtered reader from the input to remove external references
  • setup the wookdstock validation on the reader if possible
  • call read
  • if setup fails, use XOP validation

@f2par0 f2par0 marked this pull request as draft December 7, 2023 09:14
@f2par0 f2par0 force-pushed the CXF-8966 branch 2 times, most recently from 1b8d4e0 to 39c98a0 Compare December 7, 2023 10:00
@f2par0 f2par0 marked this pull request as ready for review December 7, 2023 10:38
@reta
Copy link
Member

reta commented Dec 16, 2023

@f2par0 thank you for attempt to fix the issue, I believe the issue is identified here FasterXML/woodstox#179 and it seems like the issue is not within CXF or Woodstox but MSV

@f2par0
Copy link
Author

f2par0 commented Dec 19, 2023

Yes, thanks for finding this issue. So this PR is more a workaround to avoid to call MSV writer.

@reta
Copy link
Member

reta commented Dec 19, 2023

Yes, thanks for finding this issue. So this PR is more a workaround to avoid to call MSV writer.

I would prefer the wait for fix to come from upstream if possible and not merge any workarounds

@ppalaga
Copy link
Contributor

ppalaga commented Jan 5, 2024

FasterXML/woodstox#179 is a Woodstox issue after all - see FasterXML/woodstox#179 (comment)

@ppalaga
Copy link
Contributor

ppalaga commented Jan 14, 2024

A fix for FasterXML/woodstox#179 is underway: FasterXML/woodstox#187

@NiasSt90
Copy link

Why not change in general the approach to run the validation on reader-side instead of on a (dummy) writer during a additional copy process?

I like the approach of these PR here to change.....

I've tried it by myself here in a similar way.

@ppalaga
Copy link
Contributor

ppalaga commented Feb 29, 2024

Sorry for the delay, FasterXML/woodstox#187 got stuck due to legal issues with the CLA. I hope those get resolved soonish.

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.

4 participants