Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

shell: Fix autodetection of firejail in findFirejail()

Browse source 
Fixes a regression from 3f94708eff. ExecutableFinder returns false if
the executable isn't found, but CommandFactory was expecting a null response,
This caused autodetection to always think firejail was present.

Adjust CommandFactoryTest to ensure we're always passing a string to
FirejailCommand. We need to switch findFirejail to protected so it can
be mocked.

Bug: T257282
Change-Id: Ie73418ebef6dce2bd5ec18fa38e29219d5bb2fd6
This commit is contained in:
Kunal Mehta 2020-07-06 23:53:53 -07:00
parent 3dc37a14af
commit b3f54db14c
2 changed files with 9 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
includes/shell/CommandFactory.php
View file

@ -72,9 +72,10 @@ class CommandFactory {
$this->setLogger( new NullLogger() );
}
private function findFirejail(): ?string {
protected function findFirejail(): ?string {
if ( $this->firejail === null ) {
$this->firejail = ExecutableFinder::findInDefaultPaths( 'firejail' );
// Convert a `false` from ExecutableFinder to `null` (T257282)
$this->firejail = ExecutableFinder::findInDefaultPaths( 'firejail' ) ?: null;
}
return $this->firejail;

9
tests/phpunit/unit/includes/shell/CommandFactoryTest.php
View file

@ -41,8 +41,11 @@ class CommandFactoryTest extends MediaWikiUnitTestCase {
* @covers MediaWiki\Shell\CommandFactory::create
*/
public function testFirejailCreate() {
$factory = new CommandFactory( [], false, 'firejail' );
$factory->setLogger( new NullLogger() );
$this->assertInstanceOf( FirejailCommand::class, $factory->create() );
$mock = $this->getMockBuilder( CommandFactory::class )
->setConstructorArgs( [ [], false, 'firejail' ] )
->setMethods( [ 'findFirejail' ] )
->getMock();
$mock->method( 'findFirejail' )->willReturn( '/usr/bin/firejail' );
$this->assertInstanceOf( FirejailCommand::class, $mock->create() );
}
}