From 175d125119524629c2c73b9ac1d954ac3128b538 Mon Sep 17 00:00:00 2001
From: TSUYUSATO Kitsune <make.just.on@gmail.com>
Date: Wed, 12 Jun 2024 14:02:30 +0900
Subject: [PATCH 1/2] Add a "malformed comment" check for top-level comments

This check was missing. Therefore, `Document.new("<!--")`
raises the `undefined method `[]' for nil` error, for example.

This also adds tests for "malformed comment" checks.
---
 lib/rexml/parsers/baseparser.rb |  9 +++-
 test/parse/test_comment.rb      | 96 +++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 test/parse/test_comment.rb

diff --git a/lib/rexml/parsers/baseparser.rb b/lib/rexml/parsers/baseparser.rb
index eadc78f7..5e824cce 100644
--- a/lib/rexml/parsers/baseparser.rb
+++ b/lib/rexml/parsers/baseparser.rb
@@ -228,7 +228,14 @@ def pull_event
             return process_instruction(start_position)
           elsif @source.match("<!", true)
             if @source.match("--", true)
-              return [ :comment, @source.match(/(.*?)-->/um, true)[1] ]
+              md = @source.match(/(.*?)-->/um, true)
+              if md.nil?
+                raise REXML::ParseException.new("Unclosed comment", @source)
+              end
+              if md[1] =~ /--|-\z/
+                raise REXML::ParseException.new("Malformed comment", @source)
+              end
+              return [ :comment, md[1] ]
             elsif @source.match("DOCTYPE", true)
               base_error_message = "Malformed DOCTYPE"
               unless @source.match(/\s+/um, true)
diff --git a/test/parse/test_comment.rb b/test/parse/test_comment.rb
new file mode 100644
index 00000000..8f143495
--- /dev/null
+++ b/test/parse/test_comment.rb
@@ -0,0 +1,96 @@
+require "test/unit"
+require "rexml/document"
+
+module REXMLTests
+  class TestParseComment < Test::Unit::TestCase
+    def parse(xml)
+      REXML::Document.new(xml)
+    end
+
+    class TestInvalid < self
+      def test_toplevel_unclosed_comment
+        exception = assert_raise(REXML::ParseException) do
+          parse("<!--")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Unclosed comment
+          Line: 1
+          Position: 4
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_toplevel_malformed_comment_inner
+        exception = assert_raise(REXML::ParseException) do
+          parse("<!-- -- -->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 11
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_toplevel_malformed_comment_end
+        exception = assert_raise(REXML::ParseException) do
+          parse("<!-- --->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 9
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_doctype_malformed_comment_inner
+        exception = assert_raise(REXML::ParseException) do
+          parse("<!DOCTYPE foo [<!-- -- -->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 26
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_doctype_malformed_comment_end
+        exception = assert_raise(REXML::ParseException) do
+          parse("<!DOCTYPE foo [<!-- --->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 24
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_after_doctype_malformed_comment_inner
+        exception = assert_raise(REXML::ParseException) do
+          parse("<a><!-- -- -->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 14
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+
+      def test_after_doctype_malformed_comment_end
+        exception = assert_raise(REXML::ParseException) do
+          parse("<a><!-- --->")
+        end
+        assert_equal(<<~DETAIL, exception.to_s)
+          Malformed comment
+          Line: 1
+          Position: 12
+          Last 80 unconsumed characters:
+        DETAIL
+      end
+    end
+  end
+end

From 870c2755dccd4f6d8b4bbe89f7c744a787aac3d5 Mon Sep 17 00:00:00 2001
From: Hiroya Fujinami <make.just.on@gmail.com>
Date: Wed, 12 Jun 2024 14:46:37 +0900
Subject: [PATCH 2/2] Update lib/rexml/parsers/baseparser.rb

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
---
 lib/rexml/parsers/baseparser.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rexml/parsers/baseparser.rb b/lib/rexml/parsers/baseparser.rb
index 5e824cce..eae0db8b 100644
--- a/lib/rexml/parsers/baseparser.rb
+++ b/lib/rexml/parsers/baseparser.rb
@@ -232,7 +232,7 @@ def pull_event
               if md.nil?
                 raise REXML::ParseException.new("Unclosed comment", @source)
               end
-              if md[1] =~ /--|-\z/
+              if /--|-\z/.match?(md[1])
                 raise REXML::ParseException.new("Malformed comment", @source)
               end
               return [ :comment, md[1] ]