From 11453d544c2a47b562004e383106599226c51e87 Mon Sep 17 00:00:00 2001 From: muxator Date: Fri, 1 Mar 2019 09:43:41 +0100 Subject: [PATCH] prepare to async: stricter checks This change is in preparation of the future async refactoring by Ray. It tries to extract as many changes in boolean conditions as possible, in order to make more evident identifying eventual logic bugs in the future work. This proved already useful in at least one case. BEWARE: this commit exposes an incoherency in the DB API, in which, depending on the driver used, some functions can return null or undefined. This condition will be externally fixed by the final commit in this series ("db/DB.js: prevent DB layer from returning undefined"). Until that commit, the code base may have some bugs. --- src/node/db/API.js | 32 +++++++++++++++--------------- src/node/db/AuthorManager.js | 12 +++++------ src/node/db/GroupManager.js | 8 ++++---- src/node/db/SecurityManager.js | 14 ++++++------- src/node/handler/APIHandler.js | 2 +- src/node/handler/SocketIORouter.js | 2 +- src/node/padaccess.js | 2 +- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/node/db/API.js b/src/node/db/API.js index a2c0d60b7..b6acefcd4 100644 --- a/src/node/db/API.js +++ b/src/node/db/API.js @@ -192,7 +192,7 @@ Example returns: exports.getText = function(padID, rev, callback) { // check if rev is a number - if (rev !== undefined && typeof rev != "number") { + if (rev !== undefined && typeof rev !== "number") { // try to parse the number if (isNaN(parseInt(rev))) { callback(new customError("rev is not a number", "apierror")); @@ -256,7 +256,7 @@ Example returns: exports.setText = function(padID, text, callback) { // text is required - if (typeof text != "string") { + if (typeof text !== "string") { callback(new customError("text is not a string", "apierror")); return; } @@ -285,7 +285,7 @@ Example returns: exports.appendText = function(padID, text, callback) { // text is required - if (typeof text != "string") { + if (typeof text !== "string") { callback(new customError("text is not a string", "apierror")); return; } @@ -311,7 +311,7 @@ Example returns: */ exports.getHTML = function(padID, rev, callback) { - if (rev !== undefined && typeof rev != "number") { + if (rev !== undefined && typeof rev !== "number") { if (isNaN(parseInt(rev))) { callback(new customError("rev is not a number", "apierror")); return; @@ -375,7 +375,7 @@ Example returns: exports.setHTML = function(padID, html, callback) { // html is required - if (typeof html != "string") { + if (typeof html !== "string") { callback(new customError("html is not a string", "apierror")); return; } @@ -471,7 +471,7 @@ Example returns: exports.appendChatMessage = function(padID, text, authorID, time, callback) { // text is required - if (typeof text != "string") { + if (typeof text !== "string") { callback(new customError("text is not a string", "apierror")); return; } @@ -557,7 +557,7 @@ Example returns: exports.saveRevision = function(padID, rev, callback) { // check if rev is a number - if (rev !== undefined && typeof rev != "number") { + if (rev !== undefined && typeof rev !== "number") { // try to parse the number if (isNaN(parseInt(rev))) { callback(new customError("rev is not a number", "apierror")); @@ -636,7 +636,7 @@ exports.createPad = function(padID, text, callback) { if (padID) { // ensure there is no $ in the padID - if (padID.indexOf("$") != -1) { + if (padID.indexOf("$") !== -1) { callback(new customError("createPad can't create group pads", "apierror")); return; } @@ -682,7 +682,7 @@ exports.deletePad = function(padID, callback) exports.restoreRevision = function(padID, rev, callback) { // check if rev is a number - if (rev !== undefined && typeof rev != "number") { + if (rev !== undefined && typeof rev !== "number") { // try to parse the number if (isNaN(parseInt(rev))) { callback(new customError("rev is not a number", "apierror")); @@ -838,7 +838,7 @@ exports.getPadID = function(roID, callback) readOnlyManager.getPadId(roID, function(err, retrievedPadID) { if (ERR(err, callback)) return; - if (retrievedPadID == null) { + if (retrievedPadID === null) { callback(new customError("padID does not exist", "apierror")); return; } @@ -858,7 +858,7 @@ Example returns: exports.setPublicStatus = function(padID, publicStatus, callback) { // ensure this is a group pad - if (padID && padID.indexOf("$") == -1) { + if (padID && padID.indexOf("$") === -1) { callback(new customError("You can only get/set the publicStatus of pads that belong to a group", "apierror")); return; } @@ -868,7 +868,7 @@ exports.setPublicStatus = function(padID, publicStatus, callback) if (ERR(err, callback)) return; // convert string to boolean - if (typeof publicStatus == "string") + if (typeof publicStatus === "string") publicStatus = publicStatus == "true" ? true : false; // set the password @@ -1045,7 +1045,7 @@ Example returns: */ exports.createDiffHTML = function(padID, startRev, endRev, callback) { // check if startRev is a number - if (startRev !== undefined && typeof startRev != "number") { + if (startRev !== undefined && typeof startRev !== "number") { // try to parse the number if (isNaN(parseInt(startRev))) { callback({stop: "startRev is not a number"}); @@ -1056,7 +1056,7 @@ exports.createDiffHTML = function(padID, startRev, endRev, callback) { } // check if endRev is a number - if (endRev !== undefined && typeof endRev != "number") { + if (endRev !== undefined && typeof endRev !== "number") { // try to parse the number if (isNaN(parseInt(endRev))) { callback({stop: "endRev is not a number"}); @@ -1119,13 +1119,13 @@ function is_int(value) // gets a pad safe function getPadSafe(padID, shouldExist, text, callback) { - if (typeof text == "function") { + if (typeof text === "function") { callback = text; text = null; } // check if padID is a string - if (typeof padID != "string") { + if (typeof padID !== "string") { callback(new customError("padID is not a string", "apierror")); return; } diff --git a/src/node/db/AuthorManager.js b/src/node/db/AuthorManager.js index 818eaab28..d04c4ca32 100644 --- a/src/node/db/AuthorManager.js +++ b/src/node/db/AuthorManager.js @@ -45,7 +45,7 @@ exports.doesAuthorExists = function(authorID, callback) db.get("globalAuthor:" + authorID, function(err, author) { if (ERR(err, callback)) return; - callback(null, author != null); + callback(null, author !== null); }); } @@ -98,7 +98,7 @@ function mapAuthorWithDBKey (mapperkey, mapper, callback) db.get(mapperkey + ":" + mapper, function(err, author) { if (ERR(err, callback)) return; - if (author == null) { + if (author === null) { // there is no author with this mapper, so create one exports.createAuthor(null, function(err, author) { if (ERR(err, callback)) return; @@ -212,7 +212,7 @@ exports.listPadsOfAuthor = function(authorID, callback) db.get("globalAuthor:" + authorID, function(err, author) { if (ERR(err, callback)) return; - if (author == null) { + if (author === null) { // author does not exist callback(new customError("authorID does not exist", "apierror")); @@ -242,7 +242,7 @@ exports.addPad = function(authorID, padID) // get the entry db.get("globalAuthor:" + authorID, function(err, author) { if (ERR(err)) return; - if (author == null) return; + if (author === null) return; if (author.padIDs == null) { // the entry doesn't exist so far, let's create it @@ -266,9 +266,9 @@ exports.removePad = function(authorID, padID) { db.get("globalAuthor:" + authorID, function(err, author) { if (ERR(err)) return; - if (author == null) return; + if (author === null) return; - if (author.padIDs != null) { + if (author.padIDs !== null) { // remove pad from author delete author.padIDs[padID]; db.set("globalAuthor:" + authorID, author); diff --git a/src/node/db/GroupManager.js b/src/node/db/GroupManager.js index f384f70aa..985883203 100644 --- a/src/node/db/GroupManager.js +++ b/src/node/db/GroupManager.js @@ -122,7 +122,7 @@ exports.deleteGroup = function(groupID, callback) if (ERR(err, callback)) return; groups = groups? groups.groupIDs : []; - if (groups.indexOf(groupID) == -1) { + if (groups.indexOf(groupID) === -1) { // it's not listed callback(); @@ -198,7 +198,7 @@ exports.createGroup = function(callback) exports.createGroupIfNotExistsFor = function(groupMapper, callback) { // ensure mapper is optional - if (typeof groupMapper != "string") { + if (typeof groupMapper !== "string") { callback(new customError("groupMapper is not a string", "apierror")); return; } @@ -248,7 +248,7 @@ exports.createGroupPad = function(groupID, padName, text, callback) exports.doesGroupExist(groupID, function(err, exists) { if (ERR(err, callback)) return; - if (exists == false) { + if (!exists) { // group does not exist callback(new customError("groupID does not exist", "apierror")); return; @@ -303,7 +303,7 @@ exports.listPads = function(groupID, callback) if (ERR(err, callback)) return; // ensure the group exists - if (exists == false) { + if (!exists) { callback(new customError("groupID does not exist", "apierror")); return; } diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index 117f2794f..bc8f59426 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -62,7 +62,7 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback) } } else { // a session is not required, so we'll check if it's a public pad - if (padID.indexOf("$") == -1) { + if (padID.indexOf("$") === -1) { // it's not a group pad, means we can grant access // get author for this token @@ -225,17 +225,17 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback) // --> grant access statusObject = { accessStatus: "grant", authorID: sessionAuthor }; - } else if (isPasswordProtected && passwordStatus == "correct") { + } else if (isPasswordProtected && passwordStatus === "correct") { // - the pad is password protected and password is correct // --> grant access statusObject = { accessStatus: "grant", authorID: sessionAuthor }; - } else if (isPasswordProtected && passwordStatus == "wrong") { + } else if (isPasswordProtected && passwordStatus === "wrong") { // - the pad is password protected but wrong password given // --> deny access, ask for new password and tell them that the password is wrong statusObject = { accessStatus: "wrongPassword" }; - } else if (isPasswordProtected && passwordStatus == "notGiven") { + } else if (isPasswordProtected && passwordStatus === "notGiven") { // - the pad is password protected but no password given // --> ask for password @@ -261,17 +261,17 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback) if (isPublic && !isPasswordProtected) { // --> grant access, with author of token statusObject = {accessStatus: "grant", authorID: tokenAuthor}; - } else if (isPublic && isPasswordProtected && passwordStatus == "correct") { + } else if (isPublic && isPasswordProtected && passwordStatus === "correct") { // - it's public and password protected and password is correct // --> grant access, with author of token statusObject = {accessStatus: "grant", authorID: tokenAuthor}; - } else if (isPublic && isPasswordProtected && passwordStatus == "wrong") { + } else if (isPublic && isPasswordProtected && passwordStatus === "wrong") { // - it's public and the pad is password protected but wrong password given // --> deny access, ask for new password and tell them that the password is wrong statusObject = {accessStatus: "wrongPassword"}; - } else if (isPublic && isPasswordProtected && passwordStatus == "notGiven") { + } else if (isPublic && isPasswordProtected && passwordStatus === "notGiven") { // - it's public and the pad is password protected but no password given // --> ask for password diff --git a/src/node/handler/APIHandler.js b/src/node/handler/APIHandler.js index 3ed7aa4e1..5d731ddc7 100644 --- a/src/node/handler/APIHandler.js +++ b/src/node/handler/APIHandler.js @@ -188,7 +188,7 @@ exports.handle = function(apiVersion, functionName, fields, req, res) // check the api key! fields["apikey"] = fields["apikey"] || fields["api_key"]; - if (fields["apikey"] != apikey.trim()) { + if (fields["apikey"] !== apikey.trim()) { res.statusCode = 401; res.send({code: 4, message: "no or wrong API Key", data: null}); return; diff --git a/src/node/handler/SocketIORouter.js b/src/node/handler/SocketIORouter.js index a94080b9d..08f9f47e8 100644 --- a/src/node/handler/SocketIORouter.js +++ b/src/node/handler/SocketIORouter.js @@ -95,7 +95,7 @@ exports.setSocketIO = function(_socket) { var checkAccessCallback = function(err, statusObject) { ERR(err); - if (statusObject.accessStatus == "grant") { + if (statusObject.accessStatus === "grant") { // access was granted, mark the client as authorized and handle the message clientAuthorized = true; handleMessage(client, message); diff --git a/src/node/padaccess.js b/src/node/padaccess.js index c8c888cd0..a25ad642c 100644 --- a/src/node/padaccess.js +++ b/src/node/padaccess.js @@ -6,7 +6,7 @@ module.exports = function (req, res, callback) { securityManager.checkAccess(req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, function(err, accessObj) { if (ERR(err, callback)) return; - if (accessObj.accessStatus == "grant") { + if (accessObj.accessStatus === "grant") { // there is access, continue callback(); } else {