Validate the input of the theming options

diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index 24865cc2c6e39aa91b13bc728f6b66fd907a7a51..8d9869b84a7c6f5e98d8d465a6be9e959aebd9f3 100644 (file)
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -100,6 +100,50 @@ class ThemingController extends Controller {
         * @internal param string $color
         */
        public function updateStylesheet($setting, $value) {
+               $value = trim($value);
+               switch ($setting) {
+                       case 'name':
+                               if (strlen($value) > 250) {
+                                       return new DataResponse([
+                                               'data' => [
+                                                       'message' => $this->l->t('The given name is too long'),
+                                               ],
+                                               'status' => 'error'
+                                       ]);
+                               }
+                               break;
+                       case 'url':
+                               if (strlen($value) > 500) {
+                                       return new DataResponse([
+                                               'data' => [
+                                                       'message' => $this->l->t('The given web address is too long'),
+                                               ],
+                                               'status' => 'error'
+                                       ]);
+                               }
+                               break;
+                       case 'slogan':
+                               if (strlen($value) > 500) {
+                                       return new DataResponse([
+                                               'data' => [
+                                                       'message' => $this->l->t('The given slogan is too long'),
+                                               ],
+                                               'status' => 'error'
+                                       ]);
+                               }
+                               break;
+                       case 'color':
+                               if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
+                                       return new DataResponse([
+                                               'data' => [
+                                                       'message' => $this->l->t('The given color is invalid'),
+                                               ],
+                                               'status' => 'error'
+                                       ]);
+                               }
+                               break;
+               }
+
                $this->template->set($setting, $value);
                return new DataResponse(
                        [

diff --git a/apps/theming/templates/settings-admin.php b/apps/theming/templates/settings-admin.php
index 27cdd8b60a3d4e2184466ce34328b7b1dd0f0187..3589812070e079e9c99dcc2087494ee47707dc05 100644 (file)
--- a/apps/theming/templates/settings-admin.php
+++ b/apps/theming/templates/settings-admin.php
@@ -15,25 +15,25 @@ style('theming', 'settings-admin');
        <?php } else { ?>
        <p>
                <label><span><?php p($l->t('Name')) ?></span>
-                       <input id="theming-name" type="text" placeholder="<?php p($l->t('Name')); ?>" value="<?php p($_['name']) ?>" />
+                       <input id="theming-name" type="text" placeholder="<?php p($l->t('Name')); ?>" value="<?php p($_['name']) ?>" maxlength="250" />
                </label>
                <span data-setting="name" data-toggle="tooltip" data-original-title="<?php p($l->t('reset to default')); ?>" class="theme-undo icon icon-history"></span>
        </p>
        <p>
                <label><span><?php p($l->t('Web address')) ?></span>
-                       <input id="theming-url" type="text" placeholder="<?php p($l->t('Web address https://…')); ?>" value="<?php p($_['url']) ?>" />
+                       <input id="theming-url" type="text" placeholder="<?php p($l->t('Web address https://…')); ?>" value="<?php p($_['url']) ?>" maxlength="500" />
                </label>
                <span data-setting="url" data-toggle="tooltip" data-original-title="<?php p($l->t('reset to default')); ?>" class="theme-undo icon icon-history"></span>
        </p>
        <p>
                <label><span><?php p($l->t('Slogan')) ?></span>
-                       <input id="theming-slogan" type="text" placeholder="<?php p($l->t('Slogan')); ?>" value="<?php p($_['slogan']) ?>" />
+                       <input id="theming-slogan" type="text" placeholder="<?php p($l->t('Slogan')); ?>" value="<?php p($_['slogan']) ?>" maxlength="500" />
                </label>
                <span data-setting="slogan" data-toggle="tooltip" data-original-title="<?php p($l->t('reset to default')); ?>" class="theme-undo icon icon-history"></span>
        </p>
        <p>
                <label><span><?php p($l->t('Color')) ?></span>
-                       <input id="theming-color" type="text" class="jscolor" value="<?php p($_['color']) ?>" />
+                       <input id="theming-color" type="text" class="jscolor" maxlength="6" value="<?php p($_['color']) ?>" />
                </label>
                <span data-setting="color" data-toggle="tooltip" data-original-title="<?php p($l->t('reset to default')); ?>" class="theme-undo icon icon-history"></span>
        </p>

diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index c5a947cc8b7115f294a7e14c57d829a4aa7904e3..82eb8259af58a1efc40ab8a1ee321c414e9ccbe1 100644 (file)
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -36,34 +36,34 @@ use OCP\IRequest;
 use Test\TestCase;
 
 class ThemingControllerTest extends TestCase {
-       /** @var IRequest */
+       /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
        private $request;
-       /** @var IConfig */
+       /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
        private $config;
-       /** @var Template */
+       /** @var Template|\PHPUnit_Framework_MockObject_MockObject */
        private $template;
        /** @var Util */
        private $util;
        /** @var \OCP\AppFramework\Utility\ITimeFactory */
        private $timeFactory;
-       /** @var IL10N */
+       /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
        private $l10n;
        /** @var ThemingController */
        private $themingController;
-       /** @var IRootFolder */
+       /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */
        private $rootFolder;
 
        public function setUp() {
-               $this->request = $this->getMock('\\OCP\\IRequest');
-               $this->config = $this->getMock('\\OCP\\IConfig');
-               $this->template = $this->getMockBuilder('\\OCA\\Theming\\Template')
+               $this->request = $this->getMockBuilder('OCP\IRequest')->getMock();
+               $this->config = $this->getMockBuilder('OCP\IConfig')->getMock();
+               $this->template = $this->getMockBuilder('OCA\Theming\Template')
                        ->disableOriginalConstructor()->getMock();
                $this->util = new Util();
                $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory')
                        ->disableOriginalConstructor()
                        ->getMock();
-               $this->l10n = $this->getMock('\\OCP\\IL10N');
-               $this->rootFolder = $this->getMock('\\OCP\\Files\\IRootFolder');
+               $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock();
+               $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock();
 
                $this->timeFactory->expects($this->any())
                        ->method('getTime')
@@ -83,27 +83,48 @@ class ThemingControllerTest extends TestCase {
                return parent::setUp();
        }
 
-       public function testUpdateStylesheet() {
+       public function dataUpdateStylesheet() {
+               return [
+                       ['name', str_repeat('a', 250), 'success', 'Saved'],
+                       ['name', str_repeat('a', 251), 'error', 'The given name is too long'],
+                       ['url', str_repeat('a', 500), 'success', 'Saved'],
+                       ['url', str_repeat('a', 501), 'error', 'The given web address is too long'],
+                       ['slogan', str_repeat('a', 500), 'success', 'Saved'],
+                       ['slogan', str_repeat('a', 501), 'error', 'The given slogan is too long'],
+                       ['color', '#0082c9', 'success', 'Saved'],
+                       ['color', '#0082C9', 'success', 'Saved'],
+                       ['color', '0082C9', 'error', 'The given color is invalid'],
+                       ['color', '#0082Z9', 'error', 'The given color is invalid'],
+                       ['color', 'Nextcloud', 'error', 'The given color is invalid'],
+               ];
+       }
+
+       /**
+        * @dataProvider dataUpdateStylesheet
+        *
+        * @param string $setting
+        * @param string $value
+        * @param string $status
+        * @param string $message
+        */
+       public function testUpdateStylesheet($setting, $value, $status, $message) {
                $this->template
-                       ->expects($this->once())
+                       ->expects($status === 'success' ? $this->once() : $this->never())
                        ->method('set')
-                       ->with('MySetting', 'MyValue');
+                       ->with($setting, $value);
                $this->l10n
                        ->expects($this->once())
                        ->method('t')
-                       ->with('Saved')
-                       ->willReturn('Saved');
+                       ->with($message)
+                       ->willReturn($message);
 
-               $expected = new DataResponse(
-                       [
-                               'data' =>
-                                       [
-                                               'message' => 'Saved',
-                                       ],
-                               'status' => 'success'
-                       ]
-               );
-               $this->assertEquals($expected, $this->themingController->updateStylesheet('MySetting', 'MyValue'));
+               $expected = new DataResponse([
+                       'data' => [
+                               'message' => $message,
+                       ],
+                       'status' => $status,
+               ]);
+               $this->assertEquals($expected, $this->themingController->updateStylesheet($setting, $value));
        }
 
        public function testUpdateLogoNoData() {
@@ -665,5 +686,4 @@ class ThemingControllerTest extends TestCase {
                $expected->addHeader('Expires', date(\DateTime::RFC2822, 123));
                @$this->assertEquals($expected, $this->themingController->getStylesheet());
        }
-
 }