Fix XPath escaping in WebDriverSelect

Author: OndraM

CHANGELOG.md

@@ -15,6 +15,7 @@ This project versioning adheres to [Semantic Versioning](http://semver.org/).
     - `elementTextMatches` - text in element matches regular expression
     - `numberOfWindowsToBe` - number of opened windows equals given number
 - Possibility to select option of `<select>` by its partial text (using `selectByVisiblePartialText()`)
+- `XPathEscaper` helper class to quote XPaths containing both single and double quotes.
 
 ### Changed
 - `Symfony\Process` is used to start local WebDriver processes (when browsers are run directly, without Selenium server) to workaround some PHP bugs and improve portability.
@@ -23,6 +24,9 @@ This project versioning adheres to [Semantic Versioning](http://semver.org/).
 - Deprecated `WebDriverExpectedCondition::textToBePresentInElement()` in favor of `elementTextContains()`
 - Throw an exception when attempting to deselect options of non-multiselect (it already didn't have any effect, but was silently ignored).
 
+### Fixed
+- XPath escaping in `select*()` and `deselect*()` methods of `WebDriverSelect`.
+
 ## 1.2.0 - 2016-10-14
 - Added initial support of remote Microsoft Edge browser (but starting local EdgeDriver is still not supported).
 - Utilize late static binding to make eg. `WebDriverBy` and `DesiredCapabilities` classes easily extensible.

lib/Support/XPathEscaper.php

@@ -0,0 +1,45 @@
+<?php
+// Copyright 2004-present Facebook. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Facebook\WebDriver\Support;
+
+class XPathEscaper
+{
+    /**
+     * Converts xpath strings with both quotes and ticks into:
+     *   foo'"bar -> concat('foo', "'" ,'"bar')
+     *
+     * @param string $xpathToEscape The xpath to be converted.
+     * @return string The escaped string.
+     */
+    public static function escapeQuotes($xpathToEscape)
+    {
+        // Single quotes not present => we can quote in them
+        if (strpos($xpathToEscape, "'") === false) {
+            return sprintf("'%s'", $xpathToEscape);
+        }
+
+        // Double quotes not present => we can quote in them
+        if (strpos($xpathToEscape, '"') === false) {
+            return sprintf('"%s"', $xpathToEscape);
+        }
+
+        // Both single and double quotes are present
+        return sprintf(
+            "concat('%s')",
+            str_replace("'", "', \"'\" ,'", $xpathToEscape)
+        );
+    }
+}

lib/WebDriverSelect.php

@@ -18,6 +18,7 @@
 use Facebook\WebDriver\Exception\NoSuchElementException;
 use Facebook\WebDriver\Exception\UnexpectedTagNameException;
 use Facebook\WebDriver\Exception\UnsupportedOperationException;
+use Facebook\WebDriver\Support\XPathEscaper;
 
 /**
  * Models a SELECT tag, providing helper methods to select and deselect options.
@@ -128,7 +129,7 @@ public function selectByIndex($index)
     public function selectByValue($value)
     {
         $matched = false;
-        $xpath = './/option[@value = ' . $this->escapeQuotes($value) . ']';
+        $xpath = './/option[@value = ' . XPathEscaper::escapeQuotes($value) . ']';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
 
         foreach ($options as $option) {
@@ -161,7 +162,7 @@ public function selectByValue($value)
     public function selectByVisibleText($text)
     {
         $matched = false;
-        $xpath = './/option[normalize-space(.) = ' . $this->escapeQuotes($text) . ']';
+        $xpath = './/option[normalize-space(.) = ' . XPathEscaper::escapeQuotes($text) . ']';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
 
         foreach ($options as $option) {
@@ -210,7 +211,7 @@ public function selectByVisibleText($text)
     public function selectByVisiblePartialText($text)
     {
         $matched = false;
-        $xpath = './/option[contains(normalize-space(.), ' . $this->escapeQuotes($text) . ')]';
+        $xpath = './/option[contains(normalize-space(.), ' . XPathEscaper::escapeQuotes($text) . ')]';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
 
         foreach ($options as $option) {
@@ -282,7 +283,7 @@ public function deselectByValue($value)
             throw new UnsupportedOperationException('You may only deselect options of a multi-select');
         }
 
-        $xpath = './/option[@value = ' . $this->escapeQuotes($value) . ']';
+        $xpath = './/option[@value = ' . XPathEscaper::escapeQuotes($value) . ']';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
         foreach ($options as $option) {
             if ($option->isSelected()) {
@@ -306,7 +307,7 @@ public function deselectByVisibleText($text)
             throw new UnsupportedOperationException('You may only deselect options of a multi-select');
         }
 
-        $xpath = './/option[normalize-space(.) = ' . $this->escapeQuotes($text) . ']';
+        $xpath = './/option[normalize-space(.) = ' . XPathEscaper::escapeQuotes($text) . ']';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
         foreach ($options as $option) {
             if ($option->isSelected()) {
@@ -330,44 +331,12 @@ public function deselectByVisiblePartialText($text)
             throw new UnsupportedOperationException('You may only deselect options of a multi-select');
         }
 
-        $xpath = './/option[contains(normalize-space(.), ' . $this->escapeQuotes($text) . ')]';
+        $xpath = './/option[contains(normalize-space(.), ' . XPathEscaper::escapeQuotes($text) . ')]';
         $options = $this->element->findElements(WebDriverBy::xpath($xpath));
         foreach ($options as $option) {
             if ($option->isSelected()) {
                 $option->click();
             }
         }
     }
-
-    /**
-     * Convert strings with both quotes and ticks into:
-     *   foo'"bar -> concat("foo'", '"', "bar")
-     *
-     * @param string $to_escape The string to be converted.
-     * @return string The escaped string.
-     */
-    protected function escapeQuotes($to_escape)
-    {
-        if (strpos($to_escape, '"') !== false && strpos($to_escape, "'") !== false) {
-            $substrings = explode('"', $to_escape);
-
-            $escaped = 'concat(';
-            $first = true;
-            foreach ($substrings as $string) {
-                if (!$first) {
-                    $escaped .= ", '\"',";
-                    $first = false;
-                }
-                $escaped .= '"' . $string . '"';
-            }
-
-            return $escaped;
-        }
-
-        if (strpos($to_escape, '"') !== false) {
-            return sprintf("'%s'", $to_escape);
-        }
-
-        return sprintf('"%s"', $to_escape);
-    }
 }

tests/unit/Support/XPathEscaperTest.php

@@ -0,0 +1,49 @@
+<?php
+// Copyright 2004-present Facebook. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Facebook\WebDriver\Support;
+
+class XPathEscaperTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider xpathProvider
+     * @param string $input
+     * @param string $expectedOutput
+     */
+    public function testShouldInstantiateWithCapabilitiesGivenInConstructor($input, $expectedOutput)
+    {
+        $output = XPathEscaper::escapeQuotes($input);
+
+        $this->assertSame($expectedOutput, $output);
+    }
+
+    /**
+     * @return array[]
+     */
+    public function xpathProvider()
+    {
+        return [
+            'empty string encapsulate in single quotes' => ['', "''"],
+            'string without quotes encapsulate in single quotes' => ['foo bar', "'foo bar'"],
+            'string with single quotes encapsulate in double quotes' => ['foo\'bar\'', '"foo\'bar\'"'],
+            'string with double quotes encapsulate in single quotes' => ['foo"bar"', '\'foo"bar"\''],
+            'string with both types of quotes concatenate' => ['\'"', "concat('', \"'\" ,'\"')"],
+            'string with multiple both types of quotes concatenate' => [
+                'a \'b\'"c"',
+                "concat('a ', \"'\" ,'b', \"'\" ,'\"c\"')",
+            ],
+        ];
+    }
+}