Skip to content

Commit

Permalink
Remove leading whitespace from the XML under test
Browse files Browse the repository at this point in the history
This test was originally written with the intention of asserting that
a runtime error related to XXE will be raised by the parser. However,
because initial whitespace is present before the doctype,
XmlMini_NokogiriSAX::HashBuilder has been raising an unrelated error
in this test.

Related to #41015

---

Using Nokogiri v1.10.10, the error being raised without this change
is:

> XML declaration allowed only at the start of the document

but with this change we see the expected exception from the libxml2
SAX parser:

> Entity 'a' not defined

Using Nokogiri v1.11.0, in which error handling is broken (see
sparklemotion/nokogiri#2168), without this change we see an exception
being raised by HashBuilder because `characters` is called before
`start_element` and so the content hash isn't initialized (see

The error being raised with this change is:

> Parse stack not empty!

which is not the error we want (because of
sparklemotion/nokogiri#2168), but the test still passes.

Using Nokogiri with the fix from sparklemotion/nokogiri#2169, the
error being raised without this change is:

> XML declaration allowed only at the start of the document

but with this change will be:

> Entity 'a' not defined

and we're back to the expected behavior.
  • Loading branch information
flavorjones committed Jan 6, 2021
1 parent 7f0bbed commit 76bfda8
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion activesupport/test/xml_mini/xml_mini_engine_test.rb
Expand Up @@ -50,7 +50,7 @@ def test_file_from_xml

def test_exception_thrown_on_expansion_attack
assert_raise expansion_attack_error do
Hash.from_xml(<<-eoxml)
Hash.from_xml(<<~eoxml)
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE member [
<!ENTITY a "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;">
Expand Down

0 comments on commit 76bfda8

Please sign in to comment.