Skip to content

Commit

Permalink
remove logs and rollback to type checking
Browse files Browse the repository at this point in the history
connection type checkingw
  • Loading branch information
MuraraAllan committed Feb 2, 2022
1 parent d1782e4 commit 333ec4a
Show file tree
Hide file tree
Showing 34 changed files with 206 additions and 151 deletions.
5 changes: 3 additions & 2 deletions plugins/aliases.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,15 @@ exports.aliases = function (next, connection, params) {

function _drop (plugin, connection, rcpt) {
connection.logdebug(plugin, `marking ${rcpt} for drop`);

if (!connection?.transaction?.notes) return;
connection.transaction.notes.discard = true;
}

function _alias (plugin, connection, key, config, host) {
let to;
let toAddress;

if (config.to) {
if (connection?.transaction && config.to) {
if (Array.isArray(config.to)) {
connection.logdebug(plugin, `aliasing ${connection.transaction.rcpt_to} to ${config.to}`);
connection.transaction.rcpt_to.pop();
Expand Down
19 changes: 13 additions & 6 deletions plugins/attachment.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,11 @@ exports.unarchive_recursive = function (connection, f, archive_file_name, cb) {

exports.start_attachment = function (connection, ctype, filename, body, stream) {
const plugin = this;
const txn = connection.transaction;
const txn = connection?.transaction;

function next () {
if (txn.notes.attachment_count === 0 && txn.notes.attachment_next) {

if (txn?.notes?.attachment_next && txn.notes.attachment_count === 0) {
return txn.notes.attachment_next();
}
return;
Expand Down Expand Up @@ -386,7 +387,9 @@ exports.start_attachment = function (connection, ctype, filename, body, stream)

exports.hook_data = function (next, connection) {
const plugin = this;
const txn = connection.transaction;
const txn = connection?.transaction;
if (!txn) return next();

txn.parse_body = 1;
txn.notes.attachment_count = 0;
txn.notes.attachments = [];
Expand All @@ -400,7 +403,11 @@ exports.hook_data = function (next, connection) {
}

exports.check_attachments = function (next, connection) {
const txn = connection.transaction;
const txn = connection?.transaction;
if (!txn) {
return next();
}

const ctype_config = this.config.get('attachment.ctype.regex','list');
const file_config = this.config.get('attachment.filename.regex','list');
const archive_config = this.config.get('attachment.archive.filename.regex','list');
Expand Down Expand Up @@ -492,8 +499,8 @@ exports.check_items_against_regexps = function (items, regexps) {
}

exports.wait_for_attachment_hooks = (next, connection) => {
const txn = connection.transaction;
if (txn.notes.attachment_count > 0) {
const txn = connection?.transaction;
if (txn && txn.notes?.attachment_count > 0) {
// this.loginfo("We still have attachment hooks running");
txn.notes.attachment_next = next;
}
Expand Down
7 changes: 2 additions & 5 deletions plugins/avg.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,9 @@ exports.get_tmp_file = function (transaction) {
}

exports.hook_data_post = function (next, connection) {
const plugin = this;
if (connection?.transaction == null) {
connection.logwarn(plugin, "hook_data_post could not find transaction, skipping >>>");
return next();
}
if (!connection?.transaction) return next()

const plugin = this;
const tmpfile = plugin.get_tmp_file(connection.transaction);
const ws = fs.createWriteStream(tmpfile);

Expand Down
2 changes: 1 addition & 1 deletion plugins/block_me.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports.hook_data = (next, connection) => {
}

exports.hook_data_post = function (next, connection) {
if (!connection.relaying) {
if (!connection?.relaying || !connection?.transaction) {
return next();
}

Expand Down
49 changes: 24 additions & 25 deletions plugins/bounce.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ exports.load_bounce_ini = function () {
exports.reject_all = function (next, connection, params) {
const plugin = this;
if (!plugin.cfg.check.reject_all) { return next(); }

const mail_from = params[0];

if (!plugin.has_null_sender(connection, mail_from)) {
Expand All @@ -86,35 +85,34 @@ exports.reject_all = function (next, connection, params) {

exports.single_recipient = function (next, connection) {
const plugin = this;
if (!plugin.cfg.check.single_recipient) return next();
if (!plugin.has_null_sender(connection)) return next();

const transaction = connection.transaction;

if (!plugin?.cfg?.check?.single_recipient) return next();
if (!plugin?.has_null_sender(connection)) return next();
const { transaction, relaying, remote } = connection;

// Valid bounces have a single recipient
if (connection.transaction.rcpt_to.length === 1) {
if (transaction.rcpt_to.length === 1) {
transaction.results.add(plugin,
{pass: 'single_recipient', emit: true });
return next();
}

// Skip this check for relays or private_ips
// This is because Microsoft Exchange will send mail
// to distribution groups using the null-sender if
// the option 'Do not send delivery reports' is
// checked (not sure if this is default or not)
if (connection.relaying) {
if (relaying) {
transaction.results.add(plugin,
{skip: 'single_recipient(relay)', emit: true });
return next();
}
if (connection.remote.is_private) {
if (remote.is_private) {
transaction.results.add(plugin,
{skip: 'single_recipient(private_ip)', emit: true });
return next();
}

connection.loginfo(plugin, `bounce with too many recipients to: ${connection.transaction.rcpt_to.join(',')}`);
connection.loginfo(plugin, `bounce with too many recipients to: ${transaction.rcpt_to.join(',')}`);

transaction.results.add(plugin, {fail: 'single_recipient', emit: true });

Expand All @@ -129,7 +127,6 @@ exports.empty_return_path = function (next, connection) {
if (!plugin.has_null_sender(connection)) return next();

const transaction = connection.transaction;

// Bounce messages generally do not have a Return-Path set. This checks
// for that. But whether it should is worth questioning...

Expand All @@ -144,7 +141,7 @@ exports.empty_return_path = function (next, connection) {
// Return-Path, aka Reverse-PATH, Envelope FROM, RFC5321.MailFrom
// validate that the Return-Path header is empty, RFC 3834

const rp = connection.transaction.header.get('Return-Path');
const rp = transaction.header.get('Return-Path');
if (!rp) {
transaction.results.add(plugin, {pass: 'empty_return_path' });
return next();
Expand All @@ -161,14 +158,13 @@ exports.empty_return_path = function (next, connection) {

exports.bad_rcpt = function (next, connection) {
const plugin = this;
const transaction = connection.transaction;

if (!plugin.cfg.check.bad_rcpt) return next();
if (!plugin.has_null_sender(connection)) return next();
if (!plugin.cfg.invalid_addrs) return next();
const transaction = connection.transaction;

for (let i=0; i < connection.transaction.rcpt_to.length; i++) {
const rcpt = connection.transaction.rcpt_to[i].address();
for (let i=0; i < transaction.rcpt_to.length; i++) {
const rcpt = transaction.rcpt_to[i].address();
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');
Expand All @@ -179,15 +175,16 @@ exports.bad_rcpt = function (next, connection) {
}

exports.has_null_sender = function (connection, mail_from) {
const plugin = this;
const transaction = connection.transaction;
// ok ?
const transaction = connection?.transaction;
if (!transaction) return false;

if (!mail_from) mail_from = connection.transaction.mail_from;
const plugin = this;
if (!mail_from) mail_from = transaction.mail_from;

// bounces have a null sender.
// null sender could also be tested with mail_from.user
// Why would isNull() exist if it wasn't the right way to test this?

if (mail_from.isNull()) {
transaction.results.add(plugin, {isa: 'yes'});
return true;
Expand Down Expand Up @@ -217,8 +214,8 @@ exports.non_local_msgid = function (next, connection) {
if (!plugin.cfg.check.non_local_msgid) return next();
if (!plugin.has_null_sender(connection)) return next();

const transaction = connection.transaction;

const transaction = connection?.transaction;
if (!transaction) return next();
// 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,
Expand Down Expand Up @@ -309,7 +306,7 @@ function find_received_headers (ips, body, connection, self) {
exports.bounce_spf_enable = function (next, connection) {
const plugin = this;
if (plugin.cfg.check.bounce_spf) {
connection.transaction.parse_body = true;
transaction.parse_body = true;
}
return next();
}
Expand All @@ -318,7 +315,9 @@ exports.bounce_spf = function (next, connection) {
const plugin = this;
if (!plugin.cfg.check.bounce_spf) return next();
if (!plugin.has_null_sender(connection)) return next();
const txn = connection.transaction;

const txn = connection?.transaction;
if (!txn) return next();

// Recurse through all textual parts and store all parsed IPs
// in an object to remove any duplicates which might appear.
Expand Down
9 changes: 5 additions & 4 deletions plugins/clamd.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,10 @@ exports.hook_data = function (next, connection) {

exports.hook_data_post = function (next, connection) {
const plugin = this;
const txn = connection.transaction;
const cfg = plugin.cfg;

if (!plugin.should_check(connection)) return next();

const txn = connection.transaction;
const cfg = plugin.cfg;
// Do we need to run?
if (cfg.main.only_with_attachments && !txn.notes.clamd_found_attachment) {
connection.logdebug(plugin, 'skipping: no attachments found');
Expand Down Expand Up @@ -328,7 +327,9 @@ exports.should_check = function (connection) {
const plugin = this;

let result = true; // default

// ok ?
if (!connection?.transaction) return false

if (plugin.cfg.check.authenticated == false && connection.notes.auth_user) {
connection.transaction.results.add(plugin, { skip: 'authed'});
result = false;
Expand Down
4 changes: 4 additions & 0 deletions plugins/data.signatures.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@

exports.hook_data = (next, connection) => {
// enable mail body parsing
if (!connection?.transaction) return next();

connection.transaction.parse_body = true;
next();
}

exports.hook_data_post = function (next, connection) {
if (!connection?.transaction) return next();

const sigs = this.config.get('data.signatures', 'list');

if (check_sigs(sigs, connection.transaction.body)) {
Expand Down
7 changes: 3 additions & 4 deletions plugins/data.uribl.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ exports.do_lookups = function (connection, next, hosts, type) {
const plugin = this;

// Store the results in the correct place based on the lookup type
let results = connection.results;
if (connection.transaction != null) {
results = connection.transaction.results;
}
const results = connection?.transaction?.results || connection?.results;
if (!results) return next();

if (typeof hosts === 'string') {
hosts = [ hosts ];
Expand Down Expand Up @@ -307,6 +305,7 @@ exports.hook_mail = function (next, connection, params) {
}

exports.hook_data = (next, connection) => {
if (!connection?.transaction) return next();
// enable mail body parsing
connection.transaction.parse_body = true;
return next();
Expand Down
16 changes: 9 additions & 7 deletions plugins/delay_deny.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports.hook_deny = function (next, connection, params) {
const pi_hook = params[5];

const plugin = this;
const transaction = connection.transaction;
const transaction = connection;

// Don't delay ourselves...
if (pi_name == 'delay_deny') return next();
Expand Down Expand Up @@ -100,7 +100,8 @@ exports.hook_deny = function (next, connection, params) {

exports.hook_rcpt_ok = function (next, connection, rcpt) {
const plugin = this;
const transaction = connection.transaction;
const transaction = connection?.transaction;
if (!transaction) return next();

// Bypass all pre-DATA deny for AUTH/RELAY
if (connection.relaying) {
Expand All @@ -110,15 +111,15 @@ exports.hook_rcpt_ok = function (next, connection, rcpt) {

// Apply any delayed rejections
// Check connection level pre-DATA rejections first
if (connection.notes.delay_deny_pre) {
if (connection.notes?.delay_deny_pre) {
for (let i=0; i<connection.notes.delay_deny_pre.length; i++) {
const params = connection.notes.delay_deny_pre[i];
return next(params[0], params[1]);
}
}

// Then check transaction level pre-DATA
if (transaction.notes.delay_deny_pre) {
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];

Expand All @@ -134,14 +135,15 @@ exports.hook_rcpt_ok = function (next, connection, rcpt) {
}

exports.hook_data = (next, connection) => {
const transaction = connection.transaction;
const transaction = connection?.transaction;
if (!transaction) return next();

// Add a header showing all pre-DATA rejections
const fails = [];
if (connection.notes.delay_deny_pre_fail) {
if (connection.notes?.delay_deny_pre_fail) {
fails.push.apply(Object.keys(connection.notes.delay_deny_pre_fail));
}
if (transaction.notes.delay_deny_pre_fail) {
if (transaction.notes?.delay_deny_pre_fail) {
fails.push.apply(Object.keys(transaction.notes.delay_deny_pre_fail));
}
if (fails.length) transaction.add_header('X-Haraka-Fail-Pre', fails.join(' '));
Expand Down
Loading

0 comments on commit 333ec4a

Please sign in to comment.