From b5bf109a599ea733663150e99c09eb44046b41dd Mon Sep 17 00:00:00 2001
From: Hiroya Fujinami <make.just.on@gmail.com>
Date: Thu, 13 Jun 2024 15:12:32 +0900
Subject: [PATCH] Add a "malformed comment" check for top-level comments (#145)

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

This PR also adds tests for "malformed comment" checks.

---------

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
---
 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..eae0db8b 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 /--|-\z/.match?(md[1])
+                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