Merge pull request #1680 from msimerson/1679-smtp_forward-multiple-rcpt

correct smtp_forward cfg for multiple rcpts

configfile.js

@@ -457,16 +457,20 @@ cfreader.load_ini_config = function(name, options) {
             if (match) {
                 is_array_match = regex.is_array.exec(match[1]);
                 if (is_array_match){
-                    setter = function(key, value){
+                    setter = function (key, value) {
                         key = key.replace('[]', '');
                         if (! current_sect[key]) current_sect[key] = [];
                         current_sect[key].push(value) };
                 }
                 else {
-                    setter = function(key, value) { current_sect[key] = value };
+                    setter = function (key, value) { current_sect[key] = value };
                 }
                 if (options && Array.isArray(options.booleans) &&
-                    bool_matches.indexOf(current_sect_name + '.' + match[1]) !== -1)
+                    (
+                        bool_matches.indexOf(current_sect_name + '.' + match[1]) !== -1
+                        ||
+                        bool_matches.indexOf('*.' + match[1]) !== -1
+                    ))
                 {
                     setter(match[1], regex.is_truth.test(match[2]));
                     logger.logdebug('Returning boolean ' + current_sect[match[1]] +

docs/plugins/queue/smtp_forward.md

@@ -8,8 +8,8 @@ other hosts, and inbound delivery to users.
 In comparison to `queue/smtp_proxy`, this plugin waits until queue time to
 attempt the ongoing connection. This can be a benefit in reducing connections
 to your inbound mail server when you have content filtering (such as
-spamassassin) enabled. However you miss out on the benefits of recipient
-filtering that the ongoing mail server may provide.
+spamassassin) enabled. A possible downside is that it also delays recipient
+validation that the ongoing mail server may provide until queue time.
 
 Configuration
 -------------
@@ -67,7 +67,7 @@ Configuration
 
 More specific forward routes for domains can be defined. More specific routes
 are only honored for SMTP connections with a single recipient or SMTP
-connections where every recipient is identical.
+connections where every recipient host is identical.
 
     # default SMTP host
     host=1.2.3.4
@@ -92,6 +92,8 @@ directly to the specified host. Messages with recipients only in the domains
 example1.com and example2.com will get delivered directly to 1.2.3.5.
 Everything else gets delivered to 1.2.3.4.
 
+## Per-Domain Limitations
+
 See [GitHub Issue #573](https://github.com/haraka/Haraka/issues/573) for
 background on the limitations of smtp-forward with recipients in different
 domains.

plugins/queue/smtp_forward.js

@@ -21,7 +21,7 @@ exports.load_smtp_forward_ini = function () {
 
     plugin.cfg = plugin.config.get('smtp_forward.ini', {
         booleans: [
-            '-main.enable_tls',
+            '-*.enable_tls',
             '+main.enable_outbound',
         ],
     },
@@ -43,10 +43,12 @@ exports.get_config = function (connection) {
     var rcpt_count = connection.transaction.rcpt_to.length;
     if (rcpt_count === 1) { return plugin.cfg[dom]; }
 
-    var dst_host = plugin.cfg[dom].host;
     for (var i=1; i < rcpt_count; i++) {
-        if (connection.transaction.rcpt_to[i].host !== dst_host) {
-            return plugin.cfg.main;
+        var dom2 = connection.transaction.rcpt_to[i].host;
+        if (!dom2 || !plugin.cfg[dom2]) return plugin.cfg.main;
+        if (plugin.cfg[dom2].host !== plugin.cfg[dom].host) {
+            // differing destination hosts
+            return plugin.cfg.main; // return default config
         }
     }
     return plugin.cfg[dom];

tests/config/smtp_forward.ini

@@ -0,0 +1,36 @@
+; host to connect to
+host=localhost
+;
+; port to connect to
+port=2555
+;
+; timeout backend connection from pool
+;timeout=300
+;
+; max connections in pool
+;max_connections=1000
+;
+; uncomment to enable TLS to the backend SMTP server
+enable_tls=true
+;
+; for messages that have multiple RCPT, send a separate message for each RCPT
+; when forwarding.
+one_message_per_rcpt=true
+;
+; uncomment to use smtp client authorization
+;auth_type=plain
+;auth_user=
+;auth_pass=
+
+[test.com]
+host=1.2.3.4
+enable_tls=true
+auth_user=postmaster@test.com
+auth_pass=superDuperSecret
+
+[test1.com]
+host=1.2.3.4
+enable_tls=false
+
+[test2.com]
+host=2.3.4.5

tests/plugins/queue/smtp_forward.js

@@ -1,15 +1,27 @@
 'use strict';
 
+var path         = require('path');
+
 var Address      = require('address-rfc2821').Address;
 var fixtures     = require('haraka-test-fixtures');
 
 var Connection   = fixtures.connection;
 
+function _setup (done) {
+    this.plugin = new fixtures.plugin('queue/smtp_forward');
+
+    // switch config directory to 'tests/config'
+    this.plugin.config = this.plugin.config.module_config(path.resolve('tests'));
+    this.plugin.register();
+
+    this.connection = Connection.createConnection();
+    this.connection.transaction = fixtures.transaction.createTransaction();
+
+    done();
+}
+
 exports.register = {
-    setUp : function (done) {
-        this.plugin = new fixtures.plugin('queue/smtp_forward');
-        done();
-    },
+    setUp : _setup,
     'register': function (test) {
         test.expect(1);
         this.plugin.register();
@@ -19,45 +31,76 @@ exports.register = {
 };
 
 exports.get_config = {
-    setUp : function (done) {
-        this.plugin = new fixtures.plugin('queue/smtp_forward');
-        this.plugin.register();
-
-        this.connection = Connection.createConnection();
-        this.connection.transaction = { rcpt_to: [] };
-
-        done();
-    },
+    setUp : _setup,
     'no recipient': function (test) {
-        test.expect(1);
+        test.expect(2);
         var cfg = this.plugin.get_config(this.connection);
-        test.ok(cfg.enable_tls !== undefined);
+        test.equal(cfg.host, 'localhost');
+        test.equal(cfg.enable_tls, true);
         test.done();
     },
     'null recipient': function (test) {
-        test.expect(1);
+        test.expect(2);
         this.connection.transaction.rcpt_to.push(new Address('<>'));
         var cfg = this.plugin.get_config(this.connection);
-        test.ok(cfg.enable_tls !== undefined);
+        test.equal(cfg.host, 'localhost');
+        test.equal(cfg.enable_tls, true);
         test.done();
     },
     'valid recipient': function (test) {
+        test.expect(2);
+        this.connection.transaction.rcpt_to.push(
+            new Address('<matt@example.com>')
+            );
+        var cfg = this.plugin.get_config(this.connection);
+        test.equal(cfg.enable_tls, true);
+        test.equal(cfg.host, 'localhost');
+        test.done();
+    },
+    'valid recipient with route': function (test) {
         test.expect(1);
         this.connection.transaction.rcpt_to.push(
             new Address('<matt@test.com>')
             );
         var cfg = this.plugin.get_config(this.connection);
-        test.ok(cfg.enable_tls !== undefined);
+        test.deepEqual(cfg, {
+            host: '1.2.3.4',
+            enable_tls: true,
+            auth_user: 'postmaster@test.com',
+            auth_pass: 'superDuperSecret',
+        });
         test.done();
     },
-    'valid recipient with route': function (test) {
+    'valid recipient with route & diff config': function (test) {
         test.expect(1);
-        this.plugin.cfg['test.com'] = { host: '1.2.3.4' };
         this.connection.transaction.rcpt_to.push(
+            new Address('<matt@test1.com>')
+            );
+        var cfg = this.plugin.get_config(this.connection);
+        test.deepEqual(cfg, {
+            host: '1.2.3.4',
+            enable_tls: false,
+        });
+        test.done();
+    },
+    'valid 2 recipients with same route': function (test) {
+        test.expect(1);
+        this.connection.transaction.rcpt_to.push(
+            new Address('<matt@test.com>'),
             new Address('<matt@test.com>')
             );
         var cfg = this.plugin.get_config(this.connection);
-        test.ok(cfg.host === '1.2.3.4' );
+        test.deepEqual(cfg.host, '1.2.3.4' );
+        test.done();
+    },
+    'valid 2 recipients with different routes': function (test) {
+        test.expect(1);
+        this.connection.transaction.rcpt_to.push(
+            new Address('<matt@test1.com>'),
+            new Address('<matt@test2.com>')
+            );
+        var cfg = this.plugin.get_config(this.connection);
+        test.equal(cfg.host, 'localhost' );
         test.done();
     },
 };