Skip to content

Commit

Permalink
feat: make search path for BinaryFinder customizable.
Browse files Browse the repository at this point in the history
This feature is important for nextcloud running on
distributions like NixOS, where all the standard
search paths do not exist.

Also added tests.

This fixes issue #43922

Signed-off-by: Reno Reckling <e-github@wthack.de>
  • Loading branch information
exi committed Sep 16, 2024
1 parent b9fb1db commit 2785c56
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 13 deletions.
16 changes: 16 additions & 0 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -2533,4 +2533,20 @@
* Defaults to ``true``
*/
'enable_non-accessible_features' => true,

/**
* Directories where nextcloud looks for binaries.
* This is used to find external binaries like libreoffice, sendmail, ffmpeg and more.
*
* Defaults to ``['/usr/local/sbin','/usr/local/bin','/usr/sbin','/usr/bin','/sbin','/bin','/opt/bin']``
*/
'binary_search_paths' => [
'/usr/local/sbin',
'/usr/local/bin',
'/usr/sbin',
'/usr/bin',
'/sbin',
'/bin',
'/opt/bin',
],
];
31 changes: 18 additions & 13 deletions lib/private/BinaryFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,28 @@
use OCP\IBinaryFinder;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use Symfony\Component\Process\ExecutableFinder;

/**
* Service that find the binary path for a program
*/
class BinaryFinder implements IBinaryFinder {
public const DEFAULT_BINARY_SEARCH_PATHS = [
'/usr/local/sbin',
'/usr/local/bin',
'/usr/sbin',
'/usr/bin',
'/sbin',
'/bin',
'/opt/bin',
];
private ICache $cache;

public function __construct(ICacheFactory $cacheFactory) {
public function __construct(
ICacheFactory $cacheFactory,
private IConfig $config
) {
$this->cache = $cacheFactory->createLocal('findBinaryPath');
}

Expand All @@ -37,18 +50,10 @@ public function findBinaryPath(string $program) {
if (\OCP\Util::isFunctionEnabled('exec')) {
$exeSniffer = new ExecutableFinder();
// Returns null if nothing is found
$result = $exeSniffer->find($program, null, [
'/usr/local/sbin',
'/usr/local/bin',
'/usr/sbin',
'/usr/bin',
'/sbin',
'/bin',
'/opt/bin',
]);
if ($result === null) {
$result = false;
}
$result = $exeSniffer->find(
$program,
false,

Check failure on line 55 in lib/private/BinaryFinder.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidArgument

lib/private/BinaryFinder.php:55:5: InvalidArgument: Argument 2 of Symfony\Component\Process\ExecutableFinder::find cannot be false, null|string value expected (see https://psalm.dev/004)

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 2 of Symfony\Component\Process\ExecutableFinder::find cannot be false, null|string value expected
$this->config->getSystemValue('binary_search_paths', self::DEFAULT_BINARY_SEARCH_PATHS));
}
// store the value for 5 minutes
$this->cache->set($program, $result, 300);
Expand Down
84 changes: 84 additions & 0 deletions tests/lib/BinaryFinderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<?php

declare(strict_types = 1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test;

use OC\BinaryFinder;
use OC\Memcache\ArrayCache;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;

class BinaryFinderTest extends TestCase {
private ICache $cache;
private ICacheFactory $cacheFactory;
private $oldEnv;

protected function setUp(): void {
$this->oldEnv = getenv("PATH");
// BinaryFinder always includes the "PATH" environment variable into the search path,
// which we want to avoid in this test because they are not usually found in webserver
// deployments.
putenv('PATH=""');
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->cache = new ArrayCache();
$this->cacheFactory->method('createLocal')->with('findBinaryPath')->willReturn($this->cache);
}

protected function tearDown(): void {
putenv('PATH='.$this->oldEnv);
}

public function testDefaultFindsCat() {
$config = $this->createMock(IConfig::class);
$config
->method('getSystemValue')
->with('binary_search_paths', $this->anything())
->will($this->returnCallback(function ($key, $default) {
return $default;
}));
$finder = new BinaryFinder($this->cacheFactory, $config);
$this->assertEquals($finder->findBinaryPath('cat'), '/usr/bin/cat');
$this->assertEquals($this->cache->get('cat'), '/usr/bin/cat');
}

public function testDefaultDoesNotFindCata() {
$config = $this->createMock(IConfig::class);
$config
->method('getSystemValue')
->with('binary_search_paths', $this->anything())
->will($this->returnCallback(function ($key, $default) {
return $default;
}));
$finder = new BinaryFinder($this->cacheFactory, $config);
$this->assertFalse($finder->findBinaryPath('cata'));
$this->assertFalse($this->cache->get('cata'));
}

public function testCustomPathFindsCat() {
$config = $this->createMock(IConfig::class);
$config
->method('getSystemValue')
->with('binary_search_paths', $this->anything())
->willReturn(['/usr/bin']);
$finder = new BinaryFinder($this->cacheFactory, $config);
$this->assertEquals($finder->findBinaryPath('cat'), '/usr/bin/cat');
$this->assertEquals($this->cache->get('cat'), '/usr/bin/cat');
}

public function testWrongCustomPathDoesNotFindCat() {
$config = $this->createMock(IConfig::class);
$config
->method('getSystemValue')
->with('binary_search_paths')
->willReturn(['/wrong']);
$finder = new BinaryFinder($this->cacheFactory, $config);
$this->assertFalse($finder->findBinaryPath('cat'));
$this->assertFalse($this->cache->get('cat'));
}
}

0 comments on commit 2785c56

Please sign in to comment.