From 0de3d1cdb7e6f2c8b4732b7b624306fd11895ab1 Mon Sep 17 00:00:00 2001 From: Adam Constabaris Date: Fri, 8 Feb 2019 15:23:15 -0500 Subject: [PATCH 1/3] Rethrow exceptions caught by Java SAX handler --- .../nokogiri/internals/NokogiriHandler.java | 13 +++-- test/xml/sax/test_document_error.rb | 55 +++++++++++++++++++ test/xml/sax/test_push_parser.rb | 1 + 3 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 test/xml/sax/test_document_error.rb diff --git a/ext/java/nokogiri/internals/NokogiriHandler.java b/ext/java/nokogiri/internals/NokogiriHandler.java index dd3228585e..bfcce47fba 100644 --- a/ext/java/nokogiri/internals/NokogiriHandler.java +++ b/ext/java/nokogiri/internals/NokogiriHandler.java @@ -252,8 +252,9 @@ public void error(SAXParseException ex) { final String msg = ex.getMessage(); call("error", runtime.newString(msg == null ? "" : msg)); addError(new RaiseException(XmlSyntaxError.createError(runtime, ex), true)); - } catch(RaiseException e) { - addError(e); + } catch( RaiseException rx ) { + addError(rx); + throw rx; } } @@ -263,9 +264,9 @@ public void fatalError(SAXParseException ex) { final String msg = ex.getMessage(); call("error", runtime.newString(msg == null ? "" : msg)); addError(new RaiseException(XmlSyntaxError.createFatalError(runtime, ex), true)); - - } catch(RaiseException e) { - addError(e); + } catch( RaiseException rx ) { + addError(rx); + throw rx; } } @@ -275,7 +276,7 @@ public void warning(SAXParseException ex) { call("warning", runtime.newString(msg == null ? "" : msg)); } - protected synchronized void addError(RaiseException e) { + public synchronized void addError(RaiseException e) { errors.add(e); } diff --git a/test/xml/sax/test_document_error.rb b/test/xml/sax/test_document_error.rb new file mode 100644 index 0000000000..79509232ac --- /dev/null +++ b/test/xml/sax/test_document_error.rb @@ -0,0 +1,55 @@ +require 'helper' + +module Nokogiri + module XML + module SAX + + # raises an exception when underlying parser + # encounters an XML parsing error + class ThrowingErrorDocument < Document + def error(msg) + raise(StandardError, "parsing did not complete: #{msg}") + end + end + + # only warns when underlying parser encounters + # an XML parsing error + class WarningErrorDocument < Document + def error(msg) + errors << msg + end + + def errors + @errors ||= [] + end + end + + class TestErrorHandling < Nokogiri::SAX::TestCase + def setup + super + @error_parser = Parser.new(ThrowingErrorDocument.new) + @warning_parser = Parser.new(WarningErrorDocument.new) + end + + def test_error_throwing_document_raises_exception + begin + @error_parser.parse("") # no closing element + fail '#parse should not complete successfully when document #error throws exception' + rescue StandardError => e + assert_match /parsing did not complete/, e.message + end + end + + def test_warning_document_encounters_error_but_terminates_normally + begin + @warning_parser.parse("") + assert !@warning_parser.document.errors.empty?, 'error collector did not collect an error' + rescue StandardError => e + warn(e) + fail '#parse should complete successfully unless document #error throws exception (#{e}' + end + end + end + end + end +end diff --git a/test/xml/sax/test_push_parser.rb b/test/xml/sax/test_push_parser.rb index fccb9ffd41..ea185e1d6e 100644 --- a/test/xml/sax/test_push_parser.rb +++ b/test/xml/sax/test_push_parser.rb @@ -73,6 +73,7 @@ def error msg rescue => e actual = e end + fail 'PushParser should throw error when fed ill-formed data' if actual.nil? assert_equal actual.message, "parse error" end From a4615db1f5a83d6e3c53cb34a7656df1fe1f0e27 Mon Sep 17 00:00:00 2001 From: Adam Constabaris Date: Sun, 10 Feb 2019 07:43:17 -0500 Subject: [PATCH 2/3] refactor to make codeclimate happy --- .../nokogiri/internals/NokogiriHandler.java | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/ext/java/nokogiri/internals/NokogiriHandler.java b/ext/java/nokogiri/internals/NokogiriHandler.java index bfcce47fba..87bc7169b8 100644 --- a/ext/java/nokogiri/internals/NokogiriHandler.java +++ b/ext/java/nokogiri/internals/NokogiriHandler.java @@ -246,28 +246,25 @@ public void endCDATA() { charactersBuilder.setLength(0); } - @Override - public void error(SAXParseException ex) { + void handleError(SAXParseException spx) { try { - final String msg = ex.getMessage(); + final String msg = spx.getMessage(); call("error", runtime.newString(msg == null ? "" : msg)); - addError(new RaiseException(XmlSyntaxError.createError(runtime, ex), true)); + addError(new RaiseException(XmlSyntaxError.createError(runtime, spx), true)); } catch( RaiseException rx ) { addError(rx); throw rx; } } + @Override + public void error(SAXParseException ex) { + handleError(ex); + } + @Override public void fatalError(SAXParseException ex) { - try { - final String msg = ex.getMessage(); - call("error", runtime.newString(msg == null ? "" : msg)); - addError(new RaiseException(XmlSyntaxError.createFatalError(runtime, ex), true)); - } catch( RaiseException rx ) { - addError(rx); - throw rx; - } + handleError(ex); } @Override @@ -276,7 +273,7 @@ public void warning(SAXParseException ex) { call("warning", runtime.newString(msg == null ? "" : msg)); } - public synchronized void addError(RaiseException e) { + protected synchronized void addError(RaiseException e) { errors.add(e); } From 042951377082fa5b630b61dbfdd358b31a1f78a3 Mon Sep 17 00:00:00 2001 From: Adam Constabaris Date: Tue, 12 Feb 2019 15:07:30 -0500 Subject: [PATCH 3/3] rename exception vars --- ext/java/nokogiri/internals/NokogiriHandler.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ext/java/nokogiri/internals/NokogiriHandler.java b/ext/java/nokogiri/internals/NokogiriHandler.java index 87bc7169b8..0b24898c1c 100644 --- a/ext/java/nokogiri/internals/NokogiriHandler.java +++ b/ext/java/nokogiri/internals/NokogiriHandler.java @@ -246,14 +246,14 @@ public void endCDATA() { charactersBuilder.setLength(0); } - void handleError(SAXParseException spx) { + void handleError(SAXParseException ex) { try { - final String msg = spx.getMessage(); + final String msg = ex.getMessage(); call("error", runtime.newString(msg == null ? "" : msg)); - addError(new RaiseException(XmlSyntaxError.createError(runtime, spx), true)); - } catch( RaiseException rx ) { - addError(rx); - throw rx; + addError(new RaiseException(XmlSyntaxError.createError(runtime, ex), true)); + } catch( RaiseException e) { + addError(e); + throw e; } }