From decc3481323de4f6e4b27d60251a7d848f549bd9 Mon Sep 17 00:00:00 2001 From: rhamaa Date: Thu, 17 Jan 2019 07:19:19 +0800 Subject: [PATCH 1/6] Escape string against shell command inject --- Command.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Command.php b/Command.php index a5a19ee..7fb3e29 100644 --- a/Command.php +++ b/Command.php @@ -223,13 +223,9 @@ public function getCommand(): string /** * Escapes a string in order to inject it in the shell command. */ - public function escape(string $string, bool $addQuotes = true): string + public function escape(string $string, bool $addQuotes = false): string { - $string = str_replace( - ['"', '`', '’', '\\\''], - ['\"', "'", "'", "'"], - trim($string) - ); + $string = escapeshellarg($string); return $addQuotes ? '"'.$string.'"' : $string; } From a6810ab1ed8d09f08fac35c9af340b0875ada4b6 Mon Sep 17 00:00:00 2001 From: Alex Rock Ancelet Date: Thu, 17 Jan 2019 10:35:57 +0100 Subject: [PATCH 2/6] Some minor fixes in Command::text() --- Command.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Command.php b/Command.php index a5a19ee..4ce02ab 100644 --- a/Command.php +++ b/Command.php @@ -451,16 +451,16 @@ public function stroke(string $color): self } /** - * @param string|Geometry $geometry + * @param string|array $options * * @link http://imagemagick.org/script/command-line-options.php#annotate */ - public function text(array $options = []): self + public function text($options = []): self { $mandatoryKeys = ['text', 'geometry', 'textSize']; foreach ($options as $key => $v) { if (!isset($mandatoryKeys[$key])) { - throw new \InvalidArgumentException(\sprintf('Key "%s" is missing for the %s function.', $key, __METHOD__)); + throw new \InvalidArgumentException(\sprintf('Options "%s" is missing for the %s function.', $key, __METHOD__)); } } From 20f171e13c14e7ef105113945cc7d5e01696680d Mon Sep 17 00:00:00 2001 From: rhamaa Date: Thu, 17 Jan 2019 23:50:43 +0800 Subject: [PATCH 3/6] Fix test case --- Command.php | 4 ++-- Tests/CommandTest.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Command.php b/Command.php index 7fb3e29..3377bf7 100644 --- a/Command.php +++ b/Command.php @@ -223,11 +223,11 @@ public function getCommand(): string /** * Escapes a string in order to inject it in the shell command. */ - public function escape(string $string, bool $addQuotes = false): string + public function escape(string $string): string { $string = escapeshellarg($string); - return $addQuotes ? '"'.$string.'"' : $string; + return $string; } /** diff --git a/Tests/CommandTest.php b/Tests/CommandTest.php index 0fa9936..c2efd05 100644 --- a/Tests/CommandTest.php +++ b/Tests/CommandTest.php @@ -197,13 +197,13 @@ public function testInexistingFiles() public function testEscape() { - $string = 'PSR\'s a great `code` style standard. '; + $string = '25% $(touch hacked) #'; $command = new Command(IMAGEMAGICK_DIR); - $escaped = $command->escape($string, true); + $escaped = $command->escape($string); - $this->assertEquals('"PSR\'s a great \'code\' style standard."', $escaped); + $this->assertEquals("'25% $(touch hacked) #'", $escaped); } } From 1dd72d634a208ccbe99e21f6a5d42f23a17c4946 Mon Sep 17 00:00:00 2001 From: rhamaa Date: Fri, 18 Jan 2019 00:29:50 +0800 Subject: [PATCH 4/6] Fix test case 2 --- Tests/CommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/CommandTest.php b/Tests/CommandTest.php index c2efd05..c52d108 100644 --- a/Tests/CommandTest.php +++ b/Tests/CommandTest.php @@ -153,7 +153,7 @@ public function testCommandString($source, $output, $geometry, $quality) $expected = ' '.$command->getExecutable('convert'). ' "'.$source.'"'. - ' -thumbnail "'.$geometry.'"'. + ' -thumbnail \''.$geometry.'\''. ' -quality '.$quality. ' "'.$output.'" '; From dce8f7d9680a29c3ce9e534e301793c6d19b3f8f Mon Sep 17 00:00:00 2001 From: Alex Rock Ancelet Date: Tue, 22 Jan 2019 11:04:53 +0100 Subject: [PATCH 5/6] Make some fixes on tests after last update --- Command.php | 12 ++++++------ Tests/CommandTest.php | 26 ++++++++++---------------- Tests/bootstrap.php | 9 ++++++--- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/Command.php b/Command.php index a759a6b..25e7ef9 100644 --- a/Command.php +++ b/Command.php @@ -82,7 +82,7 @@ public function __construct(string $imageMagickPath = '/usr/bin') } // For imagemagick 7 we'll use the "magick" base command each time. - if (file_exists($imageMagickPath.'/magick') || file_exists($imageMagickPath.'/magick.exe')) { + if (preg_match('~magick(?:\.exe)?$~iU', $imageMagickPath) || file_exists($imageMagickPath.'/magick') || file_exists($imageMagickPath.'/magick.exe')) { $this->version = static::VERSION_7; $imageMagickPath .= '/magick '; } elseif (file_exists($imageMagickPath.'/convert') || file_exists($imageMagickPath.'/convert.exe')) { @@ -289,7 +289,7 @@ public function fill(string $color): self */ public function resize($geometry): self { - $this->command .= ' -resize '.$this->escape($this->ref->geometry($geometry)).' '; + $this->command .= ' -resize "'.$this->ref->geometry($geometry).'" '; return $this; } @@ -301,7 +301,7 @@ public function resize($geometry): self */ public function size($geometry): self { - $this->command .= ' -size '.$this->escape($this->ref->geometry($geometry)).' '; + $this->command .= ' -size "'.$this->ref->geometry($geometry).'" '; return $this; } @@ -327,7 +327,7 @@ public function xc(string $canvasColor = 'none'): self */ public function crop($geometry): self { - $this->command .= ' -crop '.$this->escape($this->ref->geometry($geometry), false).' '; + $this->command .= ' -crop '.$this->ref->geometry($geometry, false).' '; return $this; } @@ -339,7 +339,7 @@ public function crop($geometry): self */ public function extent($geometry): self { - $this->command .= ' -extent '.$this->escape($this->ref->geometry($geometry)); + $this->command .= ' -extent '.$this->ref->geometry($geometry); return $this; } @@ -351,7 +351,7 @@ public function extent($geometry): self */ public function thumbnail($geometry): self { - $this->command .= ' -thumbnail '.$this->escape($this->ref->geometry($geometry)); + $this->command .= ' -thumbnail "'.$this->ref->geometry($geometry).'" '; return $this; } diff --git a/Tests/CommandTest.php b/Tests/CommandTest.php index c52d108..ebae9d6 100644 --- a/Tests/CommandTest.php +++ b/Tests/CommandTest.php @@ -19,28 +19,22 @@ class CommandTest extends AbstractTestCase /** * @dataProvider provideWrongConvertDirs */ - public function testWrongConvertDirs($path, $expectedMessage, $expectedException) + public function testWrongConvertDirs($path, $expectedMessage) { - $exception = ''; - $exceptionClass = ''; - $path = str_replace('\\', '/', $path); $expectedMessage = str_replace('\\', '/', $expectedMessage); - try { - new Command($path); - } catch (\Exception $e) { - $exception = $e->getMessage(); - $exceptionClass = get_class($e); - } - $this->assertContains($expectedMessage, $exception); - $this->assertEquals($exceptionClass, $expectedException); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage($expectedMessage); + + new Command($path); } public function provideWrongConvertDirs() { return array( - array('/this/is/a/dummy/dir', 'The specified path (/this/is/a/dummy/dir) is not a directory', 'InvalidArgumentException'), - array('./', 'The specified path (.) does not seem to contain ImageMagick binaries, or it is not readable', 'InvalidArgumentException'), + array('/this/is/a/dummy/dir', 'The specified path (/this/is/a/dummy/dir) is not a directory'), + array('./', 'The specified path (.) does not seem to contain ImageMagick binaries, or it is not readable'), ); } @@ -153,7 +147,7 @@ public function testCommandString($source, $output, $geometry, $quality) $expected = ' '.$command->getExecutable('convert'). ' "'.$source.'"'. - ' -thumbnail \''.$geometry.'\''. + ' -thumbnail "'.$geometry.'" '. ' -quality '.$quality. ' "'.$output.'" '; @@ -203,7 +197,7 @@ public function testEscape() $escaped = $command->escape($string); - $this->assertEquals("'25% $(touch hacked) #'", $escaped); + $this->assertEquals('"25 $(touch hacked) #"', $escaped); } } diff --git a/Tests/bootstrap.php b/Tests/bootstrap.php index 5b50dbe..d9c06d0 100644 --- a/Tests/bootstrap.php +++ b/Tests/bootstrap.php @@ -30,11 +30,14 @@ '/usr/local/bin/', ]; foreach ($possibleDirectories as $dir) { - $dir = rtrim($dir, '/\\').'/'; - echo 'Check "'.$dir.'convert" binary'."\n"; - exec($dir.'convert -version', $o, $code); + exec($dir.'convert -version 2>&1', $o, $code); + if (0 !== $code) { + echo 'Check "'.$dir.'magick" binary (for ImageMagick 7)'."\n"; + exec($dir.'magick -version 2>&1', $o, $code); + } if (0 === $code) { + echo "Resolved as $dir\n"; define('IMAGEMAGICK_DIR', $dir); break; } From a637e0393c313a53be2180405f31066748c46257 Mon Sep 17 00:00:00 2001 From: Alex Rock Ancelet Date: Tue, 22 Jan 2019 11:05:14 +0100 Subject: [PATCH 6/6] Update phpunit.xml.dist with version 6.5 params instead of 4.1 --- phpunit.xml.dist | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 7891a64..e848b1b 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,20 +1,13 @@ - Tests + Tests @@ -25,16 +18,11 @@ --> - + Command.php CommandOptions.php CommandResponse.php ./ReferenceClasses - - ./build - ./vendor - ./Tests -