Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix malformed cmdlines when many options are set #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix malformed cmdlines when many options are set #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
136 changes: 84 additions & 52 deletions lib/PHPExiftool/Writer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ public function erase($boolean, $maintainICCProfile = false)
* @param MetadataBag $metadatas A bag of metadatas
* @param string $destination The output file
*
* @return int the number of file written
* @return int the number "write" operations, or null if exiftool returned nothing we understand
* event for no-op (file unchanged), 1 is returned so the caller does not think the command failed.
*
* @throws InvalidArgumentException
*/
Expand All @@ -160,73 +161,103 @@ public function write($file, MetadataBag $metadatas, $destination = null)
throw new InvalidArgumentException(sprintf('%s does not exists', $file));
}

$command = '';
// if the -o file exists, exiftool prints an error
if($destination) {
@unlink($destination);
if (file_exists($destination)) {
throw new InvalidArgumentException(sprintf('%s cannot be replaced', $destination));
}
}

$common_args = ' -preserve -charset UTF8';
$common_args = '-ignoreMinorErrors -preserve -charset UTF8';

if (count($metadatas)) {
$common_args .= ' -codedcharacterset=utf8';
}
$commands = array();

if ($this->erase) {
/**
* if erase is specfied, we MUST start by erasing datas before doing
* anything else.
*/
if (! $destination) {
$command .= ' -all:all= ' . ($this->eraseProfile ? '' : '--icc_profile:all ') . '' . $file . ' -execute';

/**
* If no destination, all commands will overwrite in place
*/
$common_args .= ' -overwrite_original_in_place';
} else {

/**
* @todo : NO RAW
* If destination was specified, we start by creating the blank
* destination, we will write in it at next step
*/
$command .= ' -all:all= ' . ($this->eraseProfile ? '' : '--icc_profile:all ') . '-o ' . $destination . ' ' . $file . ' -execute';

$file = $destination;
$destination = null;
}
$commands[] = '-all:all= ' . ($this->eraseProfile ? '' : '--icc_profile:all') ;
}

$command .= $this->addMetadatasArg($metadatas);

if ($destination) {
$command .= ' -o ' . escapeshellarg($destination) . ' ' . $file;
} else {
if(count($metadatas) > 0) {
$commands[] = $this->addMetadatasArg($metadatas);
$common_args .= ' -codedcharacterset=utf8';
}

/**
* Preserve the filesystem modification date/time of the original file
* (FileModifyDate) when writing. Note that some filesystems (ie. Mac
* and Windows) store a creation date which is not preserved by this
* option. For these systems, the -overwrite_original_in_place option
* may be used to preserve the creation date.
*/
$command .= ' -overwrite_original_in_place ' . $file;
if ('' !== ($syncCommand = $this->getSyncCommand())) {
$commands[] = $syncCommand;
}

if ('' !== $syncCommand = $this->getSyncCommand()) {
$command .= ' -execute -overwrite_original_in_place ' . $syncCommand . ' ' . $file;
if(count($commands) == 0) {
// nothing to do...
if($destination) {
// ... but a destination
$commands[] = ''; // empty command so exiftool will copy the file for us
}
else {
// really nothing to do = 0 ops
return 1; // considered a "unchnanged"
}
}

$command .= ' -common_args' . $common_args;
if($destination) {
foreach($commands as $i=>$command) {
if($i==0) {
// the FIRST command will -o the destination
$commands[0] .= ' ' . $file . ' -o ' . $destination;
}
else {
// then the next commands will work on the destination
$commands[$i] .= ' -overwrite_original_in_place ' . $destination;
}
}
}
else {
// every command (even a single one) work on the original file
$common_args .= ' -overwrite_original_in_place ' . $file;
}

$lines = explode("\n", $this->exiftool->executeCommand($command));
$lastLine = '';

while ($lines && ! $lastLine) {
$lastLine = array_pop($lines);
if(count($commands) > 1) {
// really need "-common_args" only if many commands are chained
// nb: the file argument CAN be into -common_args
$common_args = '-common_args ' . $common_args;
}

if (preg_match("/(\d+)\ image\ files\ (created|updated)/", $lastLine, $matches)) {
return $matches[1];
$command = join(" -execute ", $commands) . ' ' . $common_args;

$ret = $this->exiftool->executeCommand($command);

// exiftool may print (return) a bunch of lines, even for a single command
// eg. deleting tags of a file with NO tags may return 2 lines...
// | exiftool -all:all= notags.jpg
// | 0 image files updated
// | 1 image files unchanged
// ... which is NOT an error
// so it's not easy to decide from the output when something went REALLY wrong
$n_unchanged = $n_changed = 0;
foreach(explode("\n", $ret) as $line) {
if (preg_match("/(\\d+) image files (copied|created|updated|unchanged)/", $line, $matches)) {
if($matches[2] == 'unchanged') {
$n_unchanged += (int)($matches[1]);
}
else {
$n_changed += (int)($matches[1]);
}
}
}

// first chance, changes happened
if($n_changed > 0) {
// return $n_changed; // nice but breaks backward compatibility
return 1; // better, backward compatible and tests are ok
}
// second chance, at least one no-op happened
if($n_unchanged > 0) {
return 1;
}
// too bad
return null;
}

Expand All @@ -249,15 +280,15 @@ public static function create(LoggerInterface $logger)
*/
protected function addMetadatasArg(MetadataBag $metadatas)
{
$command = ' ';
$command = '';

if ($this->modules & self::MODULE_MWG) {
$command .= ' -use MWG';
$command .= '-use MWG';
}

foreach ($metadatas as $metadata) {
foreach ($metadata->getValue()->asArray() as $value) {
$command .= ' -' . $metadata->getTag()->getTagname() . '='
$command .= ($command ? ' -' : '-') . $metadata->getTag()->getTagname() . '='
. escapeshellarg($value);
}
}
Expand All @@ -284,10 +315,11 @@ protected function getSyncCommand()

foreach ($availableArgs as $arg => $cmd) {
if ($this->mode & $arg) {
$syncCommand .= ' -@ ' . $cmd;
$syncCommand .= ($syncCommand ? ' -@ ' : '-@ ') . $cmd;
}
}

return $syncCommand;
}
}

4 changes: 2 additions & 2 deletions tests/lib/PHPExiftool/Test/AbstractWriterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ public function testAddMetadatasArg()
$this->assertNotContains('@ xmp2iptc', $writer->getSyncCommandTester());

$writer->setModule(WriterTester::MODULE_MWG, true);
$this->assertContains(' -use MWG', $writer->addMetadatasArgTester($metadatas));
$this->assertContains('-use MWG', $writer->addMetadatasArgTester($metadatas));

$writer->setModule(WriterTester::MODULE_MWG, false);
$this->assertNotContains(' -use MWG', $writer->addMetadatasArgTester($metadatas));
$this->assertNotContains('-use MWG', $writer->addMetadatasArgTester($metadatas));

$this->assertRegExp("/\ -XMP-iptcExt:PersonInImage=['\"]Nicolas['\"]/", $writer->addMetadatasArgTester($metadatas));
}
Expand Down