Commit 42475990 authored by Morris Jobke's avatar Morris Jobke
Browse files

Merge pull request #18445 from owncloud/ext-only-setuservars-string

setUserVars() should only attempt substitution with strings
parents 09808a90 643e3a5b
......@@ -122,7 +122,7 @@ class AmazonS3 extends \OC\Files\Storage\Common {
$params['region'] = empty($params['region']) ? 'eu-west-1' : $params['region'];
$params['hostname'] = empty($params['hostname']) ? 's3.amazonaws.com' : $params['hostname'];
if (!isset($params['port']) || $params['port'] === '') {
$params['port'] = ($params['use_ssl'] === 'false') ? 80 : 443;
$params['port'] = ($params['use_ssl'] === false) ? 80 : 443;
}
$this->params = $params;
}
......@@ -585,7 +585,7 @@ class AmazonS3 extends \OC\Files\Storage\Common {
return $this->connection;
}
$scheme = ($this->params['use_ssl'] === 'false') ? 'http' : 'https';
$scheme = ($this->params['use_ssl'] === false) ? 'http' : 'https';
$base_url = $scheme . '://' . $this->params['hostname'] . ':' . $this->params['port'] . '/';
$this->connection = S3Client::factory(array(
......
......@@ -230,7 +230,9 @@ class OC_Mount_Config {
}
}
} else {
$input = str_replace('$user', $user, $input);
if (is_string($input)) {
$input = str_replace('$user', $user, $input);
}
}
return $input;
}
......
......@@ -154,22 +154,31 @@ class DefinitionParameter implements \JsonSerializable {
/**
* Validate a parameter value against this
* Convert type as necessary
*
* @param mixed $value Value to check
* @return bool success
*/
public function validateValue($value) {
if ($this->getFlags() & self::FLAG_OPTIONAL) {
return true;
}
public function validateValue(&$value) {
$optional = $this->getFlags() & self::FLAG_OPTIONAL;
switch ($this->getType()) {
case self::VALUE_BOOLEAN:
if (!is_bool($value)) {
return false;
switch ($value) {
case 'true':
$value = true;
break;
case 'false':
$value = false;
break;
default:
return false;
}
}
break;
default:
if (empty($value)) {
if (!$value && !$optional) {
return false;
}
break;
......
......@@ -134,12 +134,12 @@ trait FrontendDefinitionTrait {
* @return bool
*/
public function validateStorageDefinition(StorageConfig $storage) {
$options = $storage->getBackendOptions();
foreach ($this->getParameters() as $name => $parameter) {
$value = isset($options[$name]) ? $options[$name] : null;
$value = $storage->getBackendOption($name);
if (!$parameter->validateValue($value)) {
return false;
}
$storage->setBackendOption($name, $value);
}
return true;
}
......
......@@ -45,11 +45,7 @@ class FTP extends \OC\Files\Storage\StreamWrapper{
$this->user=$params['user'];
$this->password=$params['password'];
if (isset($params['secure'])) {
if (is_string($params['secure'])) {
$this->secure = ($params['secure'] === 'true');
} else {
$this->secure = (bool)$params['secure'];
}
$this->secure = $params['secure'];
} else {
$this->secure = false;
}
......
......@@ -39,7 +39,7 @@ class SMB_OC extends SMB {
public function __construct($params) {
if (isset($params['host'])) {
$host = $params['host'];
$this->username_as_share = ($params['username_as_share'] === 'true');
$this->username_as_share = ($params['username_as_share'] === true);
// dummy credentials, unused, to satisfy constructor
$user = 'foo';
......
......@@ -118,6 +118,8 @@ abstract class StoragesService {
$applicableGroups[] = $applicable;
$storageConfig->setApplicableGroups($applicableGroups);
}
return $storageConfig;
}
......@@ -238,6 +240,12 @@ abstract class StoragesService {
$this->setRealStorageIds($storages, $storagesWithConfigHash);
}
// convert parameter values
foreach ($storages as $storage) {
$storage->getBackend()->validateStorageDefinition($storage);
$storage->getAuthMechanism()->validateStorageDefinition($storage);
}
return $storages;
}
......
......@@ -27,7 +27,7 @@
<input type="checkbox"
<?php if (!empty($classes)): ?> class="<?php p(implode(' ', $classes)); ?>"<?php endif; ?>
data-parameter="<?php p($parameter->getName()); ?>"
<?php if ($value == 'true'): ?> checked="checked"<?php endif; ?>
<?php if ($value === true): ?> checked="checked"<?php endif; ?>
/>
<?php p($placeholder); ?>
</label>
......
......@@ -49,6 +49,9 @@ class DefinitionParameterTest extends \Test\TestCase {
[Param::VALUE_BOOLEAN, Param::FLAG_NONE, false, true],
[Param::VALUE_BOOLEAN, Param::FLAG_NONE, 123, false],
// conversion from string to boolean
[Param::VALUE_BOOLEAN, Param::FLAG_NONE, 'false', true, false],
[Param::VALUE_BOOLEAN, Param::FLAG_NONE, 'true', true, true],
[Param::VALUE_PASSWORD, Param::FLAG_NONE, 'foobar', true],
[Param::VALUE_PASSWORD, Param::FLAG_NONE, '', false],
......@@ -60,11 +63,14 @@ class DefinitionParameterTest extends \Test\TestCase {
/**
* @dataProvider validateValueProvider
*/
public function testValidateValue($type, $flags, $value, $success) {
public function testValidateValue($type, $flags, $value, $success, $expectedValue = null) {
$param = new Param('foo', 'bar');
$param->setType($type);
$param->setFlags($flags);
$this->assertEquals($success, $param->validateValue($value));
if (isset($expectedValue)) {
$this->assertEquals($expectedValue, $value);
}
}
}
......@@ -70,9 +70,11 @@ class FrontendDefinitionTraitTest extends \Test\TestCase {
$storageConfig = $this->getMockBuilder('\OCA\Files_External\Lib\StorageConfig')
->disableOriginalConstructor()
->getMock();
$storageConfig->expects($this->once())
->method('getBackendOptions')
->willReturn([]);
$storageConfig->expects($this->any())
->method('getBackendOption')
->willReturn(null);
$storageConfig->expects($this->any())
->method('setBackendOption');
$trait = $this->getMockForTrait('\OCA\Files_External\Lib\FrontendDefinitionTrait');
$trait->setText('test');
......@@ -80,4 +82,35 @@ class FrontendDefinitionTraitTest extends \Test\TestCase {
$this->assertEquals($expectedSuccess, $trait->validateStorageDefinition($storageConfig));
}
public function testValidateStorageSet() {
$param = $this->getMockBuilder('\OCA\Files_External\Lib\DefinitionParameter')
->disableOriginalConstructor()
->getMock();
$param->method('getName')
->willReturn('param');
$param->expects($this->once())
->method('validateValue')
->will($this->returnCallback(function(&$value) {
$value = 'foobar';
return true;
}));
$storageConfig = $this->getMockBuilder('\OCA\Files_External\Lib\StorageConfig')
->disableOriginalConstructor()
->getMock();
$storageConfig->expects($this->once())
->method('getBackendOption')
->with('param')
->willReturn('barfoo');
$storageConfig->expects($this->once())
->method('setBackendOption')
->with('param', 'foobar');
$trait = $this->getMockForTrait('\OCA\Files_External\Lib\FrontendDefinitionTrait');
$trait->setText('test');
$trait->addParameter($param);
$this->assertEquals(true, $trait->validateStorageDefinition($storageConfig));
}
}
......@@ -789,6 +789,13 @@ class GlobalStoragesServiceTest extends StoragesServiceTest {
file_put_contents($configFile, json_encode($json));
$this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB')
->expects($this->exactly(4))
->method('validateStorageDefinition');
$this->backendService->getAuthMechanism('identifier:\Auth\Mechanism')
->expects($this->exactly(4))
->method('validateStorageDefinition');
$allStorages = $this->service->getAllStorages();
$this->assertCount(4, $allStorages);
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment