From d181d8acbf49e2fe87c8cf53a9431e503ccced55 Mon Sep 17 00:00:00 2001 From: Fyodor Yemelyanenko Date: Mon, 21 Aug 2017 16:11:14 +1000 Subject: tdf#84237 use XErrorHandler in CDocumentBuilder In documentbuilder.cxx added code to call XErrorHandler::warning and XErrorHandler::error functions (from DOM::warning_func and DOM::error_func) In domtest.cxx added try {} catch () block to BasicTest::validInputTest, BasicTest::warningInputTest and BasicTest::errorInputTest and to SerializerTest::serializerTest. Also uncommented lines CPPUNIT_TEST(warningInputTest); and CPPUNIT_TEST(errorInputTest); Unit tests are now working (FatalError test removed, as lib2xml doesn't distinguish between error and fatal error and counts everything as error). Change-Id: I27c5036df6a1cc5bef5dbb8171c201d81bae2ccd Reviewed-on: https://gerrit.libreoffice.org/41376 Reviewed-by: Michael Stahl Tested-by: Michael Stahl --- unoxml/qa/unit/domtest.cxx | 161 +++++++++++++++++++--------------- unoxml/source/dom/documentbuilder.cxx | 68 ++++++++++++-- unoxml/source/dom/documentbuilder.hxx | 11 +++ 3 files changed, 162 insertions(+), 78 deletions(-) (limited to 'unoxml') diff --git a/unoxml/qa/unit/domtest.cxx b/unoxml/qa/unit/domtest.cxx index 6080020e7647..18e37a8fa297 100644 --- a/unoxml/qa/unit/domtest.cxx +++ b/unoxml/qa/unit/domtest.cxx @@ -37,6 +37,7 @@ #include #include #include +#include using namespace ::comphelper; using namespace ::com::sun::star; @@ -61,16 +62,20 @@ static const char validTestFile[] = \ "; -// generates a warning: unsupported xml version, unknown xml:space +// generates a warning: unknown xml:space // value static const char warningTestFile[] = -" \ +" \ \ \ - \ + \ + \ + \ + some text \303\266\303\244\303\274 \ \ "; @@ -85,19 +90,17 @@ static const char errorTestFile[] = \ "; -// plain empty -static const char fatalTestFile[] = ""; - struct ErrorHandler : public ::cppu::WeakImplHelper< xml::sax::XErrorHandler > { sal_uInt32 mnErrCount; - sal_uInt32 mnFatalCount; +// sal_uInt32 mnFatalCount; // No fatal error counter, as lib2xml doesn't distinguish between error and fatal error + // (see See xmlFatalErrMsg from lib2xml/parse.c and __xmlRaiseError from lib2xml/error.c) sal_uInt32 mnWarnCount; - bool noErrors() const { return !mnErrCount && !mnFatalCount && !mnWarnCount; } + bool noErrors() const { return !mnErrCount /*&& !mnFatalCount*/ && !mnWarnCount; } - ErrorHandler() : mnErrCount(0), mnFatalCount(0), mnWarnCount(0) + ErrorHandler() : mnErrCount(0), /*mnFatalCount(0),*/ mnWarnCount(0) {} virtual void SAL_CALL error( const uno::Any& ) override @@ -105,9 +108,11 @@ struct ErrorHandler ++mnErrCount; } + // Just implement FatalError function as it is in XErrorHandler + // This function is never used, as lib2xml doesn't distinguish between error and fatalerror and calls error functions in both cases virtual void SAL_CALL fatalError( const uno::Any& ) override { - ++mnFatalCount; + //++mnFatalCount; } virtual void SAL_CALL warning( const uno::Any& ) override @@ -195,7 +200,6 @@ struct BasicTest : public test::BootstrapFixture rtl::Reference mxValidInStream; rtl::Reference mxWarningInStream; rtl::Reference mxErrorInStream; - rtl::Reference mxFatalInStream; virtual void setUp() override { @@ -207,58 +211,70 @@ struct BasicTest : public test::BootstrapFixture mxValidInStream.set( new SequenceInputStream(css::uno::Sequence(reinterpret_cast(validTestFile), SAL_N_ELEMENTS(validTestFile))) ); mxWarningInStream.set( new SequenceInputStream(css::uno::Sequence(reinterpret_cast(warningTestFile), SAL_N_ELEMENTS(warningTestFile))) ); mxErrorInStream.set( new SequenceInputStream(css::uno::Sequence(reinterpret_cast(errorTestFile), SAL_N_ELEMENTS(errorTestFile))) ); - mxFatalInStream.set( new SequenceInputStream(css::uno::Sequence(reinterpret_cast(fatalTestFile), SAL_N_ELEMENTS(fatalTestFile))) ); mxDomBuilder->setErrorHandler(mxErrHandler.get()); } void validInputTest() { - CPPUNIT_ASSERT_MESSAGE( "Valid input file did not result in XDocument #1", - mxDomBuilder->parse( - uno::Reference( - mxValidInStream.get())).is() ); - CPPUNIT_ASSERT_MESSAGE( "Valid input file resulted in parse errors", - mxErrHandler->noErrors() ); + try + { + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument #1", + mxDomBuilder->parse( + uno::Reference( + mxValidInStream.get())).is()); + CPPUNIT_ASSERT_MESSAGE("Valid input file resulted in parse errors", + mxErrHandler->noErrors()); + } + catch (const css::xml::sax::SAXParseException&) + { + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument (exception thrown)", false); + } } -/* + void warningInputTest() { - CPPUNIT_ASSERT_MESSAGE( "Valid input file did not result in XDocument #2", - mxDomBuilder->parse( - uno::Reference( - mxWarningInStream.get())).is() ); - CPPUNIT_ASSERT_MESSAGE( "No parse warnings in unclean input file", - mxErrHandler->mnWarnCount && !mxErrHandler->mnErrCount && !mxErrHandler->mnFatalCount ); + try + { + // We DONT expect exeption here, as mxWarningInStream is valid XML Doc + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument #2", + mxDomBuilder->parse( + uno::Reference( + mxWarningInStream.get())).is()); + } + catch (const css::xml::sax::SAXParseException& ) + { + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument #2 (exception thrown)", false); + } + CPPUNIT_ASSERT_MESSAGE("No parse warnings in unclean input file", + mxErrHandler->mnWarnCount && !mxErrHandler->mnErrCount /*&& !mxErrHandler->mnFatalCount*/); } void errorInputTest() { - CPPUNIT_ASSERT_MESSAGE( "Valid input file did not result in XDocument #3", - mxDomBuilder->parse( - uno::Reference( - mxErrorInStream.get())).is() ); - CPPUNIT_ASSERT_MESSAGE( "No parse errors in unclean input file", - !mxErrHandler->mnWarnCount && mxErrHandler->mnErrCount && !mxErrHandler->mnFatalCount ); + try + { + // We expect exeption here, as mxErrorInStream is invalid XML Doc + CPPUNIT_ASSERT_MESSAGE("Invalid input file result in XDocument #2!", + !mxDomBuilder->parse( + uno::Reference( + mxErrorInStream.get())).is()); + CPPUNIT_ASSERT_MESSAGE("No exception is thrown in unclean input file", false); + } + catch (const css::xml::sax::SAXParseException&) + { + // It's OK to catch an exeption here as we parse incorrect XML file + } + CPPUNIT_ASSERT_MESSAGE("No parse errors in unclean input file", + !mxErrHandler->mnWarnCount && mxErrHandler->mnErrCount /*&& !mxErrHandler->mnFatalCount*/); } - void fatalInputTest() - { - CPPUNIT_ASSERT_MESSAGE( "Broken input file resulted in XDocument", - !mxDomBuilder->parse( - uno::Reference( - mxFatalInStream.get())).is() ); - CPPUNIT_ASSERT_MESSAGE( "No fatal parse errors in unclean input file", - !mxErrHandler->mnWarnCount && !mxErrHandler->mnErrCount && mxErrHandler->mnFatalCount ); - }; -*/ - // Change the following lines only, if you add, remove or rename + // Change the following lines only, if you add, remove or rename // member functions of the current class, // because these macros are need by auto register mechanism. CPPUNIT_TEST_SUITE(BasicTest); CPPUNIT_TEST(validInputTest); - //CPPUNIT_TEST(warningInputTest); - //CPPUNIT_TEST(errorInputTest); - //CPPUNIT_TEST(fatalInputTest); + CPPUNIT_TEST(warningInputTest); + CPPUNIT_TEST(errorInputTest); CPPUNIT_TEST_SUITE_END(); }; @@ -294,29 +310,36 @@ struct SerializerTest : public test::BootstrapFixture void serializerTest () { - uno::Reference< xml::dom::XDocument > xDoc= - mxDomBuilder->parse( - uno::Reference( - mxInStream.get())); - CPPUNIT_ASSERT_MESSAGE( "Valid input file did not result in XDocument", - xDoc.is() ); - CPPUNIT_ASSERT_MESSAGE( "Valid input file resulted in parse errors", - mxErrHandler->noErrors() ); - - uno::Reference< xml::sax::XSAXSerializable > xSaxSerializer( - xDoc, uno::UNO_QUERY); - CPPUNIT_ASSERT_MESSAGE( "XSAXSerializable not supported", - xSaxSerializer.is() ); - - uno::Reference< xml::sax::XFastSAXSerializable > xFastSaxSerializer( - xDoc, uno::UNO_QUERY); - CPPUNIT_ASSERT_MESSAGE( "XFastSAXSerializable not supported", - xSaxSerializer.is() ); - - xFastSaxSerializer->fastSerialize( mxHandler.get(), - mxTokHandler.get(), - uno::Sequence< beans::StringPair >(), - maRegisteredNamespaces ); + try + { + uno::Reference< xml::dom::XDocument > xDoc = + mxDomBuilder->parse( + uno::Reference( + mxInStream.get())); + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument", + xDoc.is()); + CPPUNIT_ASSERT_MESSAGE("Valid input file resulted in parse errors", + mxErrHandler->noErrors()); + + uno::Reference< xml::sax::XSAXSerializable > xSaxSerializer( + xDoc, uno::UNO_QUERY); + CPPUNIT_ASSERT_MESSAGE("XSAXSerializable not supported", + xSaxSerializer.is()); + + uno::Reference< xml::sax::XFastSAXSerializable > xFastSaxSerializer( + xDoc, uno::UNO_QUERY); + CPPUNIT_ASSERT_MESSAGE("XFastSAXSerializable not supported", + xSaxSerializer.is()); + + xFastSaxSerializer->fastSerialize(mxHandler.get(), + mxTokHandler.get(), + uno::Sequence< beans::StringPair >(), + maRegisteredNamespaces); + } + catch (const css::xml::sax::SAXParseException&) + { + CPPUNIT_ASSERT_MESSAGE("Valid input file did not result in XDocument (exception thrown)", false); + } } // Change the following lines only, if you add, remove or rename diff --git a/unoxml/source/dom/documentbuilder.cxx b/unoxml/source/dom/documentbuilder.cxx index 5f3530395bfe..e8eb6097c23d 100644 --- a/unoxml/source/dom/documentbuilder.cxx +++ b/unoxml/source/dom/documentbuilder.cxx @@ -269,21 +269,71 @@ namespace DOM // default warning handler does not trigger assertion static void warning_func(void * ctx, const char * /*msg*/, ...) { - SAL_INFO( - "unoxml", - "libxml2 warning: " - << make_error_message(static_cast(ctx))); + try + { + xmlParserCtxtPtr const pctx = static_cast(ctx); + + SAL_INFO( + "unoxml", + "libxml2 warning: " + << make_error_message(pctx)); + + CDocumentBuilder * const pDocBuilder = static_cast(pctx->_private); + + if (pDocBuilder->getErrorHandler().is()) // if custom error handler is set (using setErrorHandler ()) + { + // Prepare SAXParseException to be passed to custom XErrorHandler::warning function + css::xml::sax::SAXParseException saxex; + saxex.Message = make_error_message(pctx); + saxex.LineNumber = static_cast(pctx->lastError.line); + saxex.ColumnNumber = static_cast(pctx->lastError.int2); + + // Call custom warning function + pDocBuilder->getErrorHandler()->warning(::css::uno::Any(saxex)); + } + } + catch (const css::uno::RuntimeException &e) + { + // Protect lib2xml from UNO Exception + SAL_WARN("unoxml", + "DOM::warning_func: caught RuntimeException" + << e.Message); + } } // default error handler triggers assertion static void error_func(void * ctx, const char * /*msg*/, ...) { - SAL_WARN( - "unoxml", - "libxml2 error: " - << make_error_message(static_cast(ctx))); + try + { + xmlParserCtxtPtr const pctx = static_cast(ctx); + SAL_WARN( + "unoxml", + "libxml2 error: " + << make_error_message(pctx)); + + CDocumentBuilder * const pDocBuilder = static_cast(pctx->_private); + + if (pDocBuilder->getErrorHandler().is()) // if custom error handler is set (using setErrorHandler ()) + { + // Prepare SAXParseException to be passed to custom XErrorHandler::error function + css::xml::sax::SAXParseException saxex; + saxex.Message = make_error_message(pctx); + saxex.LineNumber = static_cast(pctx->lastError.line); + saxex.ColumnNumber = static_cast(pctx->lastError.int2); + + // Call custom warning function + pDocBuilder->getErrorHandler()->error(::css::uno::Any(saxex)); + } + } + catch (const css::uno::RuntimeException &e) + { + // Protect lib2xml from UNO Exception + SAL_WARN("unoxml", + "DOM::error_func: caught RuntimeException" + << e.Message); + } } - } // extern "C" void throwEx(xmlParserCtxtPtr ctxt) diff --git a/unoxml/source/dom/documentbuilder.hxx b/unoxml/source/dom/documentbuilder.hxx index cdda893010d5..9f6253bdff8a 100644 --- a/unoxml/source/dom/documentbuilder.hxx +++ b/unoxml/source/dom/documentbuilder.hxx @@ -123,6 +123,17 @@ namespace DOM the XML document to be parsed. */ virtual void SAL_CALL setErrorHandler(const css::uno::Reference< css::xml::sax::XErrorHandler >& eh) override; + + /* + Get the ErrorHandler to be used to report errors present in + the XML document to be parsed. + */ + + const css::uno::Reference< css::xml::sax::XErrorHandler >& getErrorHandler() + { + return m_xErrorHandler; + } + }; } -- cgit