Hardening save method in BaseURL

diff --git a/src/Util/BaseURL.php b/src/Util/BaseURL.php
index 66526ff2dbb10d773b2641beb4673a77a0801a36..cd703a91776be65f273af0faa772b05472931915 100644 (file)
--- a/src/Util/BaseURL.php
+++ b/src/Util/BaseURL.php
@@ -137,35 +137,54 @@ class BaseURL
         */
        public function save($hostname = null, $sslPolicy = null, $urlPath = null)
        {
-               $success = true;
+               $currHostname  = $this->hostname;
+               $currSSLPolicy = $this->sslPolicy;
+               $currURLPath   = $this->urlPath;
 
                if (!empty($hostname) && $hostname !== $this->hostname) {
-                       $this->hostname  = $hostname;
-                       if (!$this->config->set('config', 'hostname', $this->hostname)) {
-                               $success = false;
+                       if ($this->config->set('config', 'hostname', $hostname)) {
+                               $this->hostname  = $hostname;
+                       } else {
+                               return false;
                        }
                }
 
                if (isset($sslPolicy) && $sslPolicy !== $this->sslPolicy) {
-                       $this->sslPolicy = $sslPolicy;
-                       if (!$this->config->set('system', 'ssl_policy', $this->sslPolicy)) {
-                               $success = false;
+                       if ($this->config->set('system', 'ssl_policy', $sslPolicy)) {
+                               $this->sslPolicy = $sslPolicy;
+                       } else {
+                               $this->hostname  = $currHostname;
+                               $this->config->set('config', 'hostname', $this->hostname);
+                               return false;
                        }
                }
 
                if (isset($urlPath) && $urlPath !== $this->urlPath) {
-                       $this->urlPath   = $urlPath;
-                       if (!$this->config->set('system', 'urlpath', $this->urlPath)) {
-                               $success = false;
+                       if ($this->config->set('system', 'urlpath', $urlPath)) {
+                               $this->urlPath = $urlPath;
+                       } else {
+                               $this->hostname  = $currHostname;
+                               $this->sslPolicy = $currSSLPolicy;
+                               $this->config->set('config', 'hostname', $this->hostname);
+                               $this->config->set('system', 'ssl_policy', $this->sslPolicy);
+                               return false;
                        }
                }
 
                $this->determineBaseUrl();
                if (!$this->config->set('system', 'url', $this->url)) {
-                       $success = false;
+                       $this->hostname  = $currHostname;
+                       $this->sslPolicy = $currSSLPolicy;
+                       $this->urlPath   = $currURLPath;
+                       $this->determineBaseUrl();
+
+                       $this->config->set('config', 'hostname', $this->hostname);
+                       $this->config->set('system', 'ssl_policy', $this->sslPolicy);
+                       $this->config->set('system', 'urlpath', $this->urlPath);
+                       return false;
                }
 
-               return $success;
+               return true;
        }
 
        /**

diff --git a/tests/src/Util/BaseURLTest.php b/tests/src/Util/BaseURLTest.php
index c819a22e5ff364cdc4ac26834eb0644e63f47dc6..ee88bd980461d95f90caf7d6a8b9fd0f16a27276 100644 (file)
--- a/tests/src/Util/BaseURLTest.php
+++ b/tests/src/Util/BaseURLTest.php
@@ -470,4 +470,65 @@ class BaseURLTest extends MockedTest
 
                $this->assertEquals($redirect, $baseUrl->checkRedirectHttps());
        }
+
+       public function dataWrongSave()
+       {
+               return [
+                       'wrongHostname' => [
+                               'fail' => 'hostname',
+                       ],
+                       'wrongSSLPolicy' => [
+                               'fail' => 'sslPolicy',
+                       ],
+                       'wrongURLPath' => [
+                               'fail' => 'urlPath',
+                       ],
+                       'wrongURL' => [
+                               'fail' => 'url',
+                       ],
+               ];
+       }
+
+       /**
+        * Test the save() method with wrong parameters
+        * @dataProvider dataWrongSave
+        */
+       public function testWrongSave($fail)
+       {
+               $configMock = \Mockery::mock(Configuration::class);
+               $configMock->shouldReceive('get')->with('config', 'hostname')->andReturn('friendica.local');
+               $configMock->shouldReceive('get')->with('system', 'urlpath')->andReturn('new/test');
+               $configMock->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(BaseURL::DEFAULT_SSL_SCHEME);
+               $configMock->shouldReceive('get')->with('system', 'url')->andReturn('http://friendica.local/new/test');
+
+               switch ($fail) {
+                       case 'hostname':
+                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(false)->once();
+                               break;
+                       case 'sslPolicy':
+                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(false)->once();
+                               break;
+                       case 'urlPath':
+                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'urlpath', \Mockery::any())->andReturn(false)->once();
+                               break;
+                       case 'url':
+                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'urlpath', \Mockery::any())->andReturn(true)->twice();
+                               $configMock->shouldReceive('set')->with('system', 'url', \Mockery::any())->andReturn(false)->once();
+                               break;
+               }
+
+               $baseUrl = new BaseURL($configMock, []);
+               $this->assertFalse($baseUrl->save('test', 10, 'nope'));
+
+               // nothing should have changed because we never successfully saved anything
+               $this->assertEquals($baseUrl->getHostname(), 'friendica.local');
+               $this->assertEquals($baseUrl->getUrlPath(), 'new/test');
+               $this->assertEquals($baseUrl->getSSLPolicy(), BaseURL::DEFAULT_SSL_SCHEME);
+               $this->assertEquals($baseUrl->get(), 'http://friendica.local/new/test');
+       }
 }