open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Validate urls in theming settings and properly handle error messages

Browse Source 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
pull/16619/head
Julius Härtl 5 years ago
parent 3f8f0f7609
commit 47a0254bb3
No known key found for this signature in database
GPG Key ID: 4C614C6ED2CDE6DF
3 changed files with 49 additions and 43 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 6
      apps/theming/js/settings-admin.js
  2. 68
      apps/theming/lib/Controller/ThemingController.php
  3. 18
      apps/theming/tests/Controller/ThemingControllerTest.php

6
apps/theming/js/settings-admin.js
Unescape View File

@ -32,7 +32,7 @@ function setThemingValue(setting, value) {
hideUndoButton(setting, value);
preview(setting, value, response.data.serverCssUrl);
}).fail(function(response) {
OC.msg.finishedSaving('#theming_settings_msg', response);
OC.msg.finishedSaving('#theming_settings_msg', response.responseJSON);
$('#theming_settings_loading').hide();
});
}
@ -159,7 +159,7 @@ $(document).ready(function () {
return true;
} else {
throw t('theming', 'Name cannot be empty');
}
}
} catch (error) {
$('#theming-name').attr('title', error);
$('#theming-name').tooltip({placement: 'top', trigger: 'manual'});
@ -174,7 +174,7 @@ $(document).ready(function () {
if (checkName()) {
$('#theming-name').tooltip('hide');
$('#theming-name').removeClass('error');
}
}
});
$('#theming-name').change(function(e) {

68
apps/theming/lib/Controller/ThemingController.php
Unescape View File

@ -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

18
apps/theming/tests/Controller/ThemingControllerTest.php
Unescape View File

@ -123,10 +123,13 @@ class ThemingControllerTest extends TestCase {
public function dataUpdateStylesheetSuccess() {
return [
['name', str_repeat('a', 250), 'Saved'],
['url', str_repeat('a', 500), 'Saved'],
['url', 'https://nextcloud.com/' . str_repeat('a', 478), 'Saved'],
['slogan', str_repeat('a', 500), 'Saved'],
['color', '#0082c9', 'Saved'],
['color', '#0082C9', 'Saved'],
['color', '#0082C9', 'Saved'],
['imprintUrl', 'https://nextcloud.com/' . str_repeat('a', 478), 'Saved'],
['privacyUrl', 'https://nextcloud.com/' . str_repeat('a', 478), 'Saved'],
];
}
@ -175,11 +178,17 @@ class ThemingControllerTest extends TestCase {
public function dataUpdateStylesheetError() {
return [
['name', str_repeat('a', 251), 'The given name is too long'],
['url', str_repeat('a', 501), 'The given web address is too long'],
['url', 'http://example.com/' . str_repeat('a', 501), 'The given web address is too long'],
['url', str_repeat('a', 501), 'The given web address is not a valid URL'],
['url', 'javascript:alert(1)', 'The given web address is not a valid URL'],
['slogan', str_repeat('a', 501), 'The given slogan is too long'],
['color', '0082C9', 'The given color is invalid'],
['color', '#0082Z9', 'The given color is invalid'],
['color', 'Nextcloud', 'The given color is invalid'],
['imprintUrl', '0082C9', 'The given legal notice address is not a valid URL'],
['imprintUrl', '0082C9', 'The given legal notice address is not a valid URL'],
['imprintUrl', 'javascript:foo', 'The given legal notice address is not a valid URL'],
['privacyUrl', '#0082Z9', 'The given privacy policy address is not a valid URL'],
];
}
@ -196,7 +205,7 @@ class ThemingControllerTest extends TestCase {
->method('set')
->with($setting, $value);
$this->l10n
->expects($this->once())
->expects($this->any())
->method('t')
->will($this->returnCallback(function($str) {
return $str;
@ -209,7 +218,8 @@ class ThemingControllerTest extends TestCase {
'message' => $message,
],
'status' => 'error',
]
],
Http::STATUS_BAD_REQUEST
);
$this->assertEquals($expected, $this->themingController->updateStylesheet($setting, $value));
}

Write Preview
Loading…
Cancel
Save