Merge pull request #2373 from ric2b/2372-fix-builder-not-handling-exc…

…eptions-safely Fix XML::Builder not handling exceptions safely - 2372
sparklemotion · Nov 19, 2021 · 820f99d · 820f99d
2 parents 44efbc3 + be4a198
commit 820f99d
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@ A related discussion about Trust exists at [#2357](https://github.com/sparklemot
 
 ### Fixed
 
+* XML::Builder blocks restore context properly when exceptions are raised. [[#2372](https://github.com/sparklemotion/nokogiri/issues/2372)] (Thanks, [@ric2b](https://github.com/ric2b) and [@rinthedev](https://github.com/rinthedev)!)
 * [CRuby] Fix memory leak in `Document#canonicalize` when inclusive namespaces are passed in. [[#2345](https://github.com/sparklemotion/nokogiri/issues/2345)]
 * [CRuby] Fix memory leak in `Document#canonicalize` when an argument type error is raised. [[#2345](https://github.com/sparklemotion/nokogiri/issues/2345)]
 * [CRuby] Fix memory leak in `EncodingHandler` where iconv handlers were not being cleaned up. [[#2345](https://github.com/sparklemotion/nokogiri/issues/2345)]

diff --git a/lib/nokogiri/xml/builder.rb b/lib/nokogiri/xml/builder.rb
@@ -425,15 +425,18 @@ def method_missing(method, *args, &block) # :nodoc:
       def insert(node, &block)
         node = @parent.add_child(node)
         if block
-          old_parent = @parent
-          @parent = node
-          @arity ||= block.arity
-          if @arity <= 0
-            instance_eval(&block)
-          else
-            yield(self)
+          begin
+            old_parent = @parent
+            @parent = node
+            @arity ||= block.arity
+            if @arity <= 0
+              instance_eval(&block)
+            else
+              yield(self)
+            end
+          ensure
+            @parent = old_parent
           end
-          @parent = old_parent
         end
         NodeBuilder.new(node, self)
       end

diff --git a/test/xml/test_builder.rb b/test/xml/test_builder.rb
@@ -22,6 +22,29 @@ def test_builder_multiple_nodes
         end
       end
 
+      def test_builder_resilient_to_exceptions
+        builder = Nokogiri::XML::Builder.new do |xml|
+          xml.root do
+            begin
+              xml.a { raise "badjoras" }
+            rescue StandardError
+              # Ignored
+            end
+
+            xml.b
+          end
+        end
+
+        expected_output = <<~HEREDOC
+          <?xml version="1.0"?>
+          <root>
+            <a/>
+            <b/>
+          </root>
+        HEREDOC
+        assert_equal(expected_output, builder.to_xml)
+      end
+
       def test_builder_with_utf8_text
         text = "test ﺵ "
         doc = Nokogiri::XML::Builder.new(encoding: "UTF-8") { |xml| xml.test(text) }.doc