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 <[email protected]>

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)

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