From 0097908821a9c8b58e6a04bc8fe32c08b7ba73ae Mon Sep 17 00:00:00 2001 From: Martii Date: Mon, 20 Nov 2017 19:41:00 -0700 Subject: [PATCH] Subclass the `Error` object * Some reworking of graceful failures to indicate what they are * This probably should have been done around the time `statusCodePage` was implemented with ES5 compatible inheritance. May move this routine to debug.js * One comment BUG fixed * These trapped errors are `throw`able if we choose NOTES: * Almost named the inherited `Error` Object as `userError` since that's what most of these end up being. ;) * **NOT** tested yet with the webhook... have to merge first. GitHub doesn't seem to show the statusMessage so might be out of luck here. 400 is User Error 500 is Server Error *(usually... have a few Watchpoints to figure out if they are the correct code)* * This is pass number 1 ... optimizing/tweaking will come later after this testing and additional respite... including some possible status code changes Applies to #37 --- controllers/scriptStorage.js | 161 ++++++++++++++++++++++++++++------- controllers/user.js | 128 ++++++++++++++++++++++------ libs/debug.js | 12 +++ 3 files changed, 242 insertions(+), 59 deletions(-) diff --git a/controllers/scriptStorage.js b/controllers/scriptStorage.js index 295d5fac2..ba62eb0f1 100644 --- a/controllers/scriptStorage.js +++ b/controllers/scriptStorage.js @@ -4,6 +4,7 @@ var isPro = require('../libs/debug').isPro; var isDev = require('../libs/debug').isDev; var isDbg = require('../libs/debug').isDbg; +var statusError = require('../libs/debug').statusError; // @@ -1181,11 +1182,21 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { var libraries = []; - if (!aMeta || process.env.READ_ONLY_SCRIPT_STORAGE === 'true') { - aCallback(null); + if (process.env.READ_ONLY_SCRIPT_STORAGE === 'true') { + aCallback(new statusError({ + message: 'Read only script storage. Please try again later.', + code: 501 // Not Implemented + }), null); return; } + if (!aMeta) { + aCallback(new statusError({ + message: 'Metadata block(s) missing.', + code: 400 + }), null); + return; + } // `@name` validations if (!isLibrary) { @@ -1196,7 +1207,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { // Can't install a script without a @name (maybe replace with random value) if (!name) { - aCallback(null); + aCallback(new statusError({ + message: '`@name` missing.', + code: 400 + }), null); return; } @@ -1207,15 +1221,21 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { } }); - // Can't install a script without a cleaned @name (maybe replace with random value) + // Check for non-localized presence if (!scriptName) { - aCallback(null); + aCallback(new statusError({ + message: '`@name` non-localized missing.', + code: 400 + }), null); return; } // Can't install a script name ending in a reserved extension if (/\.(?:min|user|user\.js|meta)$/.test(scriptName)) { - aCallback(null); + aCallback(new statusError({ + message: '`@name` ends in a reserved extension.', + code: 400 + }), null); return; } @@ -1226,7 +1246,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (isLibrary && !isEqualKeyset(slaveKeyset, masterKeyset)) { // Keysets do not match exactly... reject - aCallback(null); + aCallback(new statusError({ + message: '`@name` must match in UserScript and UserLibrary metadata blocks.', + code: 400 + }), null); return; } @@ -1237,7 +1260,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (isLibrary && !isEqualKeyset(slaveKeyset, masterKeyset)) { // Keysets do not match exactly... reject - aCallback(null); + aCallback(new statusError({ + message: '`@description` must match in UserScript and UserLibrary metadata blocks.', + code: 400 + }), null); return; } @@ -1248,7 +1274,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (isLibrary && !isEqualKeyset(slaveKeyset, masterKeyset)) { // Keysets do not match exactly... reject - aCallback(null); + aCallback(new statusError({ + message: '`@copyright` must match in UserScript and UserLibrary metadata blocks.', + code: 400 + }), null); return; } @@ -1259,7 +1288,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (isLibrary && !isEqualKeyset(slaveKeyset, masterKeyset)) { // Keysets do not match exactly... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` must match in UserScript and UserLibrary metadata blocks.', + code: 400 + }), null); return; } @@ -1273,7 +1305,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { thatSPDX = userKeyset[userKeyset.length - 1].split('; ')[0].replace(/\+$/, ''); if (SPDXOSI.indexOf(thatSPDX) === -1) { // No valid OSI primary e.g. last key... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` is not OSI primary and compatible in the metadata block(s).', + code: 400 + }), null); return; } @@ -1281,7 +1316,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { thisKeyComponents = userKey.split('; '); if (thisKeyComponents.length > 2) { // Too many parts... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` has too many parts in the metadata block(s).', + code: 400 + }), null); return; } @@ -1289,7 +1327,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (!isFQUrl(thisKeyComponents[1])) { // Not a web url... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` type component not a web url in the metadata block(s).', + code: 400 + }), null); return; } } @@ -1297,13 +1338,19 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { thatSPDX = thisKeyComponents[0].replace(/\+$/, ''); if (SPDX.indexOf(thatSPDX) === -1 || blockSPDX.indexOf(thatSPDX) > -1) { // Absent SPDX short code or blocked SPDX... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` has an incompatible SPDX in the metadata block(s).', + code: 400 + }), null); return; } } } else { // No licensing... reject - aCallback(null); + aCallback(new statusError({ + message: '`@license` is absent in the metadata block(s).', + code: 400 + }), null); return; } @@ -1314,7 +1361,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (isLibrary && !isEqualKeyset(slaveKeyset, masterKeyset)) { // Keysets do not match exactly... reject - aCallback(null); + aCallback(new statusError({ + message: '`@version` must match in UserScript and UserLibrary metadata blocks.', + code: 400 + }), null); return; } @@ -1325,7 +1375,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (!isFQUrl(supportURL, true)) { // Not a web url... reject - aCallback(null); + aCallback(new statusError({ + message: '`@supportURL` not a web url in the UserScript metadata block.', + code: 400 + }), null); return; } } @@ -1338,7 +1391,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (!isFQUrl(homepageURL)) { // Not a web url... reject - aCallback(null); + aCallback(new statusError({ + message: '`@homepageURL` not a web url', + code: 400 + }), null); return; } } @@ -1351,7 +1407,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (!isFQUrl(updateURL)) { // Not a web url... reject - aCallback(null); + aCallback(new statusError({ + message: '`@updateURL` not a web url', + code: 400 + }), null); return; } } @@ -1363,7 +1422,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (!isFQUrl(downloadURL)) { // Not a web url... reject - aCallback(null); + aCallback(new statusError({ + message: '`@downloadURL` not a web url', + code: 400 + }), null); return; } @@ -1373,7 +1435,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { if (rAnyLocalHost.test(downloadURL.host) && !/^\/(?:install|src\/scripts)\//.test(downloadURL.pathname)) { - aCallback(null); + aCallback(new statusError({ + message: '`@downloadURL` not .user.js', + code: 400 + }), null); return; } @@ -1382,7 +1447,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { /^\/(?:install|src\/scripts)\//.test(downloadURL.pathname) && /\.meta\.js$/.test(downloadURL.pathname)) { - aCallback(null); + aCallback(new statusError({ + message: '`@downloadURL` not .user.js', + code: 400 + }), null); return; } } @@ -1401,7 +1469,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { } if (missingExcludeAll) { - aCallback(null); + aCallback(new statusError({ + message: 'UserScript Metadata Block missing `@exclude *`.', + code: 400 + }), null); return; } } @@ -1449,8 +1520,17 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { var script = null; var s3 = null; - if (aRemoved || (!aScript && (aUpdate || collaboration))) { - aCallback(null); + if (aRemoved) { + aCallback(new statusError({ + message: 'Script removed permanently.', + code: 403 + }), null); + return; + } else if ((!aScript && (aUpdate || collaboration))) { + aCallback(new statusError({ + message: 'Collaboration restricted.', + code: 403 + }), null); return; } else if (!aScript) { // New script @@ -1485,7 +1565,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { JSON.stringify(findMeta(script.meta, 'OpenUserJS.collaborator.value')) !== JSON.stringify(collaborators))) { - aCallback(null); + aCallback(new statusError({ + message: 'Forbidden with collaboration', + code: 403 + }), null); return; } aScript.meta = aMeta; @@ -1524,7 +1607,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { aErr.stack ); - aCallback(null); + aCallback(new statusError({ + message: 'Remote storage write error', + code: 502 + }), null); return; } @@ -1538,7 +1624,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { installName + '\n' + JSON.stringify(aErr, null, ' ') ); - aCallback(null); + aCallback(new statusError({ + message: 'Database write error', + code: 502 + }), null); return; } @@ -1554,7 +1643,10 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { userDoc.name + ' was NOT role elevated from User to Author with err of:\n' + JSON.stringify(aErr, null, ' ') ); - aCallback(aScript); + aCallback(new statusError({ + message: 'Database find error', + code: 502 + }), aScript); return; } @@ -1567,7 +1659,7 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { ); // fallthrough } - aCallback(aScript); + aCallback(null, aScript); }); }); } else { @@ -1580,18 +1672,21 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) { ); // fallthrough } - aCallback(aScript); + aCallback(null, aScript); }); } } else { - aCallback(aScript); + aCallback(null, aScript); } }); }); } else { // NOTE: This shouldn't happen console.warn('S3 `new AWS.S3()` critical error'); - aCallback(null); + aCallback(new statusError({ + message: 'Storage critical error', + code: 500 + }), null); return; } }); diff --git a/controllers/user.js b/controllers/user.js index d1959c5ae..9e5aa91ba 100644 --- a/controllers/user.js +++ b/controllers/user.js @@ -4,6 +4,7 @@ var isPro = require('../libs/debug').isPro; var isDev = require('../libs/debug').isDev; var isDbg = require('../libs/debug').isDbg; +var statusError = require('../libs/debug').statusError; // @@ -1279,15 +1280,25 @@ exports.userGitHubImportScriptPage = function (aReq, aRes, aNext) { return; } - onScriptStored = function (aScript) { - if (aScript) { - options.script = aScript; - aCallback(null); + onScriptStored = function (aErr, aScript) { + if (aErr) { + statusCodePage(aReq, aRes, aNext, { + statusCode: aErr.status.code, + statusMessage: aErr.status.message + }); return; - } else { - aCallback('Error while importing script.'); + } + + if (!aScript) { + statusCodePage(aReq, aRes, aNext, { + statusCode: 500, // NOTE: Watchpoint + statusMessage: 'Error while importing script.' + }); return; } + + options.script = aScript; + aCallback(null); }; if (options.javascriptBlob.isUserJS) { @@ -1434,13 +1445,33 @@ exports.uploadScript = function (aReq, aRes, aNext) { var stream = null; var bufs = []; var authedUser = aReq.session.user; - var failUri = '/user/add/' + (isLib ? 'lib' : 'scripts'); - // Reject missing, non-js, and huge files - if (!script || - !(script.type === 'application/javascript' || script.type === 'application/x-javascript') || - script.size > settings.maximum_upload_script_size) { - aRes.redirect(failUri); + // Reject missing files + if (!script) { + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'No file selected.' + }); + return; + } + + // Reject non-js file + if (!(script.type === 'application/javascript' + || script.type === 'application/x-javascript')) { + + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'Selected file is not JavaScript.' + }); + return; + } + + // Reject huge file + if (script.size > settings.maximum_upload_script_size) { + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'Selected file is too big.' + }); return; } @@ -1456,16 +1487,26 @@ exports.uploadScript = function (aReq, aRes, aNext) { scriptStorage.getMeta(bufs, function (aMeta) { if (!isLib && !!scriptStorage.findMeta(aMeta, 'UserLibrary')) { - aRes.redirect(failUri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: + 'UserLibrary metadata block found while attempting to upload as a UserScript.' + }); return; } else if (isLib && !!!scriptStorage.findMeta(aMeta, 'UserLibrary')) { - aRes.redirect(failUri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserLibrary metadata block missing.' + }); return; } - scriptStorage.storeScript(aUser, aMeta, bufferConcat, false, function (aScript) { - if (!aScript) { - aRes.redirect(failUri); + scriptStorage.storeScript(aUser, aMeta, bufferConcat, false, function (aErr, aScript) { + if (aErr || !aScript) { + statusCodePage(aReq, aRes, aNext, { + statusCode: aErr.status.code, + statusMessage: aErr.status.message + }); return; } @@ -1510,7 +1551,7 @@ exports.submitSource = function (aReq, aRes, aNext) { function storeScript(aMeta, aSource) { User.findOne({ _id: authedUser._id }, function (aErr, aUser) { - scriptStorage.storeScript(aUser, aMeta, aSource, false, function (aScript) { + scriptStorage.storeScript(aUser, aMeta, aSource, false, function (aErr, aScript) { var redirectUri = aScript ? ((aScript.isLib ? '/libs/' : '/scripts/') + encodeURIComponent(helpers.cleanFilename(aScript.author)) + @@ -1518,8 +1559,24 @@ exports.submitSource = function (aReq, aRes, aNext) { encodeURIComponent(helpers.cleanFilename(aScript.name))) : aReq.body.url; - if (!aScript || !aReq.body.original) { - aRes.redirect(redirectUri); + if (aErr) { + statusCodePage(aReq, aRes, aNext, { + statusCode: aErr.status.code, + statusMessage: aErr.status.message + }); + return; + } + + if (!aScript) { + statusCodePage(aReq, aRes, aNext, { + statusCode: 500, // NOTE: Watch point + statusMessage: 'No script' + }); + return; + } + + if (!aReq.body.original) { + aRes.redirect(redirectUri); // NOTE: Watchpoint return; } @@ -1561,7 +1618,10 @@ exports.submitSource = function (aReq, aRes, aNext) { var hasName = false; if (!!!scriptStorage.findMeta(aMeta, 'UserScript')) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserScript metadata block missing.' + }); return; } @@ -1569,7 +1629,10 @@ exports.submitSource = function (aReq, aRes, aNext) { name = scriptStorage.findMeta(aMeta, 'UserLibrary.name'); if (!name) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserLibrary metadata block missing `@name`.' + }); return; } @@ -1580,7 +1643,10 @@ exports.submitSource = function (aReq, aRes, aNext) { }); if (!hasName) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserLibrary metadata block missing non-localized `@name`.' + }); return; } @@ -1592,7 +1658,11 @@ exports.submitSource = function (aReq, aRes, aNext) { var hasName = false; if (!!scriptStorage.findMeta(aMeta, 'UserLibrary')) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: + 'UserLibrary metadata block found while attempting to upload as a UserScript.' + }); return; } @@ -1600,7 +1670,10 @@ exports.submitSource = function (aReq, aRes, aNext) { name = scriptStorage.findMeta(aMeta, 'UserScript.name'); if (!name) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserScript metadata block missing `@name`.' + }); return; } @@ -1611,7 +1684,10 @@ exports.submitSource = function (aReq, aRes, aNext) { }); if (!hasName) { - aRes.redirect(uri); + statusCodePage(aReq, aRes, aNext, { + statusCode: 400, + statusMessage: 'UserScript metadata block missing non-localized `@name`.' + }); return; } diff --git a/libs/debug.js b/libs/debug.js index 9e68d1b6d..68a31f461 100644 --- a/libs/debug.js +++ b/libs/debug.js @@ -7,3 +7,15 @@ var isDbg = typeof v8debug === 'object'; exports.isPro = isPro; exports.isDev = isDev; exports.isDbg = isDbg; + +// ES6+ in use to eliminate extra property +class statusError extends Error { + constructor (aStatus, aCode) { + super(JSON.stringify(aStatus, null, ' ')); + this.name = this.constructor.name; + Error.captureStackTrace(this, this.constructor); + this.status = aStatus; + this.code = aCode; + } +} +exports.statusError = statusError;