transaction init consistency (#3315)

- transaction: always use conn.init_transaction, always pass in cfg #3315
- NOTICE: remove a handful of 3.0 sunset property names #3315
- outbound: pass in config when initiating txn
- outbound: minor es6 updates

Changes.md

@@ -1,26 +1,34 @@
 
 ### Unreleased
 
-#### Changed
+#### Added
 
 - doc: add CONTRIBUTORS #3312
+
+#### Changed
+
+- transaction: init with conn.init_transaction, always pass in cfg #3315
 - check for local_mx only when default route is used #3307
 - test: add a connection.response test case with DSN #3305
 - deps: bump all versions to latest #3303
 - auth_base: enable disabling constrain_sender at runtime #3298
 - connection: support IPv6 when setting remote.is_private #3295
+  - in setTLS, replace forEach with for...of
+  - NOTE: remove a handful of 3.0 sunset property names #3315
 - outbound/mx_lookup: make it async/await
-- outbound: emit log message when ignoring local MX
+- outbound: emit log message when ignoring local MX #3285
+- outbound: pass in config when initiating txn #3315
+- outbound: minor es6 updates #3315
 
 #### Fixed
 
 - fix(bin/haraka): list NPM installed plugin #3310
 - fix(bin/haraka): get hook list from doc/Plugins #3306
-- fix(outbound): call cb even if no MX is found
-- fix(helo.checks): declare reject.literal_mismatch as boolean
+- fix(outbound): call cb even if no MX is found #3294
+- fix(helo.checks): declare reject.literal_mismatch as boolean #3293
 - fix(outbound): allow LHLO over insecure socket if TLS is forced #3278
 - fix(outbound): include return path param SMTPUTF8 when required #3289
-- fix(outbound): replace empty Message-ID header
+- fix(outbound): replace empty Message-ID header #3288
 - fix(outbound): don't send SNI servername when connecting to an IP
 - fix(outbound): chown queue dir after creation #3291
 

connection.js

@@ -223,11 +223,10 @@ class Connection {
     setTLS (obj) {
         this.set('hello', 'host', undefined);
         this.set('tls',   'enabled', true);
-        const options = ['cipher','verified','verifyError','peerCertificate'];
-        options.forEach(t => {
+        for (const t of ['cipher','verified','verifyError','peerCertificate']) {
             if (obj[t] === undefined) return;
             this.set('tls', t, obj[t]);
-        })
+        }
         // prior to 2017-07, authorized and verified were both used. Verified
         // seems to be the more common and has the property updated in the
         // tls object. However, authorized has been up-to-date in the notes. Store
@@ -272,22 +271,6 @@ class Connection {
                 this.set('remote.is_private', net_utils.is_private_ip(this.remote.ip));
             }
         }
-
-        // sunset 3.0.0
-        if (prop_str === 'hello.verb') {
-            this.greeting = val;
-        }
-        else if (prop_str === 'tls.enabled') {
-            this.using_tls = val;
-        }
-        else if (prop_str === 'proxy.ip') {
-            this.haproxy_ip = val;
-        }
-        else {
-            const legacy_name = prop_str.split('.').join('_');
-            this[legacy_name] = val;
-        }
-        // /sunset
     }
     get (prop_str) {
         return prop_str.split('.').reduce((prev, curr) => {
@@ -638,7 +621,6 @@ class Connection {
         this.client.end();
     }
     get_capabilities () {
-
         return [];
     }
     tran_uuid () {

outbound/index.js

@@ -24,21 +24,20 @@ const _qfile = exports.qfile = require('./qfile');
 
 const { queue_dir, temp_fail_queue, delivery_queue } = queuelib;
 
+const smtp_ini = config.get('smtp.ini', { booleans: [ '+headers.add_received' ] })
+
 exports.temp_fail_queue = temp_fail_queue;
 exports.delivery_queue = delivery_queue;
 
 exports.net_utils = net_utils;
 exports.config = config;
 
-exports.get_stats = queuelib.get_stats;
-exports.list_queue = queuelib.list_queue;
-exports.stat_queue = queuelib.stat_queue;
-exports.scan_queue_pids = queuelib.scan_queue_pids;
-exports.flush_queue = queuelib.flush_queue;
-exports.load_pid_queue = queuelib.load_pid_queue;
-exports.ensure_queue_dir = queuelib.ensure_queue_dir;
-exports.load_queue = queuelib.load_queue;
-exports.stats = queuelib.stats;
+const qlfns = ['get_stats', 'list_queue', 'stat_queue', 'scan_queue_pids', 'flush_queue',
+    'load_pid_queue', 'ensure_queue_dir', 'load_queue', 'stats'
+]
+for (const n of qlfns) {
+    exports[n] = queuelib[n];
+}
 
 process.on('message', msg => {
     if (!msg.event) return
@@ -59,30 +58,24 @@ process.on('message', msg => {
     // ignores the message
 });
 
-exports.send_email = function () {
+exports.send_email = function (from, to, contents, next, options = {}) {
 
     if (arguments.length === 2) {
         logger.logdebug("[outbound] Sending email as a transaction");
         return this.send_trans_email(arguments[0], arguments[1]);
     }
 
-    let from = arguments[0];
-    let to   = arguments[1];
-    let contents = arguments[2];
-    const next = arguments[3];
-    const options = arguments[4] || {};
-
-    const dot_stuffed = options.dot_stuffed ? options.dot_stuffed : false;
-    const notes = options.notes ? options.notes : null;
-    const origin = options.origin ? options.origin : null;
+    const dot_stuffed = options.dot_stuffed ?? false;
+    const notes = options.notes ?? null;
+    const origin = options.origin ?? null;
 
     logger.loginfo("[outbound] Sending email via params", origin);
 
-    const transaction = trans.createTransaction();
+    const transaction = trans.createTransaction(null, smtp_ini);
 
     logger.loginfo(`[outbound] Created transaction: ${transaction.uuid}`, origin);
 
-    //Adding notes passed as parameter
+    // Adding notes passed as parameter
     if (notes) {
         transaction.notes = notes;
     }

package.json

@@ -74,7 +74,7 @@
   },
   "devDependencies": {
     "nodeunit-x": "^0.16.0",
-    "haraka-test-fixtures": "^1.3.3",
+    "haraka-test-fixtures": "^1.3.7",
     "mock-require": "^3.0.3",
     "eslint-plugin-haraka": "^1.0.15",
     "nodemailer": "^6.9.13"

plugins.js

@@ -312,7 +312,7 @@ plugins.load_plugins = override => {
         plugin_list = exports.config.get('plugins', 'list');
     }
 
-    plugin_list.forEach(plugin => {
+    for (let plugin of plugin_list) {
         if (plugin.startsWith('haraka-plugin-')) plugin = plugin.substr(14)
         if (plugins.deprecated[plugin]) {
             logger.lognotice(`the plugin ${plugin} has been replaced by '${plugins.deprecated[plugin]}'. Please update config/plugins`)
@@ -321,7 +321,7 @@ plugins.load_plugins = override => {
         else {
             plugins.load_plugin(plugin);
         }
-    });
+    }
 
     plugins.plugin_list = Object.keys(plugins.registered_plugins);
 

tests/connection.js

@@ -28,7 +28,7 @@ function _set_up (done) {
 exports.connectionRaw = {
     setUp : _set_up,
     'has remote object' (test) {
-        test.expect(5);
+        test.expect(1);
         test.deepEqual(this.connection.remote, {
             ip: null,
             port: null,
@@ -38,33 +38,23 @@ exports.connectionRaw = {
             is_private: false,
             is_local: false
         });
-        // backwards compat, sunset v3.0.0
-        test.equal(this.connection.remote_ip, null);
-        test.equal(this.connection.remote_port, null);
-        test.equal(this.connection.remote_host, null);
-        test.equal(this.connection.remote_info, null);
         test.done();
     },
     'has local object' (test) {
-        test.expect(5);
+        test.expect(3);
         test.equal(this.connection.local.ip, null);
         test.equal(this.connection.local.port, null);
         test.ok(this.connection.local.host, this.connection.local.host);
-        // backwards compat, sunset v3.0.0
-        test.equal(this.connection.local_ip, null);
-        test.equal(this.connection.local_port, null);
         test.done();
     },
     'has tls object' (test) {
-        test.expect(2);
+        test.expect(1);
         test.deepEqual(this.connection.tls, {
             enabled: false,
             advertised: false,
             verified: false,
             cipher: {},
         });
-        // backwards compat, sunset v3.0.0
-        test.equal(this.connection.using_tls, false);
         test.done();
     },
     'get_capabilities' (test) {
@@ -109,18 +99,6 @@ exports.connectionRaw = {
         test.equal('', this.connection.queue_msg('hello'));
         test.done();
     },
-    'has legacy connection properties' (test) {
-        test.expect(4);
-        this.connection.set('remote', 'ip', '172.16.15.1');
-        this.connection.set('hello', 'verb', 'EHLO');
-        this.connection.set('tls', 'enabled', true);
-
-        test.equal('172.16.15.1', this.connection.remote_ip);
-        test.equal(null, this.connection.remote_port);
-        test.equal('EHLO', this.connection.greeting);
-        test.equal(true, this.connection.using_tls);
-        test.done();
-    },
     'has normalized connection properties' (test) {
         test.expect(5);
         this.connection.set('remote', 'ip', '172.16.15.1');
@@ -140,12 +118,6 @@ exports.connectionRaw = {
         test.equal(false, this.connection.remote.is_local);
         test.done();
     },
-    'has legacy proxy property set' (test) {
-        test.expect(1);
-        this.connection.set('proxy', 'ip', '172.16.15.1');
-        test.equal('172.16.15.1', this.connection.haproxy_ip);
-        test.done();
-    },
     'has normalized proxy properties, default' (test) {
         test.expect(4);
         test.equal(false, this.connection.proxy.allowed);
@@ -355,4 +327,4 @@ exports.respond = {
         );
         test.done();
     },
-}
+}

tests/fixtures/util_hmailitem.js

@@ -2,9 +2,6 @@
 
 const { Address }     = require('address-rfc2821');
 const fixtures        = require('haraka-test-fixtures');
-const stub_connection = fixtures.connection;
-// const transaction     = fixtures.transaction;  // not yet sufficient
-const transaction     = require('../../transaction');
 
 
 /**
@@ -51,8 +48,8 @@ exports.createHMailItem = (outbound_context, options, callback) => {
     const delivery_domain = options.delivery_domain || 'domain';
     const mail_recipients = options.mail_recipients || [new Address('recipient@domain')];
 
-    const conn = stub_connection.createConnection();
-    conn.transaction = transaction.createTransaction('someuuid');
+    const conn = fixtures.connection.createConnection();
+    conn.init_transaction()
     conn.transaction.mail_from = new Address(mail_from);
 
     const todo = new outbound_context.TODOItem(delivery_domain, mail_recipients, conn.transaction);
@@ -144,7 +141,6 @@ exports.playTestSmtpConversation = (hmail, socket, test, playbook, callback) =>
 
     const welcome = getNextEntryFromPlaybook('remote', playbook);
     socket.emit('line', welcome.line);
-
 }
 
 function getNextEntryFromPlaybook (ofType, playbook) {

tests/plugins/queue/smtp_forward.js

@@ -18,7 +18,7 @@ function _setup (done) {
     this.hmail = { todo: { notes: new Notes() } };
 
     this.connection = new fixtures.connection.createConnection();
-    this.connection.transaction = new fixtures.transaction.createTransaction();
+    this.connection.init_transaction();
 
     done();
 }

tests/plugins/spamassassin.js

@@ -14,7 +14,7 @@ function _set_up (done) {
     };
 
     this.connection = fixtures.connection.createConnection();
-    this.connection.transaction = fixtures.transaction.createTransaction()
+    this.connection.init_transaction()
 
     done();
 }

tests/transaction.js

@@ -1,20 +1,16 @@
 const fs = require('fs');
 const path = require('path');
 
+const config = require('haraka-config')
 const transaction = require('../transaction');
 
 function _set_up (done) {
-    this.transaction = transaction.createTransaction();
-    done();
-}
-
-function _tear_down (done) {
-    done();
+    this.transaction = transaction.createTransaction(undefined, config.get('smtp.ini'));
+    done()
 }
 
 exports.transaction = {
     setUp : _set_up,
-    tearDown : _tear_down,
 
     'add_body_filter' (test) {
 
@@ -180,7 +176,6 @@ function write_file_data_to_transaction (test_transaction, filename) {
 
 exports.base64_handling = {
     setUp : _set_up,
-    tearDown: _tear_down,
 
     'varied-base64-fold-lengths-preserve-data' (test) {
 
@@ -230,7 +225,6 @@ exports.base64_handling = {
 // Expected: --abcd
 exports.boundarymarkercorrupt_test = {
     setUp : _set_up,
-    tearDown: _tear_down,
 
     // populate the same email data in transaction (self.transaction.add_data()) and
     // in raw buffer, then compare