From 47a0254bb372cf68626302175d2e5f9d0c10e73b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Wed, 31 Jul 2019 10:05:46 +0200
Subject: Validate urls in theming settings and properly handle error messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 apps/theming/lib/Controller/ThemingController.php | 68 +++++++++++------------
 1 file changed, 32 insertions(+), 36 deletions(-)

(limited to 'apps/theming/lib/Controller')

diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index cc8af2cae3e..47895335640 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -135,68 +135,56 @@ class ThemingController extends Controller {
 	 */
 	public function updateStylesheet($setting, $value) {
 		$value = trim($value);
+		$error = null;
 		switch ($setting) {
 			case 'name':
 				if (strlen($value) > 250) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given name is too long'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given name is too long');
 				}
 				break;
 			case 'url':
 				if (strlen($value) > 500) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given web address is too long'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given web address is too long');
+				}
+				if (!$this->isValidUrl($value)) {
+					$error = $this->l10n->t('The given web address is not a valid URL');
 				}
 				break;
 			case 'imprintUrl':
 				if (strlen($value) > 500) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given legal notice address is too long'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given legal notice address is too long');
+				}
+				if (!$this->isValidUrl($value)) {
+					$error = $this->l10n->t('The given legal notice address is not a valid URL');
 				}
 				break;
 			case 'privacyUrl':
 				if (strlen($value) > 500) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given privacy policy address is too long'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given privacy policy address is too long');
+				}
+				if (!$this->isValidUrl($value)) {
+					$error = $this->l10n->t('The given privacy policy address is not a valid URL');
 				}
 				break;
 			case 'slogan':
 				if (strlen($value) > 500) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given slogan is too long'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given slogan is too long');
 				}
 				break;
 			case 'color':
 				if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
-					return new DataResponse([
-						'data' => [
-							'message' => $this->l10n->t('The given color is invalid'),
-						],
-						'status' => 'error'
-					]);
+					$error = $this->l10n->t('The given color is invalid');
 				}
 				break;
 		}
+		if ($error !== null) {
+			return new DataResponse([
+				'data' => [
+					'message' => $error,
+				],
+				'status' => 'error'
+			], Http::STATUS_BAD_REQUEST);
+		}
 
 		$this->themingDefaults->set($setting, $value);
 
@@ -215,6 +203,14 @@ class ThemingController extends Controller {
 		);
 	}
 
+	/**
+	 * Check that a string is a valid http/https url
+	 */
+	private function isValidUrl(string $url): bool {
+		return ((strpos($url, 'http://') === 0 || strpos($url, 'https://') === 0) &&
+			filter_var($url, FILTER_VALIDATE_URL) !== false);
+	}
+
 	/**
 	 * @return DataResponse
 	 * @throws NotPermittedException
-- 
cgit v1.2.3