Added XML declaration check & Source#skip_spaces method (#282)

## Why?

### Added XML declaration check

- 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

### Added `Source#skip_spaces` method

In the case of `@source.match?(/\s+/um, true)`, if there are no spaces
at the beginning, I want to stop reading immediately.
However, it continues to read the buffer until it finds a match, but it
never finds a match.
As a result, it continues reading until the end of the file.

In the case of large XML files, drop_parsed_content occur frequently
until the buffer is cleared, which may affect performance.


## Benchmark

```
                         before       after  before(YJIT)  after(YJIT) 
                 dom     32.534      35.130        54.559       53.528 i/s -     100.000 times in 3.073715s 2.846540s 1.832883s 1.868189s
                 sax     44.785      44.089        78.303       77.842 i/s -     100.000 times in 2.232907s 2.268138s 1.277093s 1.284657s
                pull     51.750      51.105        90.819       90.658 i/s -     100.000 times in 1.932351s 1.956759s 1.101094s 1.103050s
              stream     51.427      51.444        89.820       88.971 i/s -     100.000 times in 1.944502s 1.943855s 1.113340s 1.123960s

Comparison:
                              dom
        before(YJIT):        54.6 i/s 
         after(YJIT):        53.5 i/s - 1.02x  slower
               after:        35.1 i/s - 1.55x  slower
              before:        32.5 i/s - 1.68x  slower

                              sax
        before(YJIT):        78.3 i/s 
         after(YJIT):        77.8 i/s - 1.01x  slower
              before:        44.8 i/s - 1.75x  slower
               after:        44.1 i/s - 1.78x  slower

                             pull
        before(YJIT):        90.8 i/s 
         after(YJIT):        90.7 i/s - 1.00x  slower
              before:        51.8 i/s - 1.75x  slower
               after:        51.1 i/s - 1.78x  slower

                           stream
        before(YJIT):        89.8 i/s 
         after(YJIT):        89.0 i/s - 1.01x  slower
               after:        51.4 i/s - 1.75x  slower
              before:        51.4 i/s - 1.75x  slower
```

- YJIT=ON : 0.98x - 1.00x faster
- YJIT=OFF : 0.98x - 1.07x faster

Author: naitoh

lib/rexml/parsers/baseparser.rb

@@ -144,6 +144,7 @@ module Private
         PEREFERENCE_PATTERN = /#{PEREFERENCE}/um
         TAG_PATTERN = /((?>#{QNAME_STR}))\s*/um
         CLOSE_PATTERN = /(#{QNAME_STR})\s*>/um
+        EQUAL_PATTERN = /\s*=\s*/um
         ATTLISTDECL_END = /\s+#{NAME}(?:#{ATTDEF})*\s*>/um
         NAME_PATTERN = /#{NAME}/um
         GEDECL_PATTERN = "\\s+#{NAME}\\s+#{ENTITYDEF}\\s*>"
@@ -168,6 +169,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 )
@@ -280,7 +282,7 @@ def pull_event
               return [ :comment, process_comment ]
             elsif @source.match?("DOCTYPE", true)
               base_error_message = "Malformed DOCTYPE"
-              unless @source.match?(/\s+/um, true)
+              unless @source.skip_spaces
                 if @source.match?(">")
                   message = "#{base_error_message}: name is missing"
                 else
@@ -290,7 +292,7 @@ def pull_event
                 raise REXML::ParseException.new(message, @source)
               end
               name = parse_name(base_error_message)
-              @source.match?(/\s*/um, true) # skip spaces
+              @source.skip_spaces
               if @source.match?("[", true)
                 id = [nil, nil, nil]
                 @document_status = :in_doctype
@@ -306,7 +308,7 @@ def pull_event
                   # For backward compatibility
                   id[1], id[2] = id[2], nil
                 end
-                @source.match?(/\s*/um, true) # skip spaces
+                @source.skip_spaces
                 if @source.match?("[", true)
                   @document_status = :in_doctype
                 elsif @source.match?(">", true)
@@ -319,7 +321,7 @@ def pull_event
               end
               args = [:start_doctype, name, *id]
               if @document_status == :after_doctype
-                @source.match?(/\s*/um, true)
+                @source.skip_spaces
                 @stack << [ :end_doctype ]
               end
               return args
@@ -330,7 +332,7 @@ def pull_event
           end
         end
         if @document_status == :in_doctype
-          @source.match?(/\s*/um, true) # skip spaces
+          @source.skip_spaces
           start_position = @source.position
           if @source.match?("<!", true)
             if @source.match?("ELEMENT", true)
@@ -391,7 +393,7 @@ def pull_event
               return [ :attlistdecl, element, pairs, contents ]
             elsif @source.match?("NOTATION", true)
               base_error_message = "Malformed notation declaration"
-              unless @source.match?(/\s+/um, true)
+              unless @source.skip_spaces
                 if @source.match?(">")
                   message = "#{base_error_message}: name is missing"
                 else
@@ -404,7 +406,7 @@ def pull_event
               id = parse_id(base_error_message,
                             accept_external_id: true,
                             accept_public_id: true)
-              @source.match?(/\s*/um, true) # skip spaces
+              @source.skip_spaces
               unless @source.match?(">", true)
                 message = "#{base_error_message}: garbage before end >"
                 raise REXML::ParseException.new(message, @source)
@@ -425,7 +427,7 @@ def pull_event
           end
         end
         if @document_status == :after_doctype
-          @source.match?(/\s*/um, true)
+          @source.skip_spaces
         end
         begin
           start_position = @source.position
@@ -642,6 +644,10 @@ def need_source_encoding_update?(xml_declaration_encoding)
         true
       end
 
+      def normalize_xml_declaration_encoding(xml_declaration_encoding)
+        /\AUTF-16(?:BE|LE)\z/i.match?(xml_declaration_encoding) ? "UTF-16" : nil
+      end
+
       def parse_name(base_error_message)
         md = @source.match(Private::NAME_PATTERN, true)
         unless md
@@ -735,37 +741,85 @@ def process_comment
 
       def process_instruction
         name = parse_name("Malformed XML: Invalid processing instruction node")
-        if @source.match?(/\s+/um, true)
-          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 misses spaces before version", @source)
+        end
+        unless @source.match?("version", true)
+          raise ParseException.new("Malformed XML: XML declaration misses version", @source)
+        end
+        @version = parse_attribute_value_with_equal("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
+          encoding = normalize_xml_declaration_encoding(@source.encoding)
+          return [ :xmldecl, @version, encoding, nil ] # e.g. <?xml version="1.0"?>
         end
-        if name == "xml"
-          if @document_status
-            raise ParseException.new("Malformed XML: XML declaration is not at the start", @source)
-          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
+
+        if @source.match?("encoding", true)
+          encoding = parse_attribute_value_with_equal("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
+            encoding ||= normalize_xml_declaration_encoding(@source.encoding)
+            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 = parse_attribute_value_with_equal("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.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
+        encoding ||= normalize_xml_declaration_encoding(@source.encoding)
+
+        # 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 +841,25 @@ def scan_quote
         end
       end
 
+      def parse_attribute_value_with_equal(name)
+        unless @source.match?(Private::EQUAL_PATTERN, 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,23 +874,8 @@ 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
-            @source.match?(/\s*/um, true)
+            value = parse_attribute_value_with_equal(name)
+            @source.skip_spaces
             if prefix == "xmlns"
               if local_part == "xml"
                 if value != Private::XML_PREFIXED_NAMESPACE

lib/rexml/source.rb

@@ -65,9 +65,10 @@ class Source
     attr_reader :encoding
 
     module Private
+      SPACES_PATTERN = /\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)}/
@@ -150,6 +151,10 @@ def match?(pattern, cons=false)
       end
     end
 
+    def skip_spaces
+      @scanner.skip(Private::SPACES_PATTERN) ? true : false
+    end
+
     def position
       @scanner.pos
     end

test/parse/test_document_type_declaration.rb

@@ -49,10 +49,10 @@ def test_no_name
         end
         assert_equal(<<-DETAIL.chomp, exception.to_s)
 Malformed DOCTYPE: name is missing
-Line: 3
-Position: 17
+Line: 1
+Position: 10
 Last 80 unconsumed characters:
-<!DOCTYPE>  <r/> 
+<!DOCTYPE>
         DETAIL
       end
     end