Prevent adding non-DOMElement elements in DomCrawler

stof · stof · commit 507fdf93387d · 2015-10-02T01:17:43.000+02:00
Many methods of the DomCrawler component are relying on the DOMElement
API, not only on the DOMNode API.
diff --git a/src/Symfony/Component/DomCrawler/Crawler.php b/src/Symfony/Component/DomCrawler/Crawler.php
@@ -308,10 +308,14 @@ public function addNodes(array $nodes)
     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 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.
@@ -974,7 +978,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
@@ -20,7 +20,10 @@ public function testConstructor()
         $crawler = new Crawler();
         $this->assertCount(0, $crawler, '__construct() returns an empty crawler');
 
-        $crawler = new Crawler(new \DOMNode());
+        $doc = new \DOMDocument();
+        $node = $doc->createElement('test');
+
+        $crawler = new Crawler($node);
         $this->assertCount(1, $crawler, '__construct() takes a node as a first argument');
     }
 
@@ -37,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;
         }
@@ -267,6 +271,7 @@ public function testAddNodeList()
      */
     public function testAddNodes()
     {
+        $list = array();
         foreach ($this->createNodeList() as $node) {
             $list[] = $node;
         }
@@ -290,7 +295,10 @@ public function testAddNode()
 
     public function testClear()
     {
-        $crawler = new Crawler(new \DOMNode());
+        $doc = new \DOMDocument();
+        $node = $doc->createElement('test');
+
+        $crawler = new Crawler($node);
         $crawler->clear();
         $this->assertCount(0, $crawler, '->clear() removes all the nodes from the crawler');
     }
@@ -526,7 +534,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()

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,10 @@ public function testConstructor()`
`20`	`20`	`$crawler = new Crawler();`
`21`	`21`	`$this->assertCount(0, $crawler, '__construct() returns an empty crawler');`
`22`	`22`
`23`		`- $crawler = new Crawler(new \DOMNode());`
	`23`	`+ $doc = new \DOMDocument();`
	`24`	`+ $node = $doc->createElement('test');`
	`25`	`+`
	`26`	`+ $crawler = new Crawler($node);`
`24`	`27`	`$this->assertCount(1, $crawler, '__construct() takes a node as a first argument');`
`25`	`28`	`}`
`26`	`29`
`@@ -37,6 +40,7 @@ public function testAdd()`
`37`	`40`	`$crawler->add($this->createNodeList());`
`38`	`41`	`$this->assertEquals('foo', $crawler->filterXPath('//div')->attr('class'), '->add() adds nodes from a \DOMNodeList');`
`39`	`42`
	`43`	`+ $list = array();`
`40`	`44`	`foreach ($this->createNodeList() as $node) {`
`41`	`45`	`$list[] = $node;`
`42`	`46`	`}`
`@@ -267,6 +271,7 @@ public function testAddNodeList()`
`267`	`271`	`*/`
`268`	`272`	`public function testAddNodes()`
`269`	`273`	`{`
	`274`	`+ $list = array();`
`270`	`275`	`foreach ($this->createNodeList() as $node) {`
`271`	`276`	`$list[] = $node;`
`272`	`277`	`}`
`@@ -290,7 +295,10 @@ public function testAddNode()`
`290`	`295`
`291`	`296`	`public function testClear()`
`292`	`297`	`{`
`293`		`- $crawler = new Crawler(new \DOMNode());`
	`298`	`+ $doc = new \DOMDocument();`
	`299`	`+ $node = $doc->createElement('test');`
	`300`	`+`
	`301`	`+ $crawler = new Crawler($node);`
`294`	`302`	`$crawler->clear();`
`295`	`303`	`$this->assertCount(0, $crawler, '->clear() removes all the nodes from the crawler');`
`296`	`304`	`}`
`@@ -526,7 +534,7 @@ public function testFilterXPathWithAttributeAxis()`
`526`	`534`
`527`	`535`	`public function testFilterXPathWithAttributeAxisAfterElementAxis()`
`528`	`536`	`{`
`529`		`- $this->assertCount(3, $this->createTestCrawler()->filterXPath('//form/button/attribute::*'), '->filterXPath() handles attribute axes properly when they are preceded by an element filtering axis');`
	`537`	`+ $this->assertCount(0, $this->createTestCrawler()->filterXPath('//form/button/attribute::*'), '->filterXPath() handles attribute axes properly when they are preceded by an element filtering axis');`
`530`	`538`	`}`
`531`	`539`
`532`	`540`	`public function testFilterXPathWithChildAxis()`