SonarLint pass against detected bugs and code smell issues

connection.js

@@ -1182,23 +1182,23 @@ class Connection {
         const src_port = match[4];
         const dst_port = match[5];
 
+        // TODO: is it at all possible to avoid the ESLint disable here?
         // Validate source/destination IP
         /*eslint no-fallthrough: 0 */
         switch (proto) {
             case 'TCP4':
                 if (ipaddr.IPv4.isValid(src_ip) && ipaddr.IPv4.isValid(dst_ip)) {
-                    break;
+                    break; // Valid IPv4 address
                 }
             case 'TCP6':
                 if (ipaddr.IPv6.isValid(src_ip) && ipaddr.IPv6.isValid(dst_ip)) {
-                    break;
+                    break; // Valid IPv6 address
                 }
-            // case 'UNKNOWN':
+            case 'UNKNOWN':
             default:
                 this.respond(421, 'Invalid PROXY format');
-                return this.disconnect();
+                return this.disconnect(); // Invalid address
         }
-
         // Apply changes
         this.loginfo(
             'HAProxy',

dkim.js

@@ -273,7 +273,7 @@ class DKIMObject {
         }
 
         // Now add in our original DKIM-Signature header without the b= and trailing CRLF
-        let our_sig = this.sig.replace(/([:;\s\t]|^)b=([^;]+)/, '$1b=');
+        let our_sig = this.sig.replace(/([:;\s]|^)b=([^;]+)/, '$1b=');
         if (this.headercanon === 'relaxed') {
             our_sig = this.header_canon_relaxed(our_sig);
         }
@@ -302,11 +302,7 @@ class DKIMObject {
                 }
             }
             if (!res) return this.result('no key for signature', 'invalid');
-            for (let record of res) {
-                // Node 0.11.x compatibility
-                if (Array.isArray(record)) {
-                    record = record.join('');
-                }
+            for (const record of res) {
                 if (!record.includes('p=')) {
                     this.debug(`${this.identity}: ignoring TXT record: ${record}`);
                     continue;

logger.js

@@ -141,18 +141,19 @@ logger.log = (level, data, logobj) => {
 
     // buffer until plugins are loaded
     const emptyPluginList = !plugins || Array.isArray(plugins.plugin_list) && !plugins.plugin_list.length;
+
     if (emptyPluginList) {
         logger.deferred_logs.push(item);
-        return true;
     }
-
-    // process buffered logs
-    while (logger.deferred_logs.length > 0) {
-        const log_item = logger.deferred_logs.shift();
-        plugins.run_hooks('log', logger, log_item);
+    else {
+        // process buffered logs
+        while (logger.deferred_logs.length > 0) {
+            const log_item = logger.deferred_logs.shift();
+            plugins.run_hooks('log', logger, log_item);
+        }
+        plugins.run_hooks('log', logger, item);
     }
 
-    plugins.run_hooks('log', logger, item);
     return true;
 }
 
@@ -174,7 +175,7 @@ logger.log_respond = (retval, msg, data) => {
 }
 
 logger.set_loglevel = function (level) {
-    if (level === undefined || level === null) return;
+    if (level == null) return;
 
     const loglevel_num = parseInt(level);
     if (typeof level === 'string') {
@@ -214,10 +215,7 @@ logger._init_loglevel = function () {
     this.set_loglevel(_loglevel);
 }
 
-logger.would_log = level => {
-    if (logger.loglevel < level) { return false; }
-    return true;
-}
+logger.would_log = level => logger.loglevel >= level
 
 logger.set_timestamps = value => {
     logger.timestamps = !!value;

outbound/hmail.js

@@ -205,7 +205,7 @@ class HMailItem extends events.EventEmitter {
                     // assume string
                     const matches = /^(.*?)(:(\d+))?$/.exec(mx);
                     if (!matches) {
-                        throw ("get_mx returned something that doesn't match hostname or hostname:port");
+                        throw (new Error("get_mx returned something that doesn't match hostname or hostname:port"));
                     }
                     mx_list = [{priority: 0, exchange: matches[1], port: matches[3]}];
                 }
@@ -1008,14 +1008,12 @@ class HMailItem extends events.EventEmitter {
      * @param cb - a callback for fn(err, message_body_lines)
      */
     populate_bounce_message_with_headers (from, to, reason, header, cb) {
-        const CRLF = '\r\n';
-
         const originalMessageId = header.get('Message-Id');
-
         const bounce_msg_ = config.get('outbound.bounce_message', 'data');
         const bounce_msg_html_ = config.get('outbound.bounce_message_html', 'data');
         const bounce_msg_image_ = config.get('outbound.bounce_message_image', 'data');
 
+        const CRLF = '\r\n';
         const bounce_header_lines = [];
         const bounce_body_lines = [];
         const bounce_html_lines = [];
@@ -1042,15 +1040,15 @@ class HMailItem extends events.EventEmitter {
         bounce_msg_.forEach(line => {
             line = line.replace(/\{(\w+)\}/g, (i, word) => values[word] || '?');
 
-            if (bounce_headers_done == false && line == '') {
+            if (!bounce_headers_done && line == '') {
                 bounce_headers_done = true;
             }
-            else if (bounce_headers_done == false) {
-                bounce_header_lines.push(line);
-            }
-            else if (bounce_headers_done == true) {
+            else if (bounce_headers_done) {
                 bounce_body_lines.push(line);
             }
+            else {
+                bounce_header_lines.push(line);
+            }
         });
 
         const escaped_chars = {

outbound/index.js

@@ -43,18 +43,17 @@ exports.stats = queuelib.stats;
 process.on('message', msg => {
     if (!msg.event) return
 
-    if (msg.event === 'outbound.load_pid_queue') {
-        exports.load_pid_queue(msg.data);
-        return;
-    }
-    if (msg.event === 'outbound.flush_queue') {
-        exports.flush_queue(msg.domain, process.pid);
-        return;
-    }
-    if (msg.event === 'outbound.shutdown') {
-        logger.loginfo("[outbound] Shutting down temp fail queue");
-        temp_fail_queue.shutdown();
-        return;
+    switch (msg.event) {
+        case 'outbound.load_pid_queue':
+            exports.load_pid_queue(msg.data);
+            return;
+        case 'outbound.flush_queue':
+            exports.flush_queue(msg.domain, process.pid);
+            return;
+        case 'outbound.shutdown':
+            logger.loginfo("[outbound] Shutting down temp fail queue");
+            temp_fail_queue.shutdown();
+            break;
     }
     // ignores the message
 });

outbound/queue.js

@@ -100,13 +100,13 @@ exports._load_cur_queue = (pid, iteratee, cb) => {
 exports.read_parts = file => {
     if (file.indexOf(_qfile.platformDOT) === 0) {
         logger.logwarn(`[outbound] 'Skipping' dot-file in queue folder: ${file}`);
-        return false;
+        return null;
     }
 
     const parts = _qfile.parts(file);
     if (!parts) {
         logger.logerror(`[outbound] Unrecognized file in queue folder: ${file}`);
-        return false;
+        return null;
     }
 
     return parts;

outbound/timer_queue.js

@@ -41,7 +41,7 @@ class TimerQueue {
             }
         }
 
-        throw "Should never get here";
+        throw new Error("Should never get here");
     }
 
     discard (id) {
@@ -52,7 +52,7 @@ class TimerQueue {
             }
         }
 
-        throw `${id} not found`;
+        throw new Error(`${id} not found`);
     }
 
     fire () {

plugins.js

@@ -198,7 +198,7 @@ class Plugin {
                 logger.logcrit(`Loading plugin ${this.name} failed: ${err}`);
                 return;
             }
-            throw `Loading plugin ${this.name} failed: ${err}`;
+            throw new Error(`Loading plugin ${this.name} failed: ${err}`);
         }
     }
 

plugins/auth/auth_base.js

@@ -58,7 +58,7 @@ exports.check_plain_passwd = function (connection, user, passwd, cb) {
         this.get_plain_passwd(user, connection, callback);
     }
     else {
-        throw 'Invalid number of arguments for get_plain_passwd';
+        throw new Error('Invalid number of arguments for get_plain_passwd');
     }
 }
 
@@ -80,7 +80,7 @@ exports.check_cram_md5_passwd = function (connection, user, passwd, cb) {
         this.get_plain_passwd(user, connection, callback);
     }
     else {
-        throw 'Invalid number of arguments for get_plain_passwd';
+        throw new Error('Invalid number of arguments for get_plain_passwd');
     }
 }
 

plugins/auth/auth_proxy.js

@@ -1,7 +1,8 @@
 // Proxy AUTH requests selectively by domain
 const sock  = require('./line_socket');
 const utils = require('haraka-utils');
-const smtp_regexp = /^([0-9]{3})([ -])(.*)/;
+
+const smtp_regexp = /^(\d{3})([ -])(.*)/;
 
 exports.register = function () {
     this.inherits('auth/auth_base');
@@ -25,8 +26,8 @@ exports.hook_capabilities = (next, connection) => {
 }
 
 exports.check_plain_passwd = function (connection, user, passwd, cb) {
-    let domain;
-    if ((domain = /@([^@]+)$/.exec(user))) {
+    let domain = /@([^@]+)$/.exec(user);
+    if (domain) {
         domain = domain[1].toLowerCase();
     }
     else {
@@ -82,7 +83,6 @@ exports.try_auth_proxy = function (connection, hosts, user, passwd, cb) {
     socket.on('error', err => {
         connection.logerror(self, `connection failed to host ${host}: ${err}`);
         socket.end();
-        return;
     });
     socket.send_command = function (cmd, data) {
         let line = cmd + (data ? (` ${data}`) : '');
@@ -175,8 +175,8 @@ exports.try_auth_proxy = function (connection, hosts, user, passwd, cb) {
             }
             if (code.startsWith('5')) {
                 // Initial attempt failed; strip domain and retry.
-                let u;
-                if ((u = /^([^@]+)@.+$/.exec(user))) {
+                const u = /^([^@]+)@.+$/.exec(user)
+                if (u) {
                     user = u[1];
                     if (methods.includes('PLAIN')) {
                         socket.send_command('AUTH', `PLAIN ${utils.base64(`\0${user}\0${passwd}`)}`);

plugins/avg.js

@@ -5,9 +5,9 @@
 
 const fs   = require('fs');
 const path = require('path');
-
 const sock = require('./line_socket');
-const smtp_regexp = /^([0-9]{3})([ -])(.*)/;
+
+const smtp_regexp = /^(\d{3})([ -])(.*)/;
 
 exports.register = function () {
 

plugins/clamd.js

@@ -363,12 +363,12 @@ exports.send_clamd_predata = (socket, cb) => {
 }
 
 function clamd_connect (socket, host) {
-    let match;
+    const match = /^\[([^\] ]+)\](?::(\d+))?/.exec(host);
     if (host.match(/^\//)) {
         // assume unix socket
         socket.connect(host);
     }
-    else if ((match = /^\[([^\] ]+)\](?::(\d+))?/.exec(host))) {
+    else if (match) {
         // IPv6 literal
         socket.connect((match[2] || 3310), match[1]);
     }

plugins/delay_deny.js

@@ -117,16 +117,13 @@ exports.hook_rcpt_ok = function (next, connection, rcpt) {
 
     // Then check transaction level pre-DATA
     if (transaction.notes?.delay_deny_pre) {
-        for (let i=0; i<transaction.notes.delay_deny_pre.length; i++) {
-            const params = transaction.notes.delay_deny_pre[i];
-
+        transaction.notes.delay_deny_pre.forEach((params, i) => {
             // Remove rejection from the array if it was on the rcpt hooks
             if (params[5] === 'rcpt' || params[5] === 'rcpt_ok') {
                 transaction.notes.delay_deny_pre.splice(i, 1);
             }
-
             return next(params[0], params[1]);
-        }
+        });
     }
     return next();
 }

plugins/esets.js

@@ -1,7 +1,7 @@
 // esets
 const fs = require('fs');
 const child_process = require('child_process');
-const virus_re = new RegExp('virus="([^"]+)"');
+const virus_re = /virus="([^"]+)"/;
 
 exports.hook_data_post = function (next, connection) {
     const plugin = this;
@@ -39,8 +39,8 @@ exports.hook_data_post = function (next, connection) {
         });
 
         // Get virus name
-        let virus;
-        if ((virus = virus_re.exec(stdout))) {
+        let virus = virus_re.exec(stdout);
+        if (virus) {
             virus = virus[1];
         }
 

plugins/greylist.js

@@ -342,7 +342,7 @@ exports.process_skip_rules = function (connection) {
         }
     }
 
-    return false;
+    return null;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -389,10 +389,12 @@ exports.craft_hostid = function (connection) {
 
         value = value || remote.ip;
 
+        // TODO: check against https://rules.sonarsource.com/javascript/RSPEC-1121
         return ((transaction.notes.greylist = transaction.notes.greylist || {}).hostid = value);
     }
 
-    // no rDNS . FIXME: use fcrdns results
+    // no rDNS
+    // FIXME: use fcrdns results; check against https://rules.sonarsource.com/javascript/RSPEC-1529
     if (!remote.host || [ 'Unknown' | 'DNSERROR' ].includes(remote.host))
         return chsit(null, 'no rDNS info for this host');
 
@@ -635,11 +637,8 @@ exports.domain_in_list = function (list_name, domain) {
 exports.check_rdns_for_special_cases = function (domain, label) {
 
     // ptr for these is in fact dynamic
-    if (this.domain_in_list('dyndom', domain))
-        return {
-            type : 'dynamic',
-            why : 'rDNS considered dynamic: listed in dynamic.domains config list'
-        };
-
-    return false;
+    return this.domain_in_list('dyndom', domain) ? {
+        type : 'dynamic',
+        why : 'rDNS considered dynamic: listed in dynamic.domains config list'
+    } : null;
 }