Skip to content

Commit

Permalink
Fix invalid files being listed in "log" command (#1361)
Browse files Browse the repository at this point in the history
Ignores output from other processes during the SSH connection
  • Loading branch information
pjcdawkins authored Dec 12, 2023
1 parent 7111185 commit 5a0eb35
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 24 deletions.
8 changes: 6 additions & 2 deletions src/Command/Environment/EnvironmentLogCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use Platformsh\Cli\Command\CommandBase;
use Platformsh\Cli\Util\OsUtil;
use Platformsh\Cli\Util\StringUtil;
use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface;
use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext;
use Symfony\Component\Console\Exception\InvalidArgumentException;
Expand Down Expand Up @@ -73,7 +74,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
/** @var \Doctrine\Common\Cache\CacheProvider $cache */
$cache = $this->getService('cache');
if (!$result = $cache->fetch($cacheKey)) {
$result = $host->runCommand('ls -1 ' . $logDir . '/*.log');
$result = $host->runCommand('echo -n _BEGIN_FILE_LIST_; ls -1 ' . $logDir . '/*.log; echo -n _END_FILE_LIST_');
if (is_string($result)) {
$result = trim(StringUtil::between($result, '_BEGIN_FILE_LIST_', '_END_FILE_LIST_'));
}

// Cache the list for 1 day.
$cache->save($cacheKey, $result, 86400);
Expand All @@ -84,7 +88,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$logDir . '/access.log',
$logDir . '/error.log',
];
$files = $result ? explode("\n", $result) : $defaultFiles;
$files = $result && is_string($result) ? explode("\n", $result) : $defaultFiles;

// Ask the user to choose a file.
$files = array_combine($files, array_map(function ($file) {
Expand Down
24 changes: 2 additions & 22 deletions src/Service/RemoteEnvVars.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Doctrine\Common\Cache\CacheProvider;
use Platformsh\Cli\Model\Host\HostInterface;
use Platformsh\Cli\Model\Host\LocalHost;
use Platformsh\Cli\Util\StringUtil;

/**
* A service for reading environment variables on a host.
Expand Down Expand Up @@ -61,7 +62,7 @@ public function getEnvVar($variable, HostInterface $host, $refresh = false, $ttl
$data = $this->cache->fetch($cacheKey);
if ($refresh || $data === false || $data['last_changed'] !== $host->lastChanged()) {
$output = $host->runCommand(\sprintf('echo -n \'%s\'"$%s"\'%s\'', $begin, $varName, $end));
$value = $this->extractResult($output, $begin, $end);
$value = StringUtil::between((string) $output, $begin, $end);
$data = ['last_changed' => $host->lastChanged(), 'value' => $value];
$this->cache->save($cacheKey, $data, $ttl);
} else {
Expand All @@ -71,27 +72,6 @@ public function getEnvVar($variable, HostInterface $host, $refresh = false, $ttl
return $value ?: '';
}

/**
* Extracts a result from 'echo' output between beginning and ending delimiters.
*
* @param string $output
* @param string $begin
* @param string $end
*
* @return string
*/
private function extractResult($output, $begin, $end)
{
$first = \strpos($output, $begin);
$last = \strrpos($output, $end, $first);
if ($first === false || $last === false) {
return $output;
}
$offset = $first + \strlen($begin);
$length = $last - $first - \strlen($begin);
return \substr($output, $offset, $length);
}

/**
* Read a complex environment variable (an associative array) from the application.
*
Expand Down
28 changes: 28 additions & 0 deletions src/Util/StringUtil.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace Platformsh\Cli\Util;

class StringUtil
{
/**
* Finds a substring between two delimiters.
*
* @param string $str
* @param string $begin
* @param string $end
*
* @return string|null
* The substring, or null if the delimiters are not found.
*/
public static function between($str, $begin, $end)
{
$first = \strpos($str, $begin);
$last = \strrpos($str, $end, $first);
if ($first === false || $last === false) {
return null;
}
$offset = $first + \strlen($begin);
$length = $last - $first - \strlen($begin);
return \substr($str, $offset, $length);
}
}
24 changes: 24 additions & 0 deletions tests/Util/StringUtilTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Platformsh\Cli\Tests\Util;

use Platformsh\Cli\Util\StringUtil;

class StringUtilTest extends \PHPUnit_Framework_TestCase
{
public function testBetween()
{
$cases = [
['_BEGIN_foo_END_', '_BEGIN_', '_END_', 'foo'],
['_BEGIN_foo bar', '_BEGIN_', '_END_', null],
['_BEGIN_foo bar_END_', '_BEGIN_', '_END_', 'foo bar'],
["_BEGIN_\nfoo\n_END_", '_BEGIN_', '_END_', "\nfoo\n"],
["_BEGIN_\nfoo\n_END_", "_BEGIN_\n", '_END_', "foo\n"],
["_BEGIN_\nfoo\n_END_", "_BEGIN_\n", "\n_END_", 'foo'],
];
foreach ($cases as $key => $case) {
list($str, $begin, $end, $result) = $case;
$this->assertEquals($result, StringUtil::between($str, $begin, $end), "case $key");
}
}
}

0 comments on commit 5a0eb35

Please sign in to comment.