Nemcio/NodeBB
1
0
Fork 0
mirror of https://github.com/NodeBB/NodeBB.git synced 2025-10-26 16:46:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: dont send 200 status on admin upload errors (#11707)

Browse Source 
* fix: dont send 200 status on admin upload errors

* test: update test

* bring back both checks for error

* test: add statusCode tests
This commit is contained in:
Barış Soner Uşaklı
2023-06-13 12:32:48 -04:00
committed by GitHub
parent f2c0c18879
commit 8ca65b0c78
4 changed files with 58 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
public/src/modules/uploader.js
View File

@@ -61,6 +61,7 @@ define('uploader', ['jquery-form'], function () {
if (type === 'error') { if (type === 'error') {
uploadModal.find('#fileUploadSubmitBtn').removeClass('disabled'); uploadModal.find('#fileUploadSubmitBtn').removeClass('disabled');
} }
message = message.replace(/&#44/g, '&#44');
uploadModal.find('#alert-' + type).translateText(message).removeClass('hide'); uploadModal.find('#alert-' + type).translateText(message).removeClass('hide');
} }
@@ -72,7 +73,13 @@ define('uploader', ['jquery-form'], function () {
}, },
error: function (xhr) { error: function (xhr) {
xhr = maybeParse(xhr); xhr = maybeParse(xhr);
showAlert(uploadModal, 'error', xhr.responseJSON?.status?.message || `[[error:upload-error-fallback, ${xhr.status} ${xhr.statusText}]]`); showAlert(
uploadModal,
'error',
xhr.responseJSON?.status?.message || // apiv3
xhr.responseJSON?.error || // { "error": "[[error:some-error]]]" }
`[[error:upload-error-fallback, ${xhr.status} ${xhr.statusText}]]`
);
}, },
uploadProgress: function (event, position, total, percent) { uploadProgress: function (event, position, total, percent) {
uploadModal.find('#upload-progress-bar').css('width', percent + '%'); uploadModal.find('#upload-progress-bar').css('width', percent + '%');
@@ -99,7 +106,7 @@ define('uploader', ['jquery-form'], function () {
function maybeParse(response) { function maybeParse(response) {
if (typeof response === 'string') { if (typeof response === 'string') {
try { try {
return $.parseJSON(response); return JSON.parse(response);
} catch (e) { } catch (e) {
return { error: '[[error:parse-error]]' }; return { error: '[[error:parse-error]]' };
} }

90
src/controllers/admin/uploads.js
View File

@@ -118,25 +118,23 @@ uploadsController.uploadCategoryPicture = async function (req, res, next) {
return next(new Error('[[error:invalid-json]]')); return next(new Error('[[error:invalid-json]]'));
} }
if (validateUpload(res, uploadedFile, allowedImageTypes)) { await validateUpload(uploadedFile, allowedImageTypes);
const filename = `category-${params.cid}${path.extname(uploadedFile.name)}`; const filename = `category-${params.cid}${path.extname(uploadedFile.name)}`;
await uploadImage(filename, 'category', uploadedFile, req, res, next); await uploadImage(filename, 'category', uploadedFile, req, res, next);
}
}; };
uploadsController.uploadFavicon = async function (req, res, next) { uploadsController.uploadFavicon = async function (req, res, next) {
const uploadedFile = req.files.files[0]; const uploadedFile = req.files.files[0];
const allowedTypes = ['image/x-icon', 'image/vnd.microsoft.icon']; const allowedTypes = ['image/x-icon', 'image/vnd.microsoft.icon'];
if (validateUpload(res, uploadedFile, allowedTypes)) { await validateUpload(uploadedFile, allowedTypes);
try { try {
const imageObj = await file.saveFileToLocal('favicon.ico', 'system', uploadedFile.path); const imageObj = await file.saveFileToLocal('favicon.ico', 'system', uploadedFile.path);
res.json([{ name: uploadedFile.name, url: imageObj.url }]); res.json([{ name: uploadedFile.name, url: imageObj.url }]);
} catch (err) { } catch (err) {
next(err); next(err);
} finally { } finally {
file.delete(uploadedFile.path); file.delete(uploadedFile.path);
}
} }
}; };
@@ -145,25 +143,24 @@ uploadsController.uploadTouchIcon = async function (req, res, next) {
const allowedTypes = ['image/png']; const allowedTypes = ['image/png'];
const sizes = [36, 48, 72, 96, 144, 192, 512]; const sizes = [36, 48, 72, 96, 144, 192, 512];
if (validateUpload(res, uploadedFile, allowedTypes)) { await validateUpload(uploadedFile, allowedTypes);
try { try {
const imageObj = await file.saveFileToLocal('touchicon-orig.png', 'system', uploadedFile.path); const imageObj = await file.saveFileToLocal('touchicon-orig.png', 'system', uploadedFile.path);
// Resize the image into squares for use as touch icons at various DPIs // Resize the image into squares for use as touch icons at various DPIs
for (const size of sizes) { for (const size of sizes) {
/* eslint-disable no-await-in-loop */ /* eslint-disable no-await-in-loop */
await image.resizeImage({ await image.resizeImage({
path: uploadedFile.path, path: uploadedFile.path,
target: path.join(nconf.get('upload_path'), 'system', `touchicon-${size}.png`), target: path.join(nconf.get('upload_path'), 'system', `touchicon-${size}.png`),
width: size, width: size,
height: size, height: size,
}); });
}
res.json([{ name: uploadedFile.name, url: imageObj.url }]);
} catch (err) {
next(err);
} finally {
file.delete(uploadedFile.path);
} }
res.json([{ name: uploadedFile.name, url: imageObj.url }]);
} catch (err) {
next(err);
} finally {
file.delete(uploadedFile.path);
} }
}; };
@@ -172,15 +169,14 @@ uploadsController.uploadMaskableIcon = async function (req, res, next) {
const uploadedFile = req.files.files[0]; const uploadedFile = req.files.files[0];
const allowedTypes = ['image/png']; const allowedTypes = ['image/png'];
if (validateUpload(res, uploadedFile, allowedTypes)) { await validateUpload(uploadedFile, allowedTypes);
try { try {
const imageObj = await file.saveFileToLocal('maskableicon-orig.png', 'system', uploadedFile.path); const imageObj = await file.saveFileToLocal('maskableicon-orig.png', 'system', uploadedFile.path);
res.json([{ name: uploadedFile.name, url: imageObj.url }]); res.json([{ name: uploadedFile.name, url: imageObj.url }]);
} catch (err) { } catch (err) {
next(err); next(err);
} finally { } finally {
file.delete(uploadedFile.path); file.delete(uploadedFile.path);
}
} }
}; };
@@ -219,20 +215,16 @@ uploadsController.uploadOgImage = async function (req, res, next) {
async function upload(name, req, res, next) { async function upload(name, req, res, next) {
const uploadedFile = req.files.files[0]; const uploadedFile = req.files.files[0];
if (validateUpload(res, uploadedFile, allowedImageTypes)) { await validateUpload(uploadedFile, allowedImageTypes);
const filename = name + path.extname(uploadedFile.name); const filename = name + path.extname(uploadedFile.name);
await uploadImage(filename, 'system', uploadedFile, req, res, next); await uploadImage(filename, 'system', uploadedFile, req, res, next);
}
} }
function validateUpload(res, uploadedFile, allowedTypes) { async function validateUpload(uploadedFile, allowedTypes) {
if (!allowedTypes.includes(uploadedFile.type)) { if (!allowedTypes.includes(uploadedFile.type)) {
file.delete(uploadedFile.path); file.delete(uploadedFile.path);
res.json({ error: `[[error:invalid-image-type, ${allowedTypes.join(', ')}]]` }); throw new Error(`[[error:invalid-image-type, ${allowedTypes.join(', ')}]]`);
return false;
} }
return true;
} }
async function uploadImage(filename, folder, uploadedFile, req, res, next) { async function uploadImage(filename, folder, uploadedFile, req, res, next) {

8
src/views/modals/upload-file.tpl
View File

@@ -6,10 +6,10 @@
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-hidden="true"></button> <button type="button" class="btn-close" data-bs-dismiss="modal" aria-hidden="true"></button>
</div> </div>
<div class="modal-body"> <div class="modal-body">
<form id="uploadForm" action="" method="post" enctype="multipart/form-data"> <form class="mb-3" id="uploadForm" action="" method="post" enctype="multipart/form-data">
<div class="form-group"> <div>
{{{ if description }}} {{{ if description }}}
<label for="fileInput">{description}</label> <label class="form-label" for="fileInput">{description}</label>
{{{ end }}} {{{ end }}}
<input type="file" id="fileInput" name="files[]" {{{ if accept }}}accept="{accept}"{{{ end }}}> <input type="file" id="fileInput" name="files[]" {{{ if accept }}}accept="{accept}"{{{ end }}}>
{{{ if showHelp }}} {{{ if showHelp }}}
@@ -25,7 +25,7 @@
<input type="hidden" id="params" name="params" /> <input type="hidden" id="params" name="params" />
</form> </form>
<div id="upload-progress-box" class="progress progress-striped hide"> <div id="upload-progress-box" class="progress progress-striped hide mb-3">
<div id="upload-progress-bar" class="progress-bar progress-bar-success" role="progressbar" aria-valuenow="0" aria-valuemin="0"> <div id="upload-progress-bar" class="progress-bar progress-bar-success" role="progressbar" aria-valuenow="0" aria-valuemin="0">
<span class="sr-only"> [[success:success]]</span> <span class="sr-only"> [[success:success]]</span>
</div> </div>

6
test/uploads.js
View File

@@ -340,7 +340,7 @@ describe('Upload Controllers', () => {
it('should upload site logo', (done) => { it('should upload site logo', (done) => {
helpers.uploadFile(`${nconf.get('url')}/api/admin/uploadlogo`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token, (err, res, body) => { helpers.uploadFile(`${nconf.get('url')}/api/admin/uploadlogo`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token, (err, res, body) => {
assert.ifError(err); assert.ifError(err);
assert.equal(res.statusCode, 200); assert.strictEqual(res.statusCode, 200);
assert(Array.isArray(body)); assert(Array.isArray(body));
assert.equal(body[0].url, `${nconf.get('relative_path')}/assets/uploads/system/site-logo.png`); assert.equal(body[0].url, `${nconf.get('relative_path')}/assets/uploads/system/site-logo.png`);
done(); done();
@@ -350,7 +350,8 @@ describe('Upload Controllers', () => {
it('should fail to upload invalid file type', (done) => { it('should fail to upload invalid file type', (done) => {
helpers.uploadFile(`${nconf.get('url')}/api/admin/category/uploadpicture`, path.join(__dirname, '../test/files/503.html'), { params: JSON.stringify({ cid: cid }) }, jar, csrf_token, (err, res, body) => { helpers.uploadFile(`${nconf.get('url')}/api/admin/category/uploadpicture`, path.join(__dirname, '../test/files/503.html'), { params: JSON.stringify({ cid: cid }) }, jar, csrf_token, (err, res, body) => {
assert.ifError(err); assert.ifError(err);
assert.equal(body.error, '[[error:invalid-image-type, image/png&#44; image/jpeg&#44; image/pjpeg&#44; image/jpg&#44; image/gif&#44; image/svg+xml]]'); assert.strictEqual(res.statusCode, 500);
assert.equal(body.error, '[[error:invalid-image-type, image&#x2F;png&amp;#44; image&#x2F;jpeg&amp;#44; image&#x2F;pjpeg&amp;#44; image&#x2F;jpg&amp;#44; image&#x2F;gif&amp;#44; image&#x2F;svg+xml]]');
done(); done();
}); });
}); });
@@ -358,6 +359,7 @@ describe('Upload Controllers', () => {
it('should fail to upload category image with invalid json params', (done) => { it('should fail to upload category image with invalid json params', (done) => {
helpers.uploadFile(`${nconf.get('url')}/api/admin/category/uploadpicture`, path.join(__dirname, '../test/files/test.png'), { params: 'invalid json' }, jar, csrf_token, (err, res, body) => { helpers.uploadFile(`${nconf.get('url')}/api/admin/category/uploadpicture`, path.join(__dirname, '../test/files/test.png'), { params: 'invalid json' }, jar, csrf_token, (err, res, body) => {
assert.ifError(err); assert.ifError(err);
assert.strictEqual(res.statusCode, 500);
assert.equal(body.error, '[[error:invalid-json]]'); assert.equal(body.error, '[[error:invalid-json]]');
done(); done();
}); });