Merge pull request #28815 from joshnuss/fix/hash-from-xml-nokogiri

Fixes Hash.from_xml with frozen strings for all engines
rails · Apr 20, 2017 · f626b47 · f626b47
1 parent ea11acd
commit f626b47
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 13 deletions.
diff --git a/activesupport/lib/active_support/xml_mini/libxml.rb b/activesupport/lib/active_support/xml_mini/libxml.rb
@@ -14,11 +14,9 @@ def parse(data)
         data = StringIO.new(data || '')
       end
 
-      char = data.getc
-      if char.nil?
+      if data.eof?
         {}
       else
-        data.ungetc(char)
         LibXML::XML::Parser.io(data).parse.to_hash
       end
     end

diff --git a/activesupport/lib/active_support/xml_mini/libxmlsax.rb b/activesupport/lib/active_support/xml_mini/libxmlsax.rb
@@ -66,12 +66,9 @@ def parse(data)
         data = StringIO.new(data || '')
       end
 
-      char = data.getc
-      if char.nil?
+      if data.eof?
         {}
       else
-        data.ungetc(char)
-
         LibXML::XML::Error.set_handler(&LibXML::XML::Error::QUIET_HANDLER)
         parser = LibXML::XML::SaxParser.io(data)
         document = self.document_class.new

diff --git a/activesupport/lib/active_support/xml_mini/nokogiri.rb b/activesupport/lib/active_support/xml_mini/nokogiri.rb
@@ -19,11 +19,9 @@ def parse(data)
         data = StringIO.new(data || '')
       end
 
-      char = data.getc
-      if char.nil?
+      if data.eof?
         {}
       else
-        data.ungetc(char)
         doc = Nokogiri::XML(data)
         raise doc.errors.first if doc.errors.length > 0
         doc.to_hash

diff --git a/activesupport/lib/active_support/xml_mini/nokogirisax.rb b/activesupport/lib/active_support/xml_mini/nokogirisax.rb
@@ -72,11 +72,9 @@ def parse(data)
         data = StringIO.new(data || '')
       end
 
-      char = data.getc
-      if char.nil?
+      if data.eof?
         {}
       else
-        data.ungetc(char)
         document = self.document_class.new
         parser = Nokogiri::XML::SAX::Parser.new(document)
         parser.parse(data)

diff --git a/activesupport/test/xml_mini/jdom_engine_test.rb b/activesupport/test/xml_mini/jdom_engine_test.rb
@@ -175,6 +175,11 @@ def test_children_with_non_adjacent_text
       eoxml
     end
 
+    def test_parse_from_frozen_string
+      xml_string = "<root/>".freeze
+      assert_equal({ "root" => {} }, ActiveSupport::XmlMini.parse(xml_string))
+    end
+
     private
       def assert_equal_rexml(xml)
         parsed_xml = XmlMini.parse(xml)

diff --git a/activesupport/test/xml_mini/libxml_engine_test.rb b/activesupport/test/xml_mini/libxml_engine_test.rb
@@ -191,6 +191,10 @@ def test_children_with_blank_text_and_attribute
     eoxml
   end
 
+  def test_parse_from_frozen_string
+    xml_string = "<root/>".freeze
+    assert_equal({ "root" => {} }, ActiveSupport::XmlMini.parse(xml_string))
+  end
 
   private
     def assert_equal_rexml(xml)

diff --git a/activesupport/test/xml_mini/libxmlsax_engine_test.rb b/activesupport/test/xml_mini/libxmlsax_engine_test.rb
@@ -183,6 +183,11 @@ def test_children_with_blank_text
     eoxml
   end
 
+  def test_parse_from_frozen_string
+    xml_string = "<root/>".freeze
+    assert_equal({ "root" => {} }, ActiveSupport::XmlMini.parse(xml_string))
+  end
+
   private
     def assert_equal_rexml(xml)
       parsed_xml = XmlMini.parse(xml)

diff --git a/activesupport/test/xml_mini/nokogiri_engine_test.rb b/activesupport/test/xml_mini/nokogiri_engine_test.rb
@@ -205,6 +205,11 @@ def test_children_with_blank_text_and_attribute
     eoxml
   end
 
+  def test_parse_from_frozen_string
+    xml_string = "<root/>".freeze
+    assert_equal({ "root" => {} }, ActiveSupport::XmlMini.parse(xml_string))
+  end
+
   private
     def assert_equal_rexml(xml)
       parsed_xml = XmlMini.parse(xml)

diff --git a/activesupport/test/xml_mini/nokogirisax_engine_test.rb b/activesupport/test/xml_mini/nokogirisax_engine_test.rb
@@ -206,6 +206,11 @@ def test_children_with_blank_text_and_attribute
     eoxml
   end
 
+  def test_parse_from_frozen_string
+    xml_string = "<root/>".freeze
+    assert_equal({ "root" => {} }, ActiveSupport::XmlMini.parse(xml_string))
+  end
+
   private
     def assert_equal_rexml(xml)
       parsed_xml = XmlMini.parse(xml)