feature #16058 Prevent adding non-DOMElement elements in DomCrawler (stof)

fabpot · fabpot · commit 9e60980f5adb · 2015-10-02T12:40:27.000+02:00
This PR was merged into the 2.8 branch. Discussion ---------- Prevent adding non-DOMElement elements in DomCrawler | Q | A | ------------- | --- | Bug fix? | kind of | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Many methods of the DomCrawler component are relying on the DOMElement API, not only on the DOMNode API. All the typehints in the Form and Link APIs were already fixed in 2.5 because they are unusable with other kinds of nodes (fatal errors). However, the Crawler itself was not fixed. and this means that a bunch of its APIs can trigger fatal errors when passing other kinds of nodes. Thus, there is a case where the code was allowing such nodes to be injected in the Crawler for some XPath queries. I fixed it to avoid it, adding the same kind of filtering than in other places. Commits ------- 9f362a1 Prevent adding non-DOMElement elements in DomCrawler
diff --git a/src/Symfony/Component/DomCrawler/Crawler.php b/src/Symfony/Component/DomCrawler/Crawler.php
@@ -322,10 +322,14 @@ public function addNode(\DOMNode $node)
         }
 
         if ($node instanceof \DOMDocument) {
-            parent::attach($node->documentElement);
-        } else {
-            parent::attach($node);
+            $node = $node->documentElement;
+        }
+
+        if (!$node instanceof \DOMElement) {
+            throw new \InvalidArgumentException(sprintf('Nodes set in a Crawler must be DOMElement or DOMDocument instances, "%s" given.', get_class($node)));
         }
+
+        parent::attach($node);
     }
 
     // Serializing and unserializing a crawler creates DOM objects in a corrupted state. DOM elements are not properly serializable.
@@ -988,7 +992,12 @@ private function filterRelativeXPath($xpath)
 
         foreach ($this as $node) {
             $domxpath = $this->createDOMXPath($node->ownerDocument, $prefixes);
-            $crawler->add($domxpath->query($xpath, $node));
+
+            foreach ($domxpath->query($xpath, $node) as $subNode) {
+                if ($subNode->nodeType === 1) {
+                    $crawler->add($subNode);
+                }
+            }
         }
 
         return $crawler;
diff --git a/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php b/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php
@@ -40,6 +40,7 @@ public function testAdd()
         $crawler->add($this->createNodeList());
         $this->assertEquals('foo', $crawler->filterXPath('//div')->attr('class'), '->add() adds nodes from a \DOMNodeList');
 
+        $list = array();
         foreach ($this->createNodeList() as $node) {
             $list[] = $node;
         }
@@ -59,12 +60,22 @@ public function testAdd()
     /**
      * @expectedException \InvalidArgumentException
      */
-    public function testAddInvalidNode()
+    public function testAddInvalidType()
     {
         $crawler = new Crawler();
         $crawler->add(1);
     }
 
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage Nodes set in a Crawler must be DOMElement or DOMDocument instances, "DOMNode" given.
+     */
+    public function testAddInvalidNode()
+    {
+        $crawler = new Crawler();
+        $crawler->add(new \DOMNode());
+    }
+
     /**
      * @covers Symfony\Component\DomCrawler\Crawler::addHtmlContent
      */
@@ -541,7 +552,7 @@ public function testFilterXPathWithAttributeAxis()
 
     public function testFilterXPathWithAttributeAxisAfterElementAxis()
     {
-        $this->assertCount(3, $this->createTestCrawler()->filterXPath('//form/button/attribute::*'), '->filterXPath() handles attribute axes properly when they are preceded by an element filtering axis');
+        $this->assertCount(0, $this->createTestCrawler()->filterXPath('//form/button/attribute::*'), '->filterXPath() handles attribute axes properly when they are preceded by an element filtering axis');
     }
 
     public function testFilterXPathWithChildAxis()
