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.
This commit is contained in:
muxator 2019-03-01 09:43:41 +01:00
parent e841798314
commit 11453d544c
7 changed files with 36 additions and 36 deletions

View file

@ -192,7 +192,7 @@ Example returns:
exports.getText = function(padID, rev, callback) exports.getText = function(padID, rev, callback)
{ {
// check if rev is a number // check if rev is a number
if (rev !== undefined && typeof rev != "number") { if (rev !== undefined && typeof rev !== "number") {
// try to parse the number // try to parse the number
if (isNaN(parseInt(rev))) { if (isNaN(parseInt(rev))) {
callback(new customError("rev is not a number", "apierror")); callback(new customError("rev is not a number", "apierror"));
@ -256,7 +256,7 @@ Example returns:
exports.setText = function(padID, text, callback) exports.setText = function(padID, text, callback)
{ {
// text is required // text is required
if (typeof text != "string") { if (typeof text !== "string") {
callback(new customError("text is not a string", "apierror")); callback(new customError("text is not a string", "apierror"));
return; return;
} }
@ -285,7 +285,7 @@ Example returns:
exports.appendText = function(padID, text, callback) exports.appendText = function(padID, text, callback)
{ {
// text is required // text is required
if (typeof text != "string") { if (typeof text !== "string") {
callback(new customError("text is not a string", "apierror")); callback(new customError("text is not a string", "apierror"));
return; return;
} }
@ -311,7 +311,7 @@ Example returns:
*/ */
exports.getHTML = function(padID, rev, callback) exports.getHTML = function(padID, rev, callback)
{ {
if (rev !== undefined && typeof rev != "number") { if (rev !== undefined && typeof rev !== "number") {
if (isNaN(parseInt(rev))) { if (isNaN(parseInt(rev))) {
callback(new customError("rev is not a number", "apierror")); callback(new customError("rev is not a number", "apierror"));
return; return;
@ -375,7 +375,7 @@ Example returns:
exports.setHTML = function(padID, html, callback) exports.setHTML = function(padID, html, callback)
{ {
// html is required // html is required
if (typeof html != "string") { if (typeof html !== "string") {
callback(new customError("html is not a string", "apierror")); callback(new customError("html is not a string", "apierror"));
return; return;
} }
@ -471,7 +471,7 @@ Example returns:
exports.appendChatMessage = function(padID, text, authorID, time, callback) exports.appendChatMessage = function(padID, text, authorID, time, callback)
{ {
// text is required // text is required
if (typeof text != "string") { if (typeof text !== "string") {
callback(new customError("text is not a string", "apierror")); callback(new customError("text is not a string", "apierror"));
return; return;
} }
@ -557,7 +557,7 @@ Example returns:
exports.saveRevision = function(padID, rev, callback) exports.saveRevision = function(padID, rev, callback)
{ {
// check if rev is a number // check if rev is a number
if (rev !== undefined && typeof rev != "number") { if (rev !== undefined && typeof rev !== "number") {
// try to parse the number // try to parse the number
if (isNaN(parseInt(rev))) { if (isNaN(parseInt(rev))) {
callback(new customError("rev is not a number", "apierror")); callback(new customError("rev is not a number", "apierror"));
@ -636,7 +636,7 @@ exports.createPad = function(padID, text, callback)
{ {
if (padID) { if (padID) {
// ensure there is no $ in the 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")); callback(new customError("createPad can't create group pads", "apierror"));
return; return;
} }
@ -682,7 +682,7 @@ exports.deletePad = function(padID, callback)
exports.restoreRevision = function(padID, rev, callback) exports.restoreRevision = function(padID, rev, callback)
{ {
// check if rev is a number // check if rev is a number
if (rev !== undefined && typeof rev != "number") { if (rev !== undefined && typeof rev !== "number") {
// try to parse the number // try to parse the number
if (isNaN(parseInt(rev))) { if (isNaN(parseInt(rev))) {
callback(new customError("rev is not a number", "apierror")); callback(new customError("rev is not a number", "apierror"));
@ -838,7 +838,7 @@ exports.getPadID = function(roID, callback)
readOnlyManager.getPadId(roID, function(err, retrievedPadID) { readOnlyManager.getPadId(roID, function(err, retrievedPadID) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
if (retrievedPadID == null) { if (retrievedPadID === null) {
callback(new customError("padID does not exist", "apierror")); callback(new customError("padID does not exist", "apierror"));
return; return;
} }
@ -858,7 +858,7 @@ Example returns:
exports.setPublicStatus = function(padID, publicStatus, callback) exports.setPublicStatus = function(padID, publicStatus, callback)
{ {
// ensure this is a group pad // 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")); callback(new customError("You can only get/set the publicStatus of pads that belong to a group", "apierror"));
return; return;
} }
@ -868,7 +868,7 @@ exports.setPublicStatus = function(padID, publicStatus, callback)
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
// convert string to boolean // convert string to boolean
if (typeof publicStatus == "string") if (typeof publicStatus === "string")
publicStatus = publicStatus == "true" ? true : false; publicStatus = publicStatus == "true" ? true : false;
// set the password // set the password
@ -1045,7 +1045,7 @@ Example returns:
*/ */
exports.createDiffHTML = function(padID, startRev, endRev, callback) { exports.createDiffHTML = function(padID, startRev, endRev, callback) {
// check if startRev is a number // check if startRev is a number
if (startRev !== undefined && typeof startRev != "number") { if (startRev !== undefined && typeof startRev !== "number") {
// try to parse the number // try to parse the number
if (isNaN(parseInt(startRev))) { if (isNaN(parseInt(startRev))) {
callback({stop: "startRev is not a number"}); callback({stop: "startRev is not a number"});
@ -1056,7 +1056,7 @@ exports.createDiffHTML = function(padID, startRev, endRev, callback) {
} }
// check if endRev is a number // check if endRev is a number
if (endRev !== undefined && typeof endRev != "number") { if (endRev !== undefined && typeof endRev !== "number") {
// try to parse the number // try to parse the number
if (isNaN(parseInt(endRev))) { if (isNaN(parseInt(endRev))) {
callback({stop: "endRev is not a number"}); callback({stop: "endRev is not a number"});
@ -1119,13 +1119,13 @@ function is_int(value)
// gets a pad safe // gets a pad safe
function getPadSafe(padID, shouldExist, text, callback) function getPadSafe(padID, shouldExist, text, callback)
{ {
if (typeof text == "function") { if (typeof text === "function") {
callback = text; callback = text;
text = null; text = null;
} }
// check if padID is a string // check if padID is a string
if (typeof padID != "string") { if (typeof padID !== "string") {
callback(new customError("padID is not a string", "apierror")); callback(new customError("padID is not a string", "apierror"));
return; return;
} }

View file

@ -45,7 +45,7 @@ exports.doesAuthorExists = function(authorID, callback)
db.get("globalAuthor:" + authorID, function(err, author) { db.get("globalAuthor:" + authorID, function(err, author) {
if (ERR(err, callback)) return; 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) { db.get(mapperkey + ":" + mapper, function(err, author) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
if (author == null) { if (author === null) {
// there is no author with this mapper, so create one // there is no author with this mapper, so create one
exports.createAuthor(null, function(err, author) { exports.createAuthor(null, function(err, author) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
@ -212,7 +212,7 @@ exports.listPadsOfAuthor = function(authorID, callback)
db.get("globalAuthor:" + authorID, function(err, author) { db.get("globalAuthor:" + authorID, function(err, author) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
if (author == null) { if (author === null) {
// author does not exist // author does not exist
callback(new customError("authorID does not exist", "apierror")); callback(new customError("authorID does not exist", "apierror"));
@ -242,7 +242,7 @@ exports.addPad = function(authorID, padID)
// get the entry // get the entry
db.get("globalAuthor:" + authorID, function(err, author) { db.get("globalAuthor:" + authorID, function(err, author) {
if (ERR(err)) return; if (ERR(err)) return;
if (author == null) return; if (author === null) return;
if (author.padIDs == null) { if (author.padIDs == null) {
// the entry doesn't exist so far, let's create it // 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) { db.get("globalAuthor:" + authorID, function(err, author) {
if (ERR(err)) return; if (ERR(err)) return;
if (author == null) return; if (author === null) return;
if (author.padIDs != null) { if (author.padIDs !== null) {
// remove pad from author // remove pad from author
delete author.padIDs[padID]; delete author.padIDs[padID];
db.set("globalAuthor:" + authorID, author); db.set("globalAuthor:" + authorID, author);

View file

@ -122,7 +122,7 @@ exports.deleteGroup = function(groupID, callback)
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
groups = groups? groups.groupIDs : []; groups = groups? groups.groupIDs : [];
if (groups.indexOf(groupID) == -1) { if (groups.indexOf(groupID) === -1) {
// it's not listed // it's not listed
callback(); callback();
@ -198,7 +198,7 @@ exports.createGroup = function(callback)
exports.createGroupIfNotExistsFor = function(groupMapper, callback) exports.createGroupIfNotExistsFor = function(groupMapper, callback)
{ {
// ensure mapper is optional // ensure mapper is optional
if (typeof groupMapper != "string") { if (typeof groupMapper !== "string") {
callback(new customError("groupMapper is not a string", "apierror")); callback(new customError("groupMapper is not a string", "apierror"));
return; return;
} }
@ -248,7 +248,7 @@ exports.createGroupPad = function(groupID, padName, text, callback)
exports.doesGroupExist(groupID, function(err, exists) { exports.doesGroupExist(groupID, function(err, exists) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
if (exists == false) { if (!exists) {
// group does not exist // group does not exist
callback(new customError("groupID does not exist", "apierror")); callback(new customError("groupID does not exist", "apierror"));
return; return;
@ -303,7 +303,7 @@ exports.listPads = function(groupID, callback)
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
// ensure the group exists // ensure the group exists
if (exists == false) { if (!exists) {
callback(new customError("groupID does not exist", "apierror")); callback(new customError("groupID does not exist", "apierror"));
return; return;
} }

View file

@ -62,7 +62,7 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback)
} }
} else { } else {
// a session is not required, so we'll check if it's a public pad // 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 // it's not a group pad, means we can grant access
// get author for this token // get author for this token
@ -225,17 +225,17 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback)
// --> grant access // --> grant access
statusObject = { accessStatus: "grant", authorID: sessionAuthor }; statusObject = { accessStatus: "grant", authorID: sessionAuthor };
} else if (isPasswordProtected && passwordStatus == "correct") { } else if (isPasswordProtected && passwordStatus === "correct") {
// - the pad is password protected and password is correct // - the pad is password protected and password is correct
// --> grant access // --> grant access
statusObject = { accessStatus: "grant", authorID: sessionAuthor }; statusObject = { accessStatus: "grant", authorID: sessionAuthor };
} else if (isPasswordProtected && passwordStatus == "wrong") { } else if (isPasswordProtected && passwordStatus === "wrong") {
// - the pad is password protected but wrong password given // - the pad is password protected but wrong password given
// --> deny access, ask for new password and tell them that the password is wrong // --> deny access, ask for new password and tell them that the password is wrong
statusObject = { accessStatus: "wrongPassword" }; statusObject = { accessStatus: "wrongPassword" };
} else if (isPasswordProtected && passwordStatus == "notGiven") { } else if (isPasswordProtected && passwordStatus === "notGiven") {
// - the pad is password protected but no password given // - the pad is password protected but no password given
// --> ask for password // --> ask for password
@ -261,17 +261,17 @@ exports.checkAccess = function(padID, sessionCookie, token, password, callback)
if (isPublic && !isPasswordProtected) { if (isPublic && !isPasswordProtected) {
// --> grant access, with author of token // --> grant access, with author of token
statusObject = {accessStatus: "grant", authorID: tokenAuthor}; 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 // - it's public and password protected and password is correct
// --> grant access, with author of token // --> grant access, with author of token
statusObject = {accessStatus: "grant", authorID: tokenAuthor}; 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 // - 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 // --> deny access, ask for new password and tell them that the password is wrong
statusObject = {accessStatus: "wrongPassword"}; 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 // - it's public and the pad is password protected but no password given
// --> ask for password // --> ask for password

View file

@ -188,7 +188,7 @@ exports.handle = function(apiVersion, functionName, fields, req, res)
// check the api key! // check the api key!
fields["apikey"] = fields["apikey"] || fields["api_key"]; fields["apikey"] = fields["apikey"] || fields["api_key"];
if (fields["apikey"] != apikey.trim()) { if (fields["apikey"] !== apikey.trim()) {
res.statusCode = 401; res.statusCode = 401;
res.send({code: 4, message: "no or wrong API Key", data: null}); res.send({code: 4, message: "no or wrong API Key", data: null});
return; return;

View file

@ -95,7 +95,7 @@ exports.setSocketIO = function(_socket) {
var checkAccessCallback = function(err, statusObject) { var checkAccessCallback = function(err, statusObject) {
ERR(err); ERR(err);
if (statusObject.accessStatus == "grant") { if (statusObject.accessStatus === "grant") {
// access was granted, mark the client as authorized and handle the message // access was granted, mark the client as authorized and handle the message
clientAuthorized = true; clientAuthorized = true;
handleMessage(client, message); handleMessage(client, message);

View file

@ -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) { securityManager.checkAccess(req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, function(err, accessObj) {
if (ERR(err, callback)) return; if (ERR(err, callback)) return;
if (accessObj.accessStatus == "grant") { if (accessObj.accessStatus === "grant") {
// there is access, continue // there is access, continue
callback(); callback();
} else { } else {