Skip to content
Browse files

bounce: move config loading into register

check config at connection, with callback (was at hook_mail)
load bounce_bad_rcpt into object, for *much* faster lookups
added non_local_msgid check, disabled by default
  • Loading branch information...
1 parent cd0c55f commit 499bfb7940cb0ed78de65d70f1dfdc8900923293 @msimerson msimerson committed Jun 22, 2014
Showing with 72 additions and 64 deletions.
  1. +55 −45 plugins/bounce.js
  2. +17 −19 tests/plugins/bounce.js
View
100 plugins/bounce.js
@@ -4,68 +4,78 @@ var net_utils = require('./net_utils');
exports.register = function () {
var plugin = this;
- plugin.register_hook('mail', 'refresh_config');
+ plugin.load_configs();
+
plugin.register_hook('mail', 'reject_all');
plugin.register_hook('data', 'single_recipient');
plugin.register_hook('data', 'bad_rcpt');
plugin.register_hook('data_post', 'empty_return_path');
plugin.register_hook('data_post', 'non_local_msgid');
};
-exports.refresh_config = function (next, connection) {
+exports.load_configs = function () {
var plugin = this;
- plugin.cfg = plugin.config.get('bounce.ini', {
- booleans: [
- '-check.reject_all',
- '+check.single_recipient',
- '-check.empty_return_path',
- '+check.bad_rcpt',
- '-non_local_msgid',
-
- '+reject.single_recipient',
- '-reject.empty_return_path',
- ],
- });
-
- // Legacy config handling
- if (plugin.cfg.main.reject_invalid) {
- connection.logerror(plugin, "bounce.ini is out of date, please update!");
- plugin.cfg.check.single_recipient=true;
- plugin.cfg.reject.single_recipient=true;
- }
-
- if (plugin.cfg.main.reject_all) {
- connection.logerror(plugin, "bounce.ini is out of date, please update!");
- plugin.cfg.check.reject_all=true;
- }
-
- plugin.cfg.invalid_addrs = plugin.config.get('bounce_bad_rcpt', 'list');
- return next();
+ var load_bounce_ini = function () {
+ plugin.cfg = plugin.config.get('bounce.ini', {
+ booleans: [
+ '-check.reject_all',
+ '+check.single_recipient',
+ '-check.empty_return_path',
+ '+check.bad_rcpt',
+ '-non_local_msgid',
+
+ '+reject.single_recipient',
+ '-reject.empty_return_path',
+ ],
+ }, load_bounce_ini);
+
+ // Legacy config handling
+ if (plugin.cfg.main.reject_invalid) {
+ plugin.logerror("bounce.ini is out of date, please update!");
+ plugin.cfg.check.single_recipient=true;
+ plugin.cfg.reject.single_recipient=true;
+ }
+
+ if (plugin.cfg.main.reject_all) {
+ plugin.logerror("bounce.ini is out of date, please update!");
+ plugin.cfg.check.reject_all=true;
+ }
+ };
+ load_bounce_ini();
+
+ var load_bounce_bad_rcpt = function () {
+ var invalids = {};
+ var new_list = plugin.config.get('bounce_bad_rcpt', 'list', load_bounce_bad_rcpt);
+ for (var i=0; i < new_list.length; i++) {
+ invalids[new_list[i]] = true;
+ }
+ plugin.cfg.invalid_addrs = invalids;
+ };
+ load_bounce_bad_rcpt();
};
exports.reject_all = function (next, connection, params) {
var plugin = this;
- var transaction = connection.transaction;
+ if (!plugin.cfg.check.reject_all) { return next(); }
- if (!plugin.cfg.check.reject_all) return next();
var mail_from = params[0];
if (!plugin.has_null_sender(connection, mail_from)) {
return next(); // bounce messages are from null senders
}
- transaction.results.add(plugin, {fail: 'bounces_accepted', emit: true });
+ connection.transaction.results.add(plugin, {fail: 'bounces_accepted', emit: true });
return next(DENY, "No bounces accepted here");
};
exports.single_recipient = function(next, connection) {
var plugin = this;
- var transaction = connection.transaction;
-
if (!plugin.cfg.check.single_recipient) return next();
if (!plugin.has_null_sender(connection)) return next();
+ var transaction = connection.transaction;
+
// Valid bounces have a single recipient
if (connection.transaction.rcpt_to.length === 1) {
transaction.results.add(plugin, {pass: 'single_recipient', emit: true });
@@ -84,11 +94,11 @@ exports.single_recipient = function(next, connection) {
exports.empty_return_path = function(next, connection) {
var plugin = this;
- var transaction = connection.transaction;
-
if (!plugin.cfg.check.empty_return_path) return next();
if (!plugin.has_null_sender(connection)) return next();
+ var transaction = connection.transaction;
+
// Bounce messages generally do not have a Return-Path set. This checks
// for that. But whether it should is worth questioning...
@@ -128,7 +138,7 @@ exports.bad_rcpt = function (next, connection) {
for (var i=0; i < connection.transaction.rcpt_to.length; i++) {
var rcpt = connection.transaction.rcpt_to[i].address();
- if (plugin.cfg.invalid_addrs.indexOf(rcpt) === -1) continue;
+ if (!plugin.cfg.invalid_addrs[rcpt]) continue;
transaction.results.add(plugin, {fail: 'bad_rcpt', emit: true });
return next(DENY, "That recipient does not accept bounces");
}
@@ -158,16 +168,19 @@ exports.has_null_sender = function (connection, mail_from) {
exports.non_local_msgid = function (next, connection) {
var plugin = this;
- var transaction = connection.transaction;
-
if (!plugin.cfg.check.non_local_msgid) return next();
if (!plugin.has_null_sender(connection)) return next();
+ var transaction = connection.transaction;
+
// Bounce messages usually contain the headers of the original message
// in the body. This parses the body, searching for the Message-ID header.
// It then inspects the contents of that header, extracting the domain part,
// and then checking to see if that domain is local to this server.
+ // NOTE: this only works reliably if *every* message sent has a local
+ // domain in the Message-ID. In practice, that means outbound MXes MUST
+ // check Message-ID on outbound and rewrite non-conforming Message-IDs.
var matches = transaction.body.bodytext.match(/[\r\n]Message-ID: .*?[\r\n]/gi);
if (!matches) {
connection.loginfo(plugin, "no Message-ID matches");
@@ -203,10 +216,7 @@ exports.non_local_msgid = function (next, connection) {
return next(DENY, "bounce Message-ID without valid domain, I didn't send it.");
}
- connection.loginfo(plugin, valid_domains);
- connection.logerror(plugin, "TODO: verify valid_domains are local");
- return next();
-
- // transaction.results.add(plugin, {fail: 'empty_return_path', emit: true });
- // return next(DENY, "bounce with non-local Message-ID (RFC 3834)");
+ // we wouldn't have accepted the bounce if the recipient wasn't local
+ transaction.results.add(plugin, {fail: 'Message-ID not local', emit: true });
+ return next(DENY, "bounce with non-local Message-ID (RFC 3834)");
};
View
36 tests/plugins/bounce.js
@@ -28,7 +28,7 @@ function _set_up(callback) {
single_recipient:true,
empty_return_path:true,
},
- invalid_addrs: [ 'test@bad1.com', 'test@bad2.com', ]
+ invalid_addrs: { 'test@bad1.com': true, 'test@bad2.com': true },
};
// stub out functions
@@ -49,21 +49,17 @@ function _tear_down(callback) {
callback();
}
-exports.refresh_config = {
+exports.load_configs = {
setUp : _set_up,
tearDown : _tear_down,
'yes': function (test) {
test.expect(3);
- var plugin = this.plugin;
- var cb = function () {
- test.ok(plugin.cfg.main);
- test.ok(plugin.cfg.check);
- test.ok(plugin.cfg.reject);
- };
- this.plugin.refresh_config(cb);
+ this.plugin.load_configs();
+ test.ok(this.plugin.cfg.main);
+ test.ok(this.plugin.cfg.check);
+ test.ok(this.plugin.cfg.reject);
test.done();
},
- // TODO: write a bounce.ini, w/o settings, make sure defaults apply
};
exports.reject_all = {
@@ -188,8 +184,8 @@ exports.bad_rcpt = {
test.expect(1);
this.connection.transaction.mail_from= new Address.Address('<>');
this.connection.transaction.rcpt_to= [ new Address.Address('test@good.com') ];
- var cb = function () {
- test.equal(undefined, arguments[0]);
+ var cb = function (rc) {
+ test.equal(undefined, rc);
};
this.plugin.bad_rcpt(cb, this.connection);
test.done();
@@ -198,9 +194,10 @@ exports.bad_rcpt = {
test.expect(1);
this.connection.transaction.mail_from= new Address.Address('<>');
this.connection.transaction.rcpt_to= [ new Address.Address('test@bad1.com') ];
- var cb = function () {
- test.equal(DENY, arguments[0]);
+ var cb = function (rc) {
+ test.equal(DENY, rc);
};
+ this.plugin.cfg.invalid_addrs = {'test@bad1.com': true, 'test@bad2.com': true };
this.plugin.bad_rcpt(cb, this.connection);
test.done();
},
@@ -211,9 +208,10 @@ exports.bad_rcpt = {
new Address.Address('test@bad1.com'),
new Address.Address('test@bad2.com')
];
- var cb = function () {
- test.equal(DENY, arguments[0]);
+ var cb = function (rc) {
+ test.equal(DENY, rc);
};
+ this.plugin.cfg.invalid_addrs = {'test@bad1.com': true, 'test@bad2.com': true };
this.plugin.bad_rcpt(cb, this.connection);
test.done();
},
@@ -224,10 +222,10 @@ exports.bad_rcpt = {
new Address.Address('test@good.com'),
new Address.Address('test@bad2.com')
];
- var outer = this;
- var cb = function () {
- test.equal(DENY, arguments[0]);
+ var cb = function (rc) {
+ test.equal(DENY, rc);
};
+ this.plugin.cfg.invalid_addrs = {'test@bad1.com': true, 'test@bad2.com': true };
this.plugin.bad_rcpt(cb, this.connection);
test.done();
},

0 comments on commit 499bfb7

Please sign in to comment.
Something went wrong with that request. Please try again.