fix: Extra content at the end of the document #161

naitoh · 2024-07-04T13:54:02Z

Why?

XML with additional content at the end of the document is invalid.

https://www.w3.org/TR/2006/REC-xml11-20060816/#document

[1]   document   ::=   ( prolog element Misc* ) - ( Char* RestrictedChar Char* )

https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-Misc

[27]    Misc       ::=          Comment | PI | S

https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-PI

[16]    PI         ::=          '<?' PITarget (S (Char* - (Char* '?>' Char*)))? '?>'

https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-PITarget

[17]    PITarget           ::=          Name - (('X' | 'x') ('M' | 'm') ('L' | 'l'))

## Why? XML with multiple root elements is invalid. See: ruby#160 (comment)

lib/rexml/parsers/baseparser.rb

test/parser/test_base_parser.rb

## Why? XML declaration must be the first item. https://www.w3.org/TR/2006/REC-xml11-20060816/#document ``` [1] document ::= ( prolog element Misc* ) - ( Char* RestrictedChar Char* ) ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-prolog ``` [22] prolog ::= XMLDecl Misc* (doctypedecl Misc*)? ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-XMLDecl ``` [23] XMLDecl ::= '<?xml' VersionInfo EncodingDecl? SDDecl? S? '?>' ``` See: ruby#161 (comment)

test/parse/test_processing_instruction.rb

test/parse/test_comment.rb

test/parse/test_element.rb

test/parse/test_text.rb

kou · 2024-07-06T22:02:34Z

lib/rexml/parsers/baseparser.rb

              return [ :start_element, tag, attributes ]
            end
          else
            text = @source.read_until("<")
            if text.chomp!("<")
              @source.position -= "<".bytesize
            end
+            if @tags.empty? and @have_root
+              if text.strip != ""


strip allocates a new string. Can we avoid it?
For example: /\A\s*\z/.match?(text)

## Why? XML with additional content at the end of the document is invalid. https://www.w3.org/TR/2006/REC-xml11-20060816/#document ``` [1] document ::= ( prolog element Misc* ) - ( Char* RestrictedChar Char* ) ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-Misc ``` [27] Misc ::= Comment | PI | S ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-PI ``` [16] PI ::= '<?' PITarget (S (Char* - (Char* '?>' Char*)))? '?>' ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-PITarget ``` [17] PITarget ::= Name - (('X' | 'x') ('M' | 'm') ('L' | 'l')) ```

## Why? XML declaration must be the first item. https://www.w3.org/TR/2006/REC-xml11-20060816/#document ``` [1] document ::= ( prolog element Misc* ) - ( Char* RestrictedChar Char* ) ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-prolog ``` [22] prolog ::= XMLDecl Misc* (doctypedecl Misc*)? ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-XMLDecl ``` [23] XMLDecl ::= '<?xml' VersionInfo EncodingDecl? SDDecl? S? '?>' ``` See: ruby#161 (comment)

kou · 2024-07-07T20:52:22Z

Thanks.

## Why? XML declaration must be the first item. https://www.w3.org/TR/2006/REC-xml11-20060816/#document ``` [1] document ::= ( prolog element Misc* ) - ( Char* RestrictedChar Char* ) ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-prolog ``` [22] prolog ::= XMLDecl Misc* (doctypedecl Misc*)? ``` https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-XMLDecl ``` [23] XMLDecl ::= '<?xml' VersionInfo EncodingDecl? SDDecl? S? '?>' ``` See: #161 (comment)

DmitryPogrebnoy · 2024-10-15T10:50:12Z

@naitoh @kou After this change, parsing of '<threads><thread id="1"/></threads><threads><thread id="1"/></threads>' using REXML::Parsers::PullParser.new(socket).pull throw an illegal seek exception. Should I create a separate issue for this problem?

The main cause is this if statement at lib/rexml/parsers/baseparser.rb:498

if @tags.empty? and @have_root
  raise ParseException.new("Malformed XML: Extra tag at the end of the document (got '<#{tag}')", @source)
end

kou · 2024-10-16T00:57:46Z

<threads><thread id="1"/></threads><threads><thread id="1"/></threads> is an invalid XML.
Are you suggesting that we should support an invalid XML?

DmitryPogrebnoy · 2024-10-16T12:35:57Z

@kou Well, I use socket to get xml messages from the server and parse them using PullParser. Each message is complete and valid. Before change it worked like a charm. Now it doesn't work anymore.
If the current way with REXML::Parsers::PullParser.new(socket).pull does not work, what is the right approach for such a use case?

kou · 2024-10-16T21:16:12Z

OK. Could you open a new issue for it?

DmitryPogrebnoy · 2024-10-17T08:33:08Z

@kou Yep, here it is - #214

fix multiple root elements

c4fb89b

## Why? XML with multiple root elements is invalid. See: ruby#160 (comment)

kou reviewed Jul 5, 2024

View reviewed changes

naitoh force-pushed the fix_multiple_root_elements branch from 641d9d1 to 4e9de51 Compare July 5, 2024 04:19

naitoh requested a review from kou July 5, 2024 04:30

naitoh mentioned this pull request Jul 6, 2024

Add position check for XML declaration #162

Merged

kou reviewed Jul 6, 2024

View reviewed changes

naitoh force-pushed the fix_multiple_root_elements branch from 4e9de51 to c094825 Compare July 7, 2024 00:40

naitoh requested a review from kou July 7, 2024 00:45

kou merged commit eb45c8d into ruby:master Jul 7, 2024
61 checks passed

naitoh deleted the fix_multiple_root_elements branch July 7, 2024 21:02

raymzag mentioned this pull request Jul 18, 2024

Upgrade rexml to 3.3.4 to address CVE-2024-39908, 41123, 41946 activemerchant/active_merchant#5181

Merged

Fryguy mentioned this pull request Jul 18, 2024

Stop adding extra new line after XML declaration with pretty format #164

Merged

DmitryPogrebnoy mentioned this pull request Oct 17, 2024

Can't use PullParser to parse several valid messages in a row #214

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Extra content at the end of the document #161

fix: Extra content at the end of the document #161

naitoh commented Jul 4, 2024

kou Jul 6, 2024

naitoh Jul 7, 2024

kou commented Jul 7, 2024

DmitryPogrebnoy commented Oct 15, 2024 •

edited

Loading

kou commented Oct 16, 2024

DmitryPogrebnoy commented Oct 16, 2024 •

edited

Loading

kou commented Oct 16, 2024

DmitryPogrebnoy commented Oct 17, 2024

fix: Extra content at the end of the document #161

fix: Extra content at the end of the document #161

Conversation

naitoh commented Jul 4, 2024

Why?

kou Jul 6, 2024

Choose a reason for hiding this comment

naitoh Jul 7, 2024

Choose a reason for hiding this comment

kou commented Jul 7, 2024

DmitryPogrebnoy commented Oct 15, 2024 • edited Loading

kou commented Oct 16, 2024

DmitryPogrebnoy commented Oct 16, 2024 • edited Loading

kou commented Oct 16, 2024

DmitryPogrebnoy commented Oct 17, 2024

DmitryPogrebnoy commented Oct 15, 2024 •

edited

Loading

DmitryPogrebnoy commented Oct 16, 2024 •

edited

Loading