Skip to content

Commit

Permalink
Improve escaping for WIndows (#24)
Browse files Browse the repository at this point in the history 
* os detection

* Fix tests

* fix Line indented incorrectly

* Improve json output, but I would prefer json file input

* typo

* Add default value and comments

* Windows only reverts.

* codestyle & wusa.exe is required
  • Loading branch information
@ms-slasher13 @greg-1-anderson
ms-slasher13 authored and greg-1-anderson committed Feb 13, 2019
1 parent c698fce commit a8403c6
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 11 deletions.
1 change: 1 addition & 0 deletions appveyor.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ install:
- SET PATH=%APPVEYOR_BUILD_FOLDER%/DevDesktopCommon/bintools-win/msys/bin;%PATH%
- SET PATH=C:\Program Files\MySql\MySQL Server 5.7\bin\;%PATH%
#Install PHP per https://blog.wyrihaximus.net/2016/11/running-php-unit-tests-on-windows-using-appveyor-and-chocolatey/
- ps: Set-Service wuauserv -StartupType Manual
- ps: appveyor-retry cinst --ignore-checksums -y php --version ((choco search php --exact --all-versions -r | select-string -pattern $Env:php_ver_target | Select-Object -first 1) -replace '[php|]','')
- cd c:\tools\php71
- copy php.ini-production php.ini
Expand Down
8 changes: 8 additions & 0 deletions src/ProcessBase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Symfony\Component\Console\Style\OutputStyle;
use Symfony\Component\Process\Process;
use Consolidation\SiteProcess\Util\RealtimeOutputHandler;
use Consolidation\SiteProcess\Util\Escape;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Output\ConsoleOutputInterface;

Expand Down Expand Up @@ -172,6 +173,13 @@ public function getOutputAsJson()
}
$output = preg_replace('#^[^{]*#', '', $output);
$output = preg_replace('#[^}]*$#', '', $output);
if (Escape::isWindows()) {
// Doubled double quotes were converted to \\".
// Revert to double quote.
$output = str_replace('\\"', '"', $output);
// Revert of doubled backslashes.
$output = preg_replace('#\\\\{2}#', '\\', $output);
}
if (!$json = json_decode($output, true)) {
throw new \InvalidArgumentException('Unable to decode output into JSON.');
}
Expand Down
28 changes: 20 additions & 8 deletions src/Util/Escape.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,16 @@ public static function shellArg($arg, $os = null)

/**
* isWindows determines whether the provided OS is Windows.
*
* @param string|null $os The OS to escape for.
*
* @return boolean
*/
public static function isWindows($os)
public static function isWindows($os = null)
{
// In most cases, $os will be NULL and PHP_OS will be returned. However,
// if an OS is specified in $os, return that instead.
$os = $os ?: PHP_OS;
return strtoupper(substr($os, 0, 3)) === 'WIN';
}

Expand Down Expand Up @@ -113,18 +120,23 @@ public static function linuxArg($arg)
*/
public static function windowsArg($arg)
{
if ('' === $arg || null === $arg) {
return '""';
}
if (false !== strpos($arg, "\0")) {
$arg = str_replace("\0", '?', $arg);
}
if (!preg_match('/[\/()%!^"<>&|\s]/', $arg)) {
return $arg;
}
// Double up existing backslashes
$arg = preg_replace('/\\\/', '\\\\\\\\', $arg);

// Double up double quotes
$arg = preg_replace('/"/', '""', $arg);

// Double up percents.
// $arg = preg_replace('/%/', '%%', $arg);
$arg = preg_replace('/(\\\\+)$/', '$1$1', $arg);

// Replacing whitespace for good measure (see comment above).
$arg = str_replace(["\t", "\n", "\r", "\0", "\x0B"], ' ', $arg);

$arg = str_replace(['"', '^', '%', '!'], ['""', '"^^"', '"^%"', '"^!"'], $arg);

// Add surrounding quotes.
$arg = '"' . $arg . '"';

Expand Down
16 changes: 15 additions & 1 deletion tests/RealtimeOutputHandlerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Consolidation\SiteProcess;

use Consolidation\SiteProcess\Util\Escape;
use PHPUnit\Framework\TestCase;
use Consolidation\SiteProcess\Util\ArgumentProcessor;
use Consolidation\SiteAlias\AliasRecord;
Expand All @@ -21,18 +22,28 @@ public function realtimeOutputHandlerTestValues()
'hello, world',
'',
['echo', 'hello, world'],
'LINUX',
],

[
'"hello, world"',
'',
['echo', 'hello, world'],
'WIN'
],

[
'README.md',
'',
['ls', 'README.md'],
'LINUX',
],

[
'',
'no/such/file: No such file or directory',
['ls', 'no/such/file'],
'LINUX',
],
];
}
Expand All @@ -42,8 +53,11 @@ public function realtimeOutputHandlerTestValues()
*
* @dataProvider realtimeOutputHandlerTestValues
*/
public function testRealtimeOutputHandler($expectedStdout, $expectedStderr, $args)
public function testRealtimeOutputHandler($expectedStdout, $expectedStderr, $args, $os)
{
if (Escape::isWindows() != Escape::isWindows($os)) {
$this->markTestSkipped("OS isn't supported");
}
$stdin = new ArrayInput([]);
$stdout = new BufferedOutput();
$stderr = new BufferedOutput();
Expand Down
50 changes: 48 additions & 2 deletions tests/SiteProcessTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PHPUnit\Framework\TestCase;
use Consolidation\SiteProcess\Util\ArgumentProcessor;
use Consolidation\SiteProcess\Util\Escape;
use Consolidation\SiteAlias\AliasRecord;

class SiteProcessTest extends TestCase
Expand All @@ -22,6 +23,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -32,6 +34,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -42,6 +45,7 @@ public function siteProcessTestValues()
['ls', '-al', '/path1', '/path2'],
[],
[],
NULL,
],

[
Expand All @@ -52,6 +56,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -62,6 +67,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -72,6 +78,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -82,6 +89,7 @@ public function siteProcessTestValues()
['ls', '-al'],
[],
[],
NULL,
],

[
Expand All @@ -92,6 +100,7 @@ public function siteProcessTestValues()
['ls', '-al', '/path1', '/path2'],
[],
[],
NULL,
],

[
Expand All @@ -102,6 +111,7 @@ public function siteProcessTestValues()
['ls', '-al', '/path1', '/path2'],
[],
[],
NULL,
],

[
Expand All @@ -112,6 +122,7 @@ public function siteProcessTestValues()
['ls', '-al', '/path1', '/path2'],
[],
[],
NULL,
],

[
Expand All @@ -122,6 +133,18 @@ public function siteProcessTestValues()
['drush', 'status'],
['fields' => 'root,uri'],
[],
'LINUX',
],

[
'drush status --fields=root,uri',
false,
false,
[],
['drush', 'status'],
['fields' => 'root,uri'],
[],
'WIN',
],

[
Expand All @@ -132,6 +155,7 @@ public function siteProcessTestValues()
['drush', 'rsync', 'a', 'b',],
[],
['exclude' => 'vendor'],
NULL,
],

[
Expand All @@ -142,6 +166,7 @@ public function siteProcessTestValues()
['drush', 'rsync', 'a', 'b', '--', '--include=vendor/autoload.php'],
[],
['exclude' => 'vendor'],
NULL,
],
];
}
Expand All @@ -158,8 +183,15 @@ public function testSiteProcess(
$siteAliasData,
$args,
$options,
$optionsPassedAsArgs)
$optionsPassedAsArgs,
$os)
{
if (Escape::isWindows() != Escape::isWindows($os)) {
$this->markTestSkipped("OS isn't supported");
}
if ($useTty && Escape::isWindows($os)) {
$this->markTestSkipped('Windows doesn\'t have /dev/tty support');
}
$processManager = ProcessManager::createDefault();
$siteAlias = new AliasRecord($siteAliasData, '@alias.dev');
$siteProcess = $processManager->siteProcess($siteAlias, $args, $options, $optionsPassedAsArgs);
Expand All @@ -184,18 +216,27 @@ public function siteProcessJsonTestValues()
[
'Output is empty.',
'',
'LINUX',
],
[
'Unable to decode output into JSON.',
'No json data here',
NULL,
],
[
'{"foo":"bar"}',
'{"foo":"bar"}',
NULL,
],
[
'{"foo":"b\'ar"}',
'{"foo":"b\'ar"}',
NULL,
],
[
'{"foo":"bar"}',
'Ignored leading data {"foo":"bar"} Ignored trailing data',
NULL,
],
];
}
Expand All @@ -207,11 +248,16 @@ public function siteProcessJsonTestValues()
*/
public function testSiteProcessJson(
$expected,
$data)
$data,
$os)
{
if (Escape::isWindows() != Escape::isWindows($os)) {
$this->markTestSkipped("OS isn't supported");
}
$args = ['echo', $data];
$processManager = ProcessManager::createDefault();
$siteAlias = new AliasRecord([], '@alias.dev');
$siteAlias->set('os', $os);
$siteProcess = $processManager->siteProcess($siteAlias, $args);
$siteProcess->mustRun();

Expand Down

0 comments on commit a8403c6

Please sign in to comment.