Don't process siblings when reaching a root node

When generating XML we should not process the siblings of a root node.
Doing so results in invalid XML being returned (due to siblings not
being children of the root node).

Not processing the siblings in this case also prevents the siblings loop
from getting stuck. To explain what's happening, let's assume we're
using the following document tree:

    Document
      |_ Text
      |_ Element

Now let's say we take the Text node and call "to_xml" on it. When we
start the loop we'll run into the following code:

    if child_node = children && current.children[0]
      current = child_node
    else

Here the if statement will evaluate to false because a Text node doesn't
have any child nodes, as such we enter the else branch. We now reach the
following code:

    until next_node = current.is_a?(Node) && current.next

A Text node is a descendant of Node and it happens to have another node
(the Element node) as the next sibling. As a result we enter the `until`
loop's body. We now run into this code:

    if current.is_a?(Node) && current != @start
      current = current.parent
    end

Here `current` is still our Text node and it is the @start node. As a
result the `current` re-assignment won't be evaluated.

Next we run into the following:

    after_element(current, output) if current.is_a?(Element)

    break if current == @start

The first line will not evaluate because `current` is still the `Text`
node.  The `break` *will* evaluate because `current` is the same as
@start.

This will then lead to the following code being executed:

    current = next_node

Here `next_node` is the next sibling of the Text node, which in the
above example is the Element node.

Because all of the above runs in a `while` loop we'll at some point end
up again at the start of the `until` loop. At this point the `current`
variable contains an `Element`. Because this node does *not* have a node
following it we'll once again enter the `until` loop's body.

This loop will now get stuck because `current` is a Node, it's not the
same as @start, thus `current` is set to its parent (the Document),
which also isn't the same as @start.

On the next iteration this loop will break because `current` is no
longer a node. However, because a Document _does_ have child nodes the
whole process of traversing children/siblings will keep repeating itself
forever.

To work around this we now use the following statement:

    if child_node = children && current.children[0]
      ...
    elsif current == @start
      after_element(current, output) if current.is_a?(Element)

      break
    else
      until next_node = current.is_a?(Node) && current.next
      ...
    end

This prevents processing of any siblings once we have reached the root
node, in turn preventing the loop getting stuck forever.

I'm willing to bet there are probably a few more edge cases, but I can't
think of any others at the moment.

Fixes #161
This commit is contained in:
Yorick Peterse 2016-09-10 02:14:04 +02:00
parent 7aa34fd192
commit 38284278d5
No known key found for this signature in database
GPG Key ID: EDD30D2BEB691AC9
2 changed files with 38 additions and 0 deletions

View File

@ -65,6 +65,13 @@ module Oga
if child_node = children && current.children[0] if child_node = children && current.children[0]
current = child_node current = child_node
elsif current == @start
# When we have reached the root node we should not process
# any of its siblings. If we did we'd include XML in the
# output from elements no part of the root node.
after_element(current, output) if current.is_a?(Element)
break
else else
until next_node = current.is_a?(Node) && current.next until next_node = current.is_a?(Node) && current.next
if current.is_a?(Node) && current != @start if current.is_a?(Node) && current != @start

View File

@ -47,5 +47,36 @@ describe Oga::XML::Generator do
output.should == '<root><a /><b><c /></b></root>' output.should == '<root><a /><b><c /></b></root>'
end end
end end
describe 'using a Text node in a Document as the root node' do
it 'returns a String' do
text = Oga::XML::Text.new(text: "\n")
element = Oga::XML::Element.new(name: 'foo')
document = Oga::XML::Document.new(children: [text, element])
described_class.new(text).to_xml.should == "\n"
end
end
describe 'using an Element in a Document as the root node' do
it 'returns a String' do
text = Oga::XML::Text.new(text: "\n")
element = Oga::XML::Element.new(name: 'foo')
document = Oga::XML::Document.new(children: [text, element])
described_class.new(element).to_xml.should == '<foo />'
end
end
describe 'using an Element with a sibling as the root node' do
it 'returns a String' do
element1 = Oga::XML::Element.new(name: 'a')
element2 = Oga::XML::Element.new(name: 'b')
document = Oga::XML::Document.new(children: [element1, element2])
described_class.new(element1).to_xml.should == '<a />'
described_class.new(element2).to_xml.should == '<b />'
end
end
end end
end end