MQE-1353 & 1354 #283
MQE-1353 & 1354 #283
Conversation
KevinBKozan
left a comment
KevinBKozan
left a comment
There was a problem hiding this comment.
Minor QA stuff and a fundamental issue with the arguments attribute.
etc/config/command.php
Outdated
| $tokenPassedIn = urldecode($_POST['token']); | ||
| $command = urldecode($_POST['command']); | ||
|
|
||
| if (array_key_exists("arguments", $_POST)) { |
There was a problem hiding this comment.
I don't think that this statement is ever false. Need to add a clause && !empty($_POST['arguments']), or otherwise not post arguments in MagentoWebDriver if they are null.
This is leading to weird behavious below
etc/config/command.php
Outdated
|
|
||
| if (array_key_exists("arguments", $_POST)) { | ||
| $arguments = urldecode($_POST['arguments']); | ||
| $arguments = escapeshellarg(urldecode($_POST['arguments'])); |
There was a problem hiding this comment.
I am not sure we can use escapeshellarg. According to documentation it will always add single quotes to the output, which will likely break existing tests.
Additionally, Devdocs calls out the arguments attribute as Unescaped arguments to be passed in with the CLI command.
By escaping the argument, it breaks functionality. I am not sure what to do here, as I think the token requirement is enough to prevent unwanted commands from running, and the need to be able to pass in an unescaped JSON string is an existing requirement.
There was a problem hiding this comment.
Removed command arguments escaping
# Conflicts: # src/Magento/FunctionalTestingFramework/DataGenerator/Persist/Curl/WebapiExecutor.php
Description
Fixed Issues (if relevant)
Contribution checklist