Fixed some bugs and fixed some tests in new SAX-based XmlMini backends.

[#3636 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

5 files changed, 20 insertions, 17 deletions

diff --git a/activesupport/lib/active_support/xml_mini/libxmlsax.rb b/activesupport/lib/active_support/xml_mini/libxmlsax.rb
index dba1f8755a..d7b2f4c5be 100644
--- a/activesupport/lib/active_support/xml_mini/libxmlsax.rb
+++ b/activesupport/lib/active_support/xml_mini/libxmlsax.rb
@@ -21,16 +21,13 @@ module ActiveSupport
       end
 
       def on_start_document
-        @hash = {}
+        @hash = { CONTENT_KEY => '' }
         @hash_stack = [@hash]
       end
 
       def on_end_document
-        raise "Parse stack not empty!" if @hash_stack.size > 1
-      end
-
-      def on_error(error_message)
-        raise LibXML::XML::Error, error_message
+        @hash = @hash_stack.pop
+        @hash.delete(CONTENT_KEY)
       end
 
       def on_start_element(name, attrs = {})
@@ -73,8 +70,11 @@ module ActiveSupport
         {}
       else
         data.ungetc(char)
-        document = self.document_class.new
+
+        LibXML::XML::Error.set_handler(&LibXML::XML::Error::QUIET_HANDLER)
         parser = LibXML::XML::SaxParser.io(data)
+        document = self.document_class.new
+
         parser.callbacks = document
         parser.parse
         document.hash
diff --git a/activesupport/lib/active_support/xml_mini/nokogirisax.rb b/activesupport/lib/active_support/xml_mini/nokogirisax.rb
index 42a44897ba..d538a9110f 100644
--- a/activesupport/lib/active_support/xml_mini/nokogirisax.rb
+++ b/activesupport/lib/active_support/xml_mini/nokogirisax.rb
@@ -28,7 +28,7 @@ module ActiveSupport
       end
 
       def error(error_message)
-        raise Nokogiri::XML::SyntaxError, error_message
+        raise error_message
       end
 
       def start_element(name, attrs = [])
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index 4642bb1330..5b1d53ac7b 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -902,9 +902,11 @@ class HashToXmlTest < Test::Unit::TestCase
 
   def test_expansion_count_is_limited
     expected = {
-      'ActiveSupport::XmlMini_REXML'    => 'RuntimeError',
-      'ActiveSupport::XmlMini_Nokogiri' => 'Nokogiri::XML::SyntaxError',
-      'ActiveSupport::XmlMini_LibXML'   => 'LibXML::XML::Error',
+      'ActiveSupport::XmlMini_REXML'       => 'RuntimeError',
+      'ActiveSupport::XmlMini_Nokogiri'    => 'Nokogiri::XML::SyntaxError',
+      'ActiveSupport::XmlMini_NokogiriSAX' => 'RuntimeError',
+      'ActiveSupport::XmlMini_LibXML'      => 'LibXML::XML::Error',
+      'ActiveSupport::XmlMini_LibXMLSAX'   => 'LibXML::XML::Error',
     }[ActiveSupport::XmlMini.backend.name].constantize
 
     assert_raise expected do
diff --git a/activesupport/test/xml_mini/libxmlsax_engine_test.rb b/activesupport/test/xml_mini/libxmlsax_engine_test.rb
index 6d5b3673fa..864810099e 100644
--- a/activesupport/test/xml_mini/libxmlsax_engine_test.rb
+++ b/activesupport/test/xml_mini/libxmlsax_engine_test.rb
@@ -14,8 +14,6 @@ class LibXMLSAXEngineTest < Test::Unit::TestCase
   def setup
     @default_backend = XmlMini.backend
     XmlMini.backend = 'LibXMLSAX'
-
-    LibXML::XML::Error.set_handler(&lambda { |error| }) #silence libxml, exceptions will do
   end
 
   def teardown
@@ -24,7 +22,8 @@ class LibXMLSAXEngineTest < Test::Unit::TestCase
 
   def test_exception_thrown_on_expansion_attack
     assert_raise LibXML::XML::Error do
-      attack_xml = %{<?xml version="1.0" encoding="UTF-8"?>
+      attack_xml = <<-EOT
+      <?xml version="1.0" encoding="UTF-8"?>
       <!DOCTYPE member [
         <!ENTITY a "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;">
         <!ENTITY b "&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;">
@@ -37,7 +36,8 @@ class LibXMLSAXEngineTest < Test::Unit::TestCase
       <member>
       &a;
       </member>
-     }
+      EOT
+
       Hash.from_xml(attack_xml)
     end
   end
diff --git a/activesupport/test/xml_mini/nokogirisax_engine_test.rb b/activesupport/test/xml_mini/nokogirisax_engine_test.rb
index 43f1cda0e0..1149d0fecc 100644
--- a/activesupport/test/xml_mini/nokogirisax_engine_test.rb
+++ b/activesupport/test/xml_mini/nokogirisax_engine_test.rb
@@ -8,7 +8,7 @@ rescue LoadError
   # Skip nokogiri tests
 else
 
-class NokogiriEngineTest < Test::Unit::TestCase
+class NokogiriSAXEngineTest < Test::Unit::TestCase
   include ActiveSupport
 
   def setup
@@ -36,7 +36,7 @@ class NokogiriEngineTest < Test::Unit::TestCase
   end
 
   def test_exception_thrown_on_expansion_attack
-    assert_raise Nokogiri::XML::SyntaxError do
+    assert_raise RuntimeError do
       attack_xml = <<-EOT
       <?xml version="1.0" encoding="UTF-8"?>
       <!DOCTYPE member [
@@ -52,6 +52,7 @@ class NokogiriEngineTest < Test::Unit::TestCase
       &a;
       </member>
       EOT
+
       Hash.from_xml(attack_xml)
     end
   end