Project

Profile

Help

Bug #4744

Schema validation errors not reported to ErrorListener

Added by Marek Skorek 12 months ago. Updated 6 months ago.

Status:
In Progress
Priority:
High
Assignee:
Category:
Schema conformance
Sprint/Milestone:
-
Start date:
2020-09-22
Due date:
% Done:

0%

Estimated time:
Legacy ID:
Applies to branch:
9.9
Fix Committed on Branch:
10, 9.9, trunk
Fixed in Maintenance Release:

Description

Hi,

Our regression test detected a bug in Saxon 9.9 which was fixed in Saxon 9.7.21 version. I attached a test XML file and XSD to validate this file against.

Please feel free to add this test case to Saxon test suites.

UniqueKeyBug.xml (117 Bytes) UniqueKeyBug.xml Test XML file Marek Skorek, 2020-09-22 10:31
UniqueKeyBug.xsd (1.1 KB) UniqueKeyBug.xsd XSD to validate test file against Marek Skorek, 2020-09-22 10:31

History

#1 Updated by Marek Skorek 12 months ago

This was previously reported in https://saxonica.plan.io/issues/3490

#2 Updated by Michael Kay 12 months ago

  • Category set to Schema conformance
  • Status changed from New to In Progress
  • Assignee set to Michael Kay

In both 9.9.1.7 and 10.2 I'm seeing a validation error here:

Validation error on line 2 column 69 of UniqueKeyBug.xml:
  Non-unique value found for constraint uniqueKey: ("A", "B", "C", "D")
  See http://www.w3.org/TR/xmlschema-1/#cvc-identity-constraint clause 4.1

which is the outcome I would expect. If you aren't seeing an error, could you provide more details of how you are running the validation, please?

#3 Updated by Marek Skorek 12 months ago

Hello Micheal, thank you for your response.

I can confirm that running validation from command line produces validation error as expected.

After further investigation I found that real source of our problem is that version 9.9.1.7 doesn't notify error listner (set up by method setErrorListener on SchemaValidator instance) as opposed to version 9.7.0.21 where error method was invoked with following exception

org.xml.sax.SAXParseException; lineNumber: 4; columnNumber: 33; Non-unique value found for constraint uniqueKey: ("A", "B", "C", "D")
....
Caused by: ValidationException: Non-unique value found for constraint uniqueKey: ("A", "B", "C", "D")
....

Do you have any suggestions?

#4 Updated by Michael Kay 12 months ago

OK, I'll try and see if I can reproduce it with a SchemaValidator. 9.9 introduced a new mechanism for reporting invalidity, but setErrorListener() should still work.

#5 Updated by Michael Kay 12 months ago

Sorry, can't reproduce the problem. The following JUnit test works as expected (on both 9.9 and 10.2):

public void testErrorListenerBug4744() {
        final List<TransformerException> errorList = new ArrayList<>();
        FileOutputStream out = null;
        try {
            out = new FileOutputStream(ConfigTest.DATA_DIR + "sandpit/validation.out");
            Processor proc = new Processor(true);
            SchemaManager sm = proc.getSchemaManager();
            StreamSource source_xsd = new StreamSource(new File("...../UniqueKeyBug.xsd"));
            StreamSource source_xml = new StreamSource(new File("...../UniqueKeyBug.xml"));
            ErrorListener listener = new ErrorListener() {
                @Override
                public void error(TransformerException exception) {
                    errorList.add(exception);
                }

                @Override
                public void fatalError(TransformerException exception) throws TransformerException {
                    throw exception;
                }

                @Override
                public void warning(TransformerException exception) {
                    errorList.add(exception);
                }
            };

            SchemaValidator sv = sm.newSchemaValidator();
            sv.setErrorListener(listener);
            sm.load(source_xsd);

            Serializer serializer = proc.newSerializer();
            serializer.setOutputProperty(Serializer.Property.INDENT, "yes");
            serializer.setOutputProperty(Serializer.Property.OMIT_XML_DECLARATION, "yes");
            serializer.setOutputStream(out);
            sv.setDestination(serializer);

            sv.validate(source_xml);
            fail("Invalidity not detected") ;
            out.close();

        } catch (SaxonApiException sapie) {
            try {
                out.close();
            } catch (IOException ee) {
                fail();
            }
            assertEquals(1, errorList.size());
        } catch (Exception e) {
            fail(e.getMessage());
        }
    }

Perhaps you're expecting it to be notified to the fatalError() callback?

#6 Updated by Marek Skorek 12 months ago

We are not using ErrorListener directly, instead we use our implementation of ErrorHandler wrapped in SchemaFactoryImpl.ErrorListenerWrappingErrorHandler, so you are right that ErrorListener is notified about validation error, but this particular implementation doesn't proppagate this error to our ErrorHandler.

For some reason incoming ValidationException is marked as hasBeenReported=true (in our case this behaviour is the same regardless of library version), but in new library version there is additional check in ErrorListenerWrappingErrorHandler.error method

if (exception instanceof XPathException) {
    if (((XPathException)exception).hasBeenReported()) {
        return;
    }

 

    ((XPathException)exception).setHasBeenReported(true);
}

preventing ErrorHandler invocation.

#7 Updated by Michael Kay 12 months ago

  • Subject changed from Unique key validation error which was fixed in 9.7.21 is alive in 9.9 again to Schema validation errors not reported to ErrorListener

Looks as if you've identified the problem, now I just have to work out a fix.

#8 Updated by Marek Skorek 11 months ago

Hi Micheal, thats the last bug which prevents us from upgrading from 9.7 to 9.9. Do you have a plan to fix this? If so, when?

#9 Updated by Marek Skorek 6 months ago

Do you have a plan to fix this issue? If yes then when?

#10 Updated by Michael Kay 6 months ago

It will be fixed in the next maintenance release. Current status is that we found a fix but it caused regressions, and this revealed that we needed more tests in this area.

#11 Updated by Marek Skorek 6 months ago

Thanks. We are looking forward to it ;)

#12 Updated by Michael Kay 6 months ago

Sorry this one has taken so long. Various attempts at fixing what looks like a very simple problem got tied up in complexities involving other bugs and configuration issues.

The basic problem (as you identify) is that for some errors -- specifically, errors in integrity constraints -- the flag "hasBeenNotified" is set on an error before notifying the error, and some error listeners look at this flag and interpret it as a signal to ignore the error.

The basic problem is still there in 10.x, but I haven't been able to find a combination of circumstances where it matters. However, moving the setting of the flag until after the signalling of the error seems to make sense, and clears the problem on the 9.9 branch.

I've looked again at the apparent regressions caused by the change, and have decided they were spurious. The failure that worried me was in a test for bug #4507. This test appears in the unit tests for 9.9 but appears not to have been migrated to 10.x (probably because the data file is 500Mb, which is a bit heavy for a permanent regression test). After careful analysis I have decided that the expected results were incorrect and the code is actually behaving correctly.

So I think we can deem this fixed; the only problem now is the logistics of delivering a fix. Ordinarily, we would no longer be producing new maintenance releases on the 9.9 branch if there's any chance of a workaround (because the code is now very stable, and any changes risk destabilisation). We're about to ship 10.5, and I'd encourage you to move forward onto the 10.x branch. Alternatively, I think there's a very simple workaround for 9.9: instead of using SchemaFactoryImpl.ErrorListenerWrappingErrorHandler as your ErrorListener, make a copy (or subclass) that ignores the "hasBeenReported()" flag:

public void error(TransformerException exception) throws TransformerException {
            try {
>                if (exception instanceof XPathException) {
>                    if (((XPathException) exception).hasBeenReported()) {
>                        return;
>                    }
>                    ((XPathException) exception).setHasBeenReported(true);
>              }
                handler.error(toSAXException(exception));
            } catch (SAXException e) {
                throw toTransformerException(e);
            }
        }

On the 10.x branch we have deprecated SchemaValidator.setErrorListener() because the JAXP ErrorListener (and the ErrorHandler, for that matter) require creation of an exception object for each validation error reported, which is very expensive. However, it remains available. The ErrorListenerWrappingErrorHandler is gone, but you can easily provide your own, or you can move forward to the new InvalidityHandlerWrappingErrorHandler which replaces it.

#13 Updated by Michael Kay 6 months ago

  • Fix Committed on Branch 10, trunk added

#14 Updated by Michael Kay 6 months ago

  • Fix Committed on Branch 9.9 added

#15 Updated by Eryk Rze┼║nik 6 months ago

Is there a reason why InvalidityHandlerWrappingErrorHandler is not a static class? (At least in version 10.3)

#16 Updated by Michael Kay 6 months ago

No, no reason. In fact I've already made this change for the next maintenance release.

Please register to edit this issue

Also available in: Atom PDF