MQE-802: Make ActionObject list of allowed characters comprehensive

dev/tests/verification/Resources/CharacterReplacementTest.txt

@@ -0,0 +1,37 @@
+<?php
+namespace Magento\AcceptanceTest\_generated\Backend;
+
+use Magento\FunctionalTestingFramework\AcceptanceTester;
+use Magento\FunctionalTestingFramework\DataGenerator\Handlers\DataObjectHandler;
+use Magento\FunctionalTestingFramework\DataGenerator\Persist\DataPersistenceHandler;
+use Magento\FunctionalTestingFramework\DataGenerator\Objects\EntityDataObject;
+use \Codeception\Util\Locator;
+use Yandex\Allure\Adapter\Annotation\Features;
+use Yandex\Allure\Adapter\Annotation\Stories;
+use Yandex\Allure\Adapter\Annotation\Title;
+use Yandex\Allure\Adapter\Annotation\Description;
+use Yandex\Allure\Adapter\Annotation\Parameter;
+use Yandex\Allure\Adapter\Annotation\Severity;
+use Yandex\Allure\Adapter\Model\SeverityLevel;
+use Yandex\Allure\Adapter\Annotation\TestCaseId;
+
+/**
+ */
+class CharacterReplacementTestCest
+{
+	/**
+	 * @Parameter(name = "AcceptanceTester", value="$I")
+	 * @param AcceptanceTester $I
+	 * @return void
+	 * @throws \Exception
+	 */
+	public function CharacterReplacementTest(AcceptanceTester $I)
+	{
+		$I->click("#element .abcdefghijklmnopqrstuvwxyz1234567890");
+		$I->click("#element .`~!@#$%^&*()-_=+{}[]|\;:\".,></?()3., ");
+		$I->click("#element .words, and, commas, and, spaces");
+		$I->click("#abcdefghijklmnopqrstuvwxyz1234567890 .abcdefghijklmnopqrstuvwxyz1234567890");
+		$I->click("#`~!@#$%^&*()-_=+{}[]|\;:\".,></?() .`~!@#$%^&*()-_=+{}[]|\;:\".,></?()");
+		$I->click("#words, and, commas, and, spaces .words, and, commas, and, spaces");
+	}
+}

dev/tests/verification/TestModule/Test/BasicFunctionalTest.xml

@@ -108,6 +108,6 @@
         <waitForElementNotVisible selector=".functionalTestSelector" time="30" stepKey="waitForElementNotVisibleKey1" />
         <waitForElementVisible selector=".functionalTestSelector" time="30" stepKey="waitForElementVisibleKey1" />
         <waitForJS function="someJsFunction" time="30" stepKey="waitForJSKey1" />
-        <waitForText selector=".functionalTestSelector" userInput="someInput" time="30" stepKey=""/>
+        <waitForText selector=".functionalTestSelector" userInput="someInput" time="30" stepKey="waitForText1"/>
     </test>
 </tests>

dev/tests/verification/TestModule/Test/CharacterReplacementTest.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+
+<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:noNamespaceSchemaLocation="../../../../../src/Magento/FunctionalTestingFramework/Test/etc/testSchema.xsd">
+    <test name="CharacterReplacementTest">
+        <click stepKey="allChars1" selector="{{SampleSection.oneParamElement('abcdefghijklmnopqrstuvwxyz1234567890')}}"/>
+        <click stepKey="allChars2" selector="{{SampleSection.oneParamElement('`~!@#$%^&amp;*()-_=+{}[]|\;:&quot;.,&gt;&lt;/?()$213., ')}}"/>
+        <click stepKey="allChars3" selector="{{SampleSection.oneParamElement('words, and, commas, and, spaces')}}"/>
+        <click stepKey="allChars4" selector="{{SampleSection.twoParamElement('abcdefghijklmnopqrstuvwxyz1234567890','abcdefghijklmnopqrstuvwxyz1234567890')}}"/>
+        <click stepKey="allChars5" selector="{{SampleSection.twoParamElement('`~!@#$%^&amp;*()-_=+{}[]|\;:&quot;.,&gt;&lt;/?()','`~!@#$%^&amp;*()-_=+{}[]|\;:&quot;.,&gt;&lt;/?()')}}"/>
+        <click stepKey="allChars6" selector="{{SampleSection.twoParamElement('words, and, commas, and, spaces', 'words, and, commas, and, spaces')}}"/>
+    </test>
+</tests>

dev/tests/verification/Tests/ReferenceReplacementGenerationTest.php

@@ -69,4 +69,13 @@ public function testSectionReferenceReplacementCest()
     {
         $this->generateAndCompareTest(self::SECTION_REPLACEMENT_TEST);
     }
+
+    /**
+     * Tests replacement of all characters into string literal references.
+     * Used to ensure users can input everything but single quotes into 'stringLiteral' in parameterized selectors
+     */
+    public function testCharacterReplacementCest()
+    {
+        $this->generateAndCompareTest("CharacterReplacementTest");
+    }
 }

src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/DataObjectHandler.php

@@ -29,6 +29,7 @@ class DataObjectHandler implements ObjectHandlerInterface
     const _ENTITY_KEY = 'entityKey';
     const _SEPARATOR = '->';
     const _REQUIRED_ENTITY = 'requiredEntity';
+    const DATA_NAME_ERROR_MSG = "Entity names cannot contain non alphanumeric characters.\tData='%s'";
 
     /**
      * The singleton instance of this class
@@ -110,6 +111,10 @@ private function processParserOutput($parserOutput)
         $rawEntities = $parserOutput[self::_ENTITY];
 
         foreach ($rawEntities as $name => $rawEntity) {
+            if (preg_match('/[^a-zA-Z0-9_]/', $name)) {
+                throw new XmlException(sprintf(self::DATA_NAME_ERROR_MSG, $name));
+            }
+
             $type = $rawEntity[self::_TYPE];
             $data = [];
             $linkedEntities = [];

src/Magento/FunctionalTestingFramework/Page/Handlers/PageObjectHandler.php

@@ -10,6 +10,7 @@
 use Magento\FunctionalTestingFramework\ObjectManagerFactory;
 use Magento\FunctionalTestingFramework\Page\Objects\PageObject;
 use Magento\FunctionalTestingFramework\XmlParser\PageParser;
+use Magento\FunctionalTestingFramework\Exceptions\XmlException;
 
 class PageObjectHandler implements ObjectHandlerInterface
 {
@@ -19,6 +20,7 @@ class PageObjectHandler implements ObjectHandlerInterface
     const MODULE = 'module';
     const PARAMETERIZED = 'parameterized';
     const AREA = 'area';
+    const NAME_BLACKLIST_ERROR_MSG = "Page names cannot contain non alphanumeric characters.\tPage='%s'";
 
     /**
      * The singleton instance of this class
@@ -49,6 +51,9 @@ private function __construct()
         }
 
         foreach ($parserOutput as $pageName => $pageData) {
+            if (preg_match('/[^a-zA-Z0-9_]/', $pageName)) {
+                throw new XmlException(sprintf(self::NAME_BLACKLIST_ERROR_MSG, $pageName));
+            }
             $area = $pageData[self::AREA];
             $url = $pageData[self::URL];
 

src/Magento/FunctionalTestingFramework/Page/Handlers/SectionObjectHandler.php

@@ -11,6 +11,7 @@
 use Magento\FunctionalTestingFramework\Page\Objects\ElementObject;
 use Magento\FunctionalTestingFramework\Page\Objects\SectionObject;
 use Magento\FunctionalTestingFramework\XmlParser\SectionParser;
+use Magento\FunctionalTestingFramework\Exceptions\XmlException;
 
 class SectionObjectHandler implements ObjectHandlerInterface
 {
@@ -21,6 +22,8 @@ class SectionObjectHandler implements ObjectHandlerInterface
     const LOCATOR_FUNCTION = 'locatorFunction';
     const TIMEOUT = 'timeout';
     const PARAMETERIZED = 'parameterized';
+    const SECTION_NAME_ERROR_MSG = "Section names cannot contain non alphanumeric characters.\tSection='%s'";
+    const ELEMENT_NAME_ERROR_MSG = "Element names cannot contain non alphanumeric characters.\tElement='%s'";
 
     /**
      * Singleton instance of this class
@@ -54,21 +57,32 @@ private function __construct()
         foreach ($parserOutput as $sectionName => $sectionData) {
             $elements = [];
 
-            foreach ($sectionData[SectionObjectHandler::ELEMENT] as $elementName => $elementData) {
-                $elementType = $elementData[SectionObjectHandler::TYPE];
-                $elementSelector = $elementData[SectionObjectHandler::SELECTOR] ?? null;
-                $elementLocatorFunc = $elementData[SectionObjectHandler::LOCATOR_FUNCTION] ?? null;
-                $elementTimeout = $elementData[SectionObjectHandler::TIMEOUT] ?? null;
-                $elementParameterized = $elementData[SectionObjectHandler::PARAMETERIZED] ?? false;
-
-                $elements[$elementName] = new ElementObject(
-                    $elementName,
-                    $elementType,
-                    $elementSelector,
-                    $elementLocatorFunc,
-                    $elementTimeout,
-                    $elementParameterized
-                );
+            if (preg_match('/[^a-zA-Z0-9_]/', $sectionName)) {
+                throw new XmlException(sprintf(self::SECTION_NAME_ERROR_MSG, $sectionName));
+            }
+
+            try {
+                foreach ($sectionData[SectionObjectHandler::ELEMENT] as $elementName => $elementData) {
+                    if (preg_match('/[^a-zA-Z0-9_]/', $elementName)) {
+                        throw new XmlException(sprintf(self::ELEMENT_NAME_ERROR_MSG, $elementName, $sectionName));
+                    }
+                    $elementType = $elementData[SectionObjectHandler::TYPE];
+                    $elementSelector = $elementData[SectionObjectHandler::SELECTOR] ?? null;
+                    $elementLocatorFunc = $elementData[SectionObjectHandler::LOCATOR_FUNCTION] ?? null;
+                    $elementTimeout = $elementData[SectionObjectHandler::TIMEOUT] ?? null;
+                    $elementParameterized = $elementData[SectionObjectHandler::PARAMETERIZED] ?? false;
+
+                    $elements[$elementName] = new ElementObject(
+                        $elementName,
+                        $elementType,
+                        $elementSelector,
+                        $elementLocatorFunc,
+                        $elementTimeout,
+                        $elementParameterized
+                    );
+                }
+            } catch (XmlException $exception) {
+                throw new XmlException($exception->getMessage() . " in Section '{$sectionName}'");
             }
 
             $this->sectionObjects[$sectionName] = new SectionObject($sectionName, $elements);

src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php

@@ -32,8 +32,8 @@ class ActionObject
     const ACTION_ATTRIBUTE_URL = 'url';
     const ACTION_ATTRIBUTE_SELECTOR = 'selector';
     const ACTION_ATTRIBUTE_VARIABLE_REGEX_PARAMETER = '/\(.+\)/';
-    const ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN = '/{{[\w.\[\]()\',$ ]+}}/';
-    const ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN_WITH_PARAMS= '/{{[\w]+\.[\w]+\([\w ,.\'{$}]+\)}}/';
+    const ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN = '/{{[\w]+\.[\w]+}}/';
+    const ACTION_ATTRIBUTE_VARIABLE_REGEX_PATTERN_WITH_PARAMS= '/{{[\w]+\.[\w]+\(.+\)}}/';
 
     /**
      * The unique identifier for the action
@@ -343,18 +343,37 @@ private function stripAndSplitReference($reference)
     }
 
     /**
-     * Returns an array containing all parameters found inside () block of test input.
+     * Returns an array containing all parameters found inside () block of test input. Expected delimiter is ', '.
      * Returns null if no parameters were found.
      *
      * @param string $reference
      * @return array|null
      */
     private function stripAndReturnParameters($reference)
     {
+        // 'string', or 'string,!@#$%^&*()_+, '
+        $literalParametersRegex = "/'[^']+'/";
+        $postCleanupDelimiter = "::::";
+
         preg_match(ActionObject::ACTION_ATTRIBUTE_VARIABLE_REGEX_PARAMETER, $reference, $matches);
         if (!empty($matches)) {
-            $strippedReference = str_replace(')', '', str_replace('(', '', $matches[0]));
-            return explode(',', $strippedReference);
+            $strippedReference = ltrim(rtrim($matches[0], ")"), "(");
+
+            // Pull out all 'string' references, as they can contain 'string with , comma in it'
+            preg_match_all($literalParametersRegex, $strippedReference, $literalReferences);
+            $strippedReference = preg_replace($literalParametersRegex, '&&stringReference&&', $strippedReference);
+
+            // Sanitize 'string, data.field,$persisted.field$' => 'string::::data.field::::$persisted.field$'
+            $strippedReference = preg_replace('/,/', ', ', $strippedReference);
+            $strippedReference = str_replace(',', $postCleanupDelimiter, $strippedReference);
+            $strippedReference = str_replace(' ', '', $strippedReference);
+
+            // Replace string references into string, needed to keep sequence
+            foreach ($literalReferences[0] as $key => $value) {
+                $strippedReference = preg_replace('/&&stringReference&&/', $value, $strippedReference, 1);
+            }
+
+            return explode($postCleanupDelimiter, $strippedReference);
         }
         return null;
     }

src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php

@@ -21,7 +21,8 @@ class ActionObjectExtractor extends BaseObjectExtractor
     const ACTION_GROUP_REF = 'ref';
     const ACTION_GROUP_ARGUMENTS = 'arguments';
     const ACTION_GROUP_ARG_VALUE = 'value';
-    const BEFORE_AFTER_ERROR_MSG = "Merge Error - Steps cannot have both before and after attributes.\tTestStep='%s'";
+    const BEFORE_AFTER_ERROR_MSG = "Merge Error - Steps cannot have both before and after attributes.\tStepKey='%s'";
+    const STEP_KEY_BLACKLIST_ERROR_MSG = "StepKeys cannot contain non alphanumeric characters.\tStepKey='%s'";
 
     /**
      * ActionObjectExtractor constructor.
@@ -46,6 +47,10 @@ public function extractActions($testActions)
         foreach ($testActions as $actionName => $actionData) {
             $stepKey = $actionData[self::TEST_STEP_MERGE_KEY];
 
+            if (preg_match('/[^a-zA-Z0-9_]/', $stepKey)) {
+                throw new XmlException(sprintf(self::STEP_KEY_BLACKLIST_ERROR_MSG, $actionName));
+            }
+
             $actionAttributes = $this->stripDescriptorTags(
                 $actionData,
                 self::TEST_STEP_MERGE_KEY,

src/Magento/FunctionalTestingFramework/Test/Util/TestObjectExtractor.php

@@ -6,6 +6,7 @@
 
 namespace Magento\FunctionalTestingFramework\Test\Util;
 
+use Magento\FunctionalTestingFramework\Exceptions\XmlException;
 use Magento\FunctionalTestingFramework\Test\Objects\TestObject;
 
 /**
@@ -104,12 +105,16 @@ public function extractTestData($testData)
         }
 
         // TODO extract filename info and store
-        return new TestObject(
-            $testData[self::NAME],
-            $this->actionObjectExtractor->extractActions($testActions),
-            $testAnnotations,
-            $testHooks,
-            $filename
-        );
+        try {
+            return new TestObject(
+                $testData[self::NAME],
+                $this->actionObjectExtractor->extractActions($testActions),
+                $testAnnotations,
+                $testHooks,
+                $filename
+            );
+        } catch (XmlException $exception) {
+            throw new XmlException($exception->getMessage() . ' in Test ' . $filename);
+        }
     }
 }