Merge pull request php-webdriver#1064 from OndraM/fix/assert-return-data

Do not fail on fatal error on unexpected response when getting element(s)

Author: OndraM

CHANGELOG.md

@@ -6,9 +6,12 @@ This project versioning adheres to [Semantic Versioning](http://semver.org/).
 - Capability key `ChromeOptions::CAPABILITY_W3C` used to set ChromeOptions is now deprecated in favor of `ChromeOptions::CAPABILITY`, which now also contains the W3C compatible value (`goog:chromeOptions`).
 - ChromeOptions are now passed to the driver always as a W3C compatible key `goog:chromeOptions`, even in the deprecated OSS JsonWire payload (as ChromeDriver [supports](https://bugs.chromium.org/p/chromedriver/issues/detail?id=1786) this since 2017).
 
+### Fixed
+- Handle unexpected response when getting element(s) by throwing an exception, not triggering fatal error.
+
 ## 1.14.0 - 2023-02-09
 ### Added
-- `PhpWebDriverExceptionInterface` as a common interface to identify all exceptions thrown in php-webdriver
+- `PhpWebDriverExceptionInterface` as a common interface to identify all exceptions thrown in php-webdriver.
 
 ### Changed
 - Require PHP ^7.3.

lib/Exception/Internal/UnexpectedResponseException.php

@@ -14,6 +14,16 @@ public static function forError(string $message): self
         return new self($message);
     }
 
+    public static function forElementNotArray($response): self
+    {
+        return new self(
+            sprintf(
+                "Unexpected server response for getting an element. Expected array, but the response was: '%s'\n",
+                print_r($response, true)
+            )
+        );
+    }
+
     public static function forJsonDecodingError(int $jsonLastError, string $rawResults): self
     {
         return new self(

lib/Remote/JsonWireCompat.php

@@ -2,6 +2,7 @@
 
 namespace Facebook\WebDriver\Remote;
 
+use Facebook\WebDriver\Exception\Internal\UnexpectedResponseException;
 use Facebook\WebDriver\WebDriverBy;
 
 /**
@@ -18,8 +19,17 @@ abstract class JsonWireCompat
      */
     public const WEB_DRIVER_ELEMENT_IDENTIFIER = 'element-6066-11e4-a52e-4f735466cecf';
 
-    public static function getElement(array $rawElement)
+    /**
+     * @param mixed $rawElement Value is validated to by an array, exception is thrown otherwise
+     * @throws UnexpectedResponseException When value of other type than array is given
+     */
+    public static function getElement($rawElement)
     {
+        // The method intentionally accept mixed, so that assertion of the rawElement format could be done on one place
+        if (!is_array($rawElement)) {
+            throw UnexpectedResponseException::forElementNotArray($rawElement);
+        }
+
         if (array_key_exists(self::WEB_DRIVER_ELEMENT_IDENTIFIER, $rawElement)) {
             // W3C's WebDriver
             return $rawElement[self::WEB_DRIVER_ELEMENT_IDENTIFIER];

lib/Remote/RemoteWebDriver.php

@@ -219,10 +219,6 @@ public function findElement(WebDriverBy $by)
             JsonWireCompat::getUsing($by, $this->isW3cCompliant)
         );
 
-        if ($raw_element === null) {
-            throw UnexpectedResponseException::forError('Unexpected server response to findElement command');
-        }
-
         return $this->newElement(JsonWireCompat::getElement($raw_element));
     }
 
@@ -239,8 +235,8 @@ public function findElements(WebDriverBy $by)
             JsonWireCompat::getUsing($by, $this->isW3cCompliant)
         );
 
-        if ($raw_elements === null) {
-            throw UnexpectedResponseException::forError('Unexpected server response to findElements command');
+        if (!is_array($raw_elements)) {
+            throw UnexpectedResponseException::forError('Server response to findElements command is not an array');
         }
 
         $elements = [];

lib/Remote/RemoteWebElement.php

@@ -5,6 +5,7 @@
 use Facebook\WebDriver\Exception\ElementNotInteractableException;
 use Facebook\WebDriver\Exception\Internal\IOException;
 use Facebook\WebDriver\Exception\Internal\LogicException;
+use Facebook\WebDriver\Exception\Internal\UnexpectedResponseException;
 use Facebook\WebDriver\Exception\PhpWebDriverExceptionInterface;
 use Facebook\WebDriver\Exception\UnsupportedOperationException;
 use Facebook\WebDriver\Interactions\Internal\WebDriverCoordinates;
@@ -131,6 +132,10 @@ public function findElements(WebDriverBy $by)
             $params
         );
 
+        if (!is_array($raw_elements)) {
+            throw UnexpectedResponseException::forError('Server response to findChildElements command is not an array');
+        }
+
         $elements = [];
         foreach ($raw_elements as $raw_element) {
             $elements[] = $this->newElement(JsonWireCompat::getElement($raw_element));

lib/Remote/ShadowRoot.php

@@ -2,6 +2,7 @@
 
 namespace Facebook\WebDriver\Remote;
 
+use Facebook\WebDriver\Exception\Internal\UnexpectedResponseException;
 use Facebook\WebDriver\Exception\UnknownErrorException;
 use Facebook\WebDriver\WebDriverBy;
 use Facebook\WebDriver\WebDriverElement;
@@ -73,6 +74,12 @@ public function findElements(WebDriverBy $locator)
             $params
         );
 
+        if (!is_array($rawElements)) {
+            throw UnexpectedResponseException::forError(
+                'Server response to findElementsFromShadowRoot command is not an array'
+            );
+        }
+
         $elements = [];
         foreach ($rawElements as $rawElement) {
             $elements[] = new RemoteWebElement($this->executor, JsonWireCompat::getElement($rawElement), true);

tests/functional/Remote/JsonWireCompatTest.php

@@ -0,0 +1,17 @@
+<?php declare(strict_types=1);
+
+namespace Facebook\WebDriver\Remote;
+
+use Facebook\WebDriver\Exception\Internal\UnexpectedResponseException;
+use PHPUnit\Framework\TestCase;
+
+class JsonWireCompatTest extends TestCase
+{
+    public function testShouldThrowExceptionWhenElementIsNotArray()
+    {
+        $this->expectException(UnexpectedResponseException::class);
+        $this->expectExceptionMessage('Unexpected server response for getting an element. Expected array');
+
+        JsonWireCompat::getElement(null);
+    }
+}

tests/unit/Remote/RemoteWebDriverTest.php

@@ -114,15 +114,11 @@ public function testShouldCreateWebDriverWaitInstance(): void
     }
 
     /**
-     * @covers ::findElement
      * @covers ::findElements
      * @covers \Facebook\WebDriver\Exception\Internal\UnexpectedResponseException
-     * @dataProvider provideMethods
      */
-    public function testShouldThrowExceptionOnUnexpectedNullValueFromRemoteEnd(
-        string $method,
-        string $expectedExceptionMessage
-    ): void {
+    public function testShouldThrowExceptionOnUnexpectedValueFromRemoteEndWhenFindingElements(): void
+    {
         $executorMock = $this->createMock(HttpCommandExecutor::class);
         $executorMock->expects($this->once())
             ->method('execute')
@@ -132,15 +128,7 @@ public function testShouldThrowExceptionOnUnexpectedNullValueFromRemoteEnd(
         $this->driver->setCommandExecutor($executorMock);
 
         $this->expectException(UnexpectedResponseException::class);
-        $this->expectExceptionMessage($expectedExceptionMessage);
-        call_user_func([$this->driver, $method], $this->createMock(WebDriverBy::class));
-    }
-
-    public function provideMethods(): array
-    {
-        return [
-            ['findElement', 'Unexpected server response to findElement command'],
-            ['findElements', 'Unexpected server response to findElements command'],
-        ];
+        $this->expectExceptionMessage('Server response to findElements command is not an array');
+        $this->driver->findElements($this->createMock(WebDriverBy::class));
     }
 }