mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #6585 from nextcloud/theming-fileupload-errorhandling 

Theming: Handle errors on uploaded files properly
This commit is contained in:
Roeland Jago Douma 2017-10-15 21:44:48 +02:00 committed by GitHub
parent a5912cbe33 f15e85c4f5
commit 1b2568e637
4 changed files with 222 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/theming/css/settings-admin.css
View File

@ -78,6 +78,7 @@ form.uploadButton {
#theming_settings_msg {
vertical-align: middle;
border-radius: 3px;
}
#theming-preview-logo {

2
apps/theming/js/settings-admin.js
View File

@ -141,6 +141,7 @@ $(document).ready(function () {
fail: function (e, response){
OC.msg.finishedError('#theming_settings_msg', response._response.jqXHR.responseJSON.data.message);
$('label#uploadlogo').addClass('icon-upload').removeClass('icon-loading-small');
$('#theming_settings_loading').hide();
}
};
var uploadParamsLogin = {
@ -159,6 +160,7 @@ $(document).ready(function () {
fail: function (e, response){
$('label#upload-login-background').removeClass('icon-loading-small').addClass('icon-upload');
OC.msg.finishedError('#theming_settings_msg', response._response.jqXHR.responseJSON.data.message);
$('#theming_settings_loading').hide();
}
};

38
apps/theming/lib/Controller/ThemingController.php
View File

@ -207,12 +207,34 @@ class ThemingController extends Controller {
}
$newLogo = $this->request->getUploadedFile('uploadlogo');
$newBackgroundLogo = $this->request->getUploadedFile('upload-login-background');
$error = null;
$phpFileUploadErrors = [
UPLOAD_ERR_OK => $this->l10n->t('There is no error, the file uploaded with success'),
UPLOAD_ERR_INI_SIZE => $this->l10n->t('The uploaded file exceeds the upload_max_filesize directive in php.ini'),
UPLOAD_ERR_FORM_SIZE => $this->l10n->t('The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'),
UPLOAD_ERR_PARTIAL => $this->l10n->t('The uploaded file was only partially uploaded'),
UPLOAD_ERR_NO_FILE => $this->l10n->t('No file was uploaded'),
UPLOAD_ERR_NO_TMP_DIR => $this->l10n->t('Missing a temporary folder'),
UPLOAD_ERR_CANT_WRITE => $this->l10n->t('Failed to write file to disk.'),
UPLOAD_ERR_EXTENSION => $this->l10n->t('A PHP extension stopped the file upload.'),
];
if (empty($newLogo) && empty($newBackgroundLogo)) {
$error = $this->l10n->t('No file uploaded');
}
if (!empty($newLogo) && array_key_exists('error', $newLogo) && $newLogo['error'] !== UPLOAD_ERR_OK) {
$error = $phpFileUploadErrors[$newLogo['error']];
}
if (!empty($newBackgroundLogo) && array_key_exists('error', $newBackgroundLogo) && $newBackgroundLogo['error'] !== UPLOAD_ERR_OK) {
$error = $phpFileUploadErrors[$newBackgroundLogo['error']];
}
if ($error !== null) {
return new DataResponse(
[
'data' => [
'message' => $this->l10n->t('No file uploaded')
]
'message' => $error
],
'status' => 'failure',
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
@ -227,6 +249,18 @@ class ThemingController extends Controller {
if (!empty($newLogo)) {
$target = $folder->newFile('logo');
$supportedFormats = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml', 'text/svg'];
if (!in_array($newLogo['type'], $supportedFormats)) {
return new DataResponse(
[
'data' => [
'message' => $this->l10n->t('Unsupported image type'),
],
'status' => 'failure',
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
}
$target->putContent(file_get_contents($newLogo['tmp_name'], 'r'));
$this->themingDefaults->set('logoMime', $newLogo['type']);
$name = $newLogo['name'];

203
apps/theming/tests/Controller/ThemingControllerTest.php
View File

@ -131,8 +131,9 @@ class ThemingControllerTest extends TestCase {
$this->l10n
->expects($this->once())
->method('t')
->with($message)
->willReturn($message);
->will($this->returnCallback(function($str) {
return $str;
}));
$this->scssCacher
->expects($this->once())
->method('getCachedSCSS')
@ -183,8 +184,9 @@ class ThemingControllerTest extends TestCase {
$this->l10n
->expects($this->once())
->method('t')
->with($message)
->willReturn($message);
->will($this->returnCallback(function($str) {
return $str;
}));
$expected = new DataResponse(
[
@ -215,10 +217,11 @@ class ThemingControllerTest extends TestCase {
->with('upload-login-background')
->willReturn(null);
$this->l10n
->expects($this->once())
->expects($this->any())
->method('t')
->with('No file uploaded')
->willReturn('No file uploaded');
->will($this->returnCallback(function($str) {
return $str;
}));
$expected = new DataResponse(
[
@ -226,6 +229,56 @@ class ThemingControllerTest extends TestCase {
[
'message' => 'No file uploaded',
],
'status' => 'failure',
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
$this->assertEquals($expected, $this->themingController->updateLogo());
}
public function testUpdateLogoInvalidMimeType() {
$this->request
->expects($this->at(0))
->method('getParam')
->with('backgroundColor')
->willReturn(false);
$this->request
->expects($this->at(1))
->method('getUploadedFile')
->with('uploadlogo')
->willReturn([
'tmp_name' => 'logo.pdf',
'type' => 'application/pdf',
'name' => 'logo.pdf',
'error' => 0,
]);
$this->request
->expects($this->at(2))
->method('getUploadedFile')
->with('upload-login-background')
->willReturn(null);
$this->l10n
->expects($this->any())
->method('t')
->will($this->returnCallback(function($str) {
return $str;
}));
$folder = $this->createMock(ISimpleFolder::class);
$this->appData
->expects($this->once())
->method('getFolder')
->with('images')
->willReturn($folder);
$expected = new DataResponse(
[
'data' =>
[
'message' => 'Unsupported image type',
],
'status' => 'failure'
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
@ -258,13 +311,17 @@ class ThemingControllerTest extends TestCase {
public function dataUpdateImages() {
return [
[false],
[true]
['image/jpeg', false],
['image/jpeg', true],
['image/gif'],
['image/png'],
['image/svg+xml'],
['text/svg'],
];
}
/** @dataProvider dataUpdateImages */
public function testUpdateLogoNormalLogoUpload($folderExists) {
public function testUpdateLogoNormalLogoUpload($mimeType, $folderExists=true) {
$tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg';
$destination = \OC::$server->getTempManager()->getTemporaryFolder();
@ -280,8 +337,9 @@ class ThemingControllerTest extends TestCase {
->with('uploadlogo')
->willReturn([
'tmp_name' => $tmpLogo,
'type' => 'text/svg',
'type' => $mimeType,
'name' => 'logo.svg',
'error' => 0,
]);
$this->request
->expects($this->at(2))
@ -289,10 +347,11 @@ class ThemingControllerTest extends TestCase {
->with('upload-login-background')
->willReturn(null);
$this->l10n
->expects($this->once())
->expects($this->any())
->method('t')
->with('Saved')
->willReturn('Saved');
->will($this->returnCallback(function($str) {
return $str;
}));
$file = $this->createMock(ISimpleFile::class);
@ -357,12 +416,14 @@ class ThemingControllerTest extends TestCase {
'tmp_name' => $tmpLogo,
'type' => 'text/svg',
'name' => 'logo.svg',
'error' => 0,
]);
$this->l10n
->expects($this->once())
->expects($this->any())
->method('t')
->with('Saved')
->willReturn('Saved');
->will($this->returnCallback(function($str) {
return $str;
}));
$file = $this->createMock(ISimpleFile::class);
$folder = $this->createMock(ISimpleFolder::class);
@ -425,12 +486,14 @@ class ThemingControllerTest extends TestCase {
'tmp_name' => $tmpLogo,
'type' => 'text/svg',
'name' => 'logo.svg',
'error' => 0,
]);
$this->l10n
->expects($this->once())
->expects($this->any())
->method('t')
->with('Unsupported image type')
->willReturn('Unsupported image type');
->will($this->returnCallback(function($str) {
return $str;
}));
$folder = $this->createMock(ISimpleFolder::class);
$this->appData
@ -452,6 +515,106 @@ class ThemingControllerTest extends TestCase {
$this->assertEquals($expected, $this->themingController->updateLogo());
}
public function dataPhpUploadErrors() {
return [
[UPLOAD_ERR_INI_SIZE, 'The uploaded file exceeds the upload_max_filesize directive in php.ini'],
[UPLOAD_ERR_FORM_SIZE, 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'],
[UPLOAD_ERR_PARTIAL, 'The uploaded file was only partially uploaded'],
[UPLOAD_ERR_NO_FILE, 'No file was uploaded'],
[UPLOAD_ERR_NO_TMP_DIR, 'Missing a temporary folder'],
[UPLOAD_ERR_CANT_WRITE, 'Failed to write file to disk.'],
[UPLOAD_ERR_EXTENSION, 'A PHP extension stopped the file upload.'],
];
}
/**
* @dataProvider dataPhpUploadErrors
*/
public function testUpdateLogoLoginScreenUploadWithInvalidImageUpload($error, $expectedErrorMessage) {
$this->request
->expects($this->at(0))
->method('getParam')
->with('backgroundColor')
->willReturn(false);
$this->request
->expects($this->at(1))
->method('getUploadedFile')
->with('uploadlogo')
->willReturn(null);
$this->request
->expects($this->at(2))
->method('getUploadedFile')
->with('upload-login-background')
->willReturn([
'tmp_name' => '',
'type' => 'text/svg',
'name' => 'logo.svg',
'error' => $error,
]);
$this->l10n
->expects($this->any())
->method('t')
->will($this->returnCallback(function($str) {
return $str;
}));
$expected = new DataResponse(
[
'data' =>
[
'message' => $expectedErrorMessage,
],
'status' => 'failure'
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
$this->assertEquals($expected, $this->themingController->updateLogo());
}
/**
* @dataProvider dataPhpUploadErrors
*/
public function testUpdateLogoUploadWithInvalidImageUpload($error, $expectedErrorMessage) {
$this->request
->expects($this->at(0))
->method('getParam')
->with('backgroundColor')
->willReturn(false);
$this->request
->expects($this->at(1))
->method('getUploadedFile')
->with('uploadlogo')
->willReturn([
'tmp_name' => '',
'type' => 'text/svg',
'name' => 'logo.svg',
'error' => $error,
]);
$this->request
->expects($this->at(2))
->method('getUploadedFile')
->with('upload-login-background')
->willReturn(null);
$this->l10n
->expects($this->any())
->method('t')
->will($this->returnCallback(function($str) {
return $str;
}));
$expected = new DataResponse(
[
'data' =>
[
'message' => $expectedErrorMessage
],
'status' => 'failure'
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
$this->assertEquals($expected, $this->themingController->updateLogo());
}
public function testUndo() {
$this->l10n
->expects($this->once())