diff --git a/dev/tests/verification/Resources/CharacterReplacementTest.txt b/dev/tests/verification/Resources/CharacterReplacementTest.txt new file mode 100644 index 000000000..13b66cb50 --- /dev/null +++ b/dev/tests/verification/Resources/CharacterReplacementTest.txt @@ -0,0 +1,37 @@ +click("#element .abcdefghijklmnopqrstuvwxyz1234567890"); + $I->click("#element .`~!@#$%^&*()-_=+{}[]|\;:\".,>click("#element .words, and, commas, and, spaces"); + $I->click("#abcdefghijklmnopqrstuvwxyz1234567890 .abcdefghijklmnopqrstuvwxyz1234567890"); + $I->click("#`~!@#$%^&*()-_=+{}[]|\;:\".,>click("#words, and, commas, and, spaces .words, and, commas, and, spaces"); + } +} diff --git a/dev/tests/verification/TestModule/Test/BasicFunctionalTest.xml b/dev/tests/verification/TestModule/Test/BasicFunctionalTest.xml index 70bc46635..a2f0d11b0 100644 --- a/dev/tests/verification/TestModule/Test/BasicFunctionalTest.xml +++ b/dev/tests/verification/TestModule/Test/BasicFunctionalTest.xml @@ -108,6 +108,6 @@ - + \ No newline at end of file diff --git a/dev/tests/verification/TestModule/Test/CharacterReplacementTest.xml b/dev/tests/verification/TestModule/Test/CharacterReplacementTest.xml new file mode 100644 index 000000000..12450ed90 --- /dev/null +++ b/dev/tests/verification/TestModule/Test/CharacterReplacementTest.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + diff --git a/dev/tests/verification/Tests/ReferenceReplacementGenerationTest.php b/dev/tests/verification/Tests/ReferenceReplacementGenerationTest.php index b6ccbdc8f..ef5c0bce6 100644 --- a/dev/tests/verification/Tests/ReferenceReplacementGenerationTest.php +++ b/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"); + } } diff --git a/src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/DataObjectHandler.php b/src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/DataObjectHandler.php index 3bd3cffe3..f639280ec 100644 --- a/src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/DataObjectHandler.php +++ b/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 = []; diff --git a/src/Magento/FunctionalTestingFramework/Page/Handlers/PageObjectHandler.php b/src/Magento/FunctionalTestingFramework/Page/Handlers/PageObjectHandler.php index e5416f749..1985af473 100644 --- a/src/Magento/FunctionalTestingFramework/Page/Handlers/PageObjectHandler.php +++ b/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]; diff --git a/src/Magento/FunctionalTestingFramework/Page/Handlers/SectionObjectHandler.php b/src/Magento/FunctionalTestingFramework/Page/Handlers/SectionObjectHandler.php index b07f9428f..a361579e8 100644 --- a/src/Magento/FunctionalTestingFramework/Page/Handlers/SectionObjectHandler.php +++ b/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); diff --git a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php b/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php index 6b97c77f5..5595f5b16 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Objects/ActionObject.php +++ b/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,7 +343,7 @@ 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 @@ -351,10 +351,29 @@ private function stripAndSplitReference($reference) */ 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; } diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php index ae1f65280..4b3419660 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/ActionObjectExtractor.php +++ b/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, diff --git a/src/Magento/FunctionalTestingFramework/Test/Util/TestObjectExtractor.php b/src/Magento/FunctionalTestingFramework/Test/Util/TestObjectExtractor.php index 0ec3fd0dc..d9044a0a1 100644 --- a/src/Magento/FunctionalTestingFramework/Test/Util/TestObjectExtractor.php +++ b/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); + } } }