Added XML declaration check

naitoh · naitoh · commit 5107b028db52 · 2025-08-13T00:07:48.000+09:00
## Why? - The version attribute is required in XML declaration. - Only version attribute, encoding attribute, and standalone attribute are allowed in XML declaration. - XML declaration is only allowed once. See: https://www.w3.org/TR/xml/#NT-XMLDecl
diff --git a/lib/rexml/parsers/baseparser.rb b/lib/rexml/parsers/baseparser.rb
@@ -168,6 +168,7 @@ def initialize( source )
         @entity_expansion_limit = Security.entity_expansion_limit
         @entity_expansion_text_limit = Security.entity_expansion_text_limit
         @source.ensure_buffer
+        @version = nil
       end
 
       def add_listener( listener )
@@ -735,37 +736,93 @@ def process_comment
 
       def process_instruction
         name = parse_name("Malformed XML: Invalid processing instruction node")
-        if @source.skip_spaces?
-          match_data = @source.match(/(.*?)\?>/um, true)
-          unless match_data
-            raise ParseException.new("Malformed XML: Unclosed processing instruction", @source)
+        if name == "xml"
+          xml_declaration
+        else # PITarget
+          if @source.skip_spaces? # e.g. <?name content?>
+            start_position = @source.position
+            content = @source.read_until("?>")
+            unless content.chomp!("?>")
+              @source.position = start_position
+              raise ParseException.new("Malformed XML: Unclosed processing instruction: <#{name}>", @source)
+            end
+          else # e.g. <?name?>
+            content = nil
+            unless @source.match?("?>", true)
+              raise ParseException.new("Malformed XML: Unclosed processing instruction: <#{name}>", @source)
+            end
           end
-          content = match_data[1]
-        else
-          content = nil
+          [:processing_instruction, name, content]
+        end
+      end
+
+      def xml_declaration
+        unless @version.nil?
+          raise ParseException.new("Malformed XML: XML declaration is duplicated", @source)
+        end
+        if @document_status
+          raise ParseException.new("Malformed XML: XML declaration is not at the start", @source)
+        end
+        unless @source.skip_spaces?
+          raise ParseException.new("Malformed XML: XML declaration is missing spaces", @source)
+        end
+        unless @source.match?("version", true)
+          raise ParseException.new("Malformed XML: XML declaration is missing version", @source)
+        end
+        @version = get_attribute("xml")
+        unless @source.skip_spaces?
           unless @source.match?("?>", true)
-            raise ParseException.new("Malformed XML: Unclosed processing instruction", @source)
+            raise ParseException.new("Malformed XML: Unclosed XML declaration", @source)
           end
-        end
-        if name == "xml"
-          if @document_status
-            raise ParseException.new("Malformed XML: XML declaration is not at the start", @source)
+          if /\AUTF-16(?:BE|LE)\z/i =~ @source.encoding
+            encoding = "UTF-16"
           end
-          version = VERSION.match(content)
-          version = version[1] unless version.nil?
-          encoding = ENCODING.match(content)
-          encoding = encoding[1] unless encoding.nil?
-          if need_source_encoding_update?(encoding)
-            @source.encoding = encoding
+          return [ :xmldecl, @version, encoding, nil ] # e.g. <?xml version="1.0"?>
+        end
+
+        if @source.match?("encoding", true)
+          encoding = get_attribute("xml")
+          unless @source.skip_spaces?
+            unless @source.match?("?>", true)
+              raise ParseException.new("Malformed XML: Unclosed XML declaration", @source)
+            end
+            if need_source_encoding_update?(encoding)
+              @source.encoding = encoding
+            end
+
+            if encoding.nil? and /\AUTF-16(?:BE|LE)\z/i =~ @source.encoding
+              encoding = "UTF-16"
+            end
+            return [ :xmldecl, @version, encoding, nil ] # e.g. <?xml version="1.1" encoding="UTF-8"?>
           end
-          if encoding.nil? and /\AUTF-16(?:BE|LE)\z/i =~ @source.encoding
-            encoding = "UTF-16"
+        end
+
+        if @source.match?("standalone", true)
+          standalone = get_attribute("xml")
+          case standalone
+          when "yes", "no"
+          else
+            raise ParseException.new("Malformed XML: XML declaration standalone is not yes or no : <#{standalone}>", @source)
           end
-          standalone = STANDALONE.match(content)
-          standalone = standalone[1] unless standalone.nil?
-          return [ :xmldecl, version, encoding, standalone ]
         end
-        [:processing_instruction, name, content]
+        @source.match?(/\s*/um, true) # skip spaces
+        unless @source.match?("?>", true)
+          raise ParseException.new("Malformed XML: Unclosed XML declaration", @source)
+        end
+
+        if need_source_encoding_update?(encoding)
+          @source.encoding = encoding
+        end
+
+        if encoding.nil? and /\AUTF-16(?:BE|LE)\z/i =~ @source.encoding
+          encoding = "UTF-16"
+        end
+
+        # e.g. <?xml version="1.0" ?>
+        #      <?xml version="1.1" encoding="UTF-8" ?>
+        #      <?xml version="1.1" standalone="yes"?>
+        #      <?xml version="1.1" encoding="UTF-8" standalone="yes" ?>
+        [ :xmldecl, @version, encoding, standalone ]
       end
 
       if StringScanner::Version < "3.1.1"
@@ -787,6 +844,25 @@ def scan_quote
         end
       end
 
+      def get_attribute(name)
+        unless @source.match?(/\s*=\s*/um, true)
+          message = "Missing attribute equal: <#{name}>"
+          raise REXML::ParseException.new(message, @source)
+        end
+        unless quote = scan_quote
+          message = "Missing attribute value start quote: <#{name}>"
+          raise REXML::ParseException.new(message, @source)
+        end
+        start_position = @source.position
+        value = @source.read_until(quote)
+        unless value.chomp!(quote)
+          @source.position = start_position
+          message = "Missing attribute value end quote: <#{name}>: <#{quote}>"
+          raise REXML::ParseException.new(message, @source)
+        end
+        value
+      end
+
       def parse_attributes(prefixes)
         attributes = {}
         expanded_names = {}
@@ -801,22 +877,7 @@ def parse_attributes(prefixes)
             name = match[1]
             prefix = match[2]
             local_part = match[3]
-
-            unless @source.match?(/\s*=\s*/um, true)
-              message = "Missing attribute equal: <#{name}>"
-              raise REXML::ParseException.new(message, @source)
-            end
-            unless quote = scan_quote
-              message = "Missing attribute value start quote: <#{name}>"
-              raise REXML::ParseException.new(message, @source)
-            end
-            start_position = @source.position
-            value = @source.read_until(quote)
-            unless value.chomp!(quote)
-              @source.position = start_position
-              message = "Missing attribute value end quote: <#{name}>: <#{quote}>"
-              raise REXML::ParseException.new(message, @source)
-            end
+            value = get_attribute(name)
             @source.match?(/\s*/um, true)
             if prefix == "xmlns"
               if local_part == "xml"
diff --git a/lib/rexml/source.rb b/lib/rexml/source.rb
@@ -68,7 +68,7 @@ module Private
       SKIP_SPACES = /\s+/um
       SCANNER_RESET_SIZE = 100000
       PRE_DEFINED_TERM_PATTERNS = {}
-      pre_defined_terms = ["'", '"', "<", "]]>"]
+      pre_defined_terms = ["'", '"', "<", "]]>", "?>"]
       if StringScanner::Version < "3.1.1"
         pre_defined_terms.each do |term|
           PRE_DEFINED_TERM_PATTERNS[term] = /#{Regexp.escape(term)}/
@@ -143,7 +143,7 @@ def match(pattern, cons=false)
       end
     end
 
-    def match?(pattern, cons=false)
+    def match?(pattern, cons=false, read_keep=true)
       if cons
         !@scanner.skip(pattern).nil?
       else
diff --git a/test/parse/test_processing_instruction.rb b/test/parse/test_processing_instruction.rb
@@ -30,7 +30,7 @@ def test_unclosed_content
           parse("<?name content")
         end
         assert_equal(<<-DETAIL.chomp, exception.to_s)
-Malformed XML: Unclosed processing instruction
+Malformed XML: Unclosed processing instruction: <name>
 Line: 1
 Position: 14
 Last 80 unconsumed characters:
@@ -43,14 +43,27 @@ def test_unclosed_no_content
           parse("<?name")
         end
         assert_equal(<<-DETAIL.chomp, exception.to_s)
-Malformed XML: Unclosed processing instruction
+Malformed XML: Unclosed processing instruction: <name>
 Line: 1
 Position: 6
 Last 80 unconsumed characters:
 
         DETAIL
       end
 
+      def test_xml_declaration_duplicated
+        exception = assert_raise(REXML::ParseException) do
+          parse('<?xml version="1.0"?><?xml version="1.0"?>')
+        end
+        assert_equal(<<-DETAIL.chomp, exception.to_s)
+Malformed XML: XML declaration is duplicated
+Line: 1
+Position: 42
+Last 80 unconsumed characters:
+ version="1.0"?>
+        DETAIL
+      end
+
       def test_xml_declaration_not_at_document_start
         exception = assert_raise(REXML::ParseException) do
           parser = REXML::Parsers::BaseParser.new('<a><?xml version="1.0" ?></a>')
@@ -64,7 +77,118 @@ def test_xml_declaration_not_at_document_start
           Line: 1
           Position: 25
           Last 80 unconsumed characters:
+           version="1.0" ?>
+        DETAIL
+      end
+
+      def test_xml_declaration_missing_spaces
+        exception = assert_raise(REXML::ParseException) do
+          parser = REXML::Parsers::BaseParser.new('<?xml?>')
+          while parser.has_next?
+            parser.pull
+          end
+        end
+
+        assert_equal(<<~DETAIL.chomp, exception.to_s)
+          Malformed XML: XML declaration is missing spaces
+          Line: 1
+          Position: 7
+          Last 80 unconsumed characters:
+          ?>
+        DETAIL
+      end
+
+      def test_xml_declaration_missing_version
+        exception = assert_raise(REXML::ParseException) do
+          parser = REXML::Parsers::BaseParser.new('<?xml ?>')
+          while parser.has_next?
+            parser.pull
+          end
+        end
+
+        assert_equal(<<~DETAIL.chomp, exception.to_s)
+          Malformed XML: XML declaration is missing version
+          Line: 1
+          Position: 8
+          Last 80 unconsumed characters:
+          ?>
+        DETAIL
+      end
+
+      def test_xml_declaration_unclosed_content
+        exception = assert_raise(REXML::ParseException) do
+          parse('<?xml version="1.0"')
+        end
+        assert_equal(<<-DETAIL.chomp, exception.to_s)
+Malformed XML: Unclosed XML declaration
+Line: 1
+Position: 19
+Last 80 unconsumed characters:
+
+        DETAIL
+      end
 
+      def test_xml_declaration_unclosed_content_missing_space_after_version
+        exception = assert_raise(REXML::ParseException) do
+          parser = REXML::Parsers::BaseParser.new('<?xml version="1.0"encoding="UTF-8"?>')
+          while parser.has_next?
+            parser.pull
+          end
+        end
+
+        assert_equal(<<~DETAIL.chomp, exception.to_s)
+          Malformed XML: Unclosed XML declaration
+          Line: 1
+          Position: 37
+          Last 80 unconsumed characters:
+          encoding="UTF-8"?>
+        DETAIL
+      end
+
+      def test_xml_declaration_unclosed_content_missing_space_after_encoding
+        exception = assert_raise(REXML::ParseException) do
+          parser = REXML::Parsers::BaseParser.new('<?xml version="1.0" encoding="UTF-8"standalone="no"?>')
+          while parser.has_next?
+            parser.pull
+          end
+        end
+
+        assert_equal(<<~DETAIL.chomp, exception.to_s)
+          Malformed XML: Unclosed XML declaration
+          Line: 1
+          Position: 53
+          Last 80 unconsumed characters:
+          standalone="no"?>
+        DETAIL
+      end
+
+      def test_xml_declaration_unclosed_content_with_unknown_attributes
+        exception = assert_raise(REXML::ParseException) do
+          parser = REXML::Parsers::BaseParser.new('<?xml version="1.0" test="no"?>')
+          while parser.has_next?
+            parser.pull
+          end
+        end
+
+        assert_equal(<<~DETAIL.chomp, exception.to_s)
+          Malformed XML: Unclosed XML declaration
+          Line: 1
+          Position: 31
+          Last 80 unconsumed characters:
+          test="no"?>
+        DETAIL
+      end
+
+      def test_xml_declaration_standalone_no_yes_or_no
+        exception = assert_raise(REXML::ParseException) do
+          parse('<?xml version="1.0" standalone="YES"?>')
+        end
+        assert_equal(<<-DETAIL.chomp, exception.to_s)
+Malformed XML: XML declaration standalone is not yes or no : <YES>
+Line: 1
+Position: 38
+Last 80 unconsumed characters:
+?>
         DETAIL
       end
     end
@@ -113,7 +237,7 @@ def test_content_question
     def test_linear_performance_gt
       seq = [10000, 50000, 100000, 150000, 200000]
       assert_linear_performance(seq, rehearsal: 10) do |n|
-        REXML::Document.new("<?xml version=\"1.0\" " + ">" * n + " ?>")
+        REXML::Document.new("<?name content " + ">" * n + " ?>")
       end
     end
 
diff --git a/test/test_xml_declaration.rb b/test/test_xml_declaration.rb
@@ -7,7 +7,7 @@ module REXMLTests
   class TestXmlDeclaration < Test::Unit::TestCase
     def setup
       xml = <<~XML
-      <?xml encoding= 'UTF-8' standalone='yes'?>
+      <?xml version='1.0' encoding= 'UTF-8' standalone='yes'?>
       <root>
       </root>
       XML
