helo.checks: only skip subsequent checks if success

haraka · May 8, 2024 · bf948fa · bf948fa
1 parent 2e6ff61
commit bf948fa
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 71 deletions.
diff --git a/config/attachment.ctype.regex b/config/attachment.ctype.regex
diff --git a/config/attachment.filename.regex b/config/attachment.filename.regex
diff --git a/config/data.headers.ini b/config/data.headers.ini
diff --git a/plugins/helo.checks.js b/plugins/helo.checks.js
@@ -36,17 +36,17 @@ exports.register = function () {
         this.register_hook('ehlo', c);
     }
 
-    // Always emit a log entry
-    this.register_hook('helo', 'emit_log');
-    this.register_hook('ehlo', 'emit_log');
-
     // IP literal that doesn't match remote IP
     this.register_hook('helo', 'literal_mismatch');
     this.register_hook('ehlo', 'literal_mismatch');
 
     this.cfg.check.literal_mismatch = this.cfg.check.literal_mismatch ?? 2;
     this.cfg.reject.literal_mismatch = this.cfg.reject.literal_mismatch ?? false;
 
+    // Always emit a log entry
+    this.register_hook('helo', 'emit_log');
+    this.register_hook('ehlo', 'emit_log');
+
     if (this.cfg.check.match_re) {
         const load_re_file = () => {
             const regex_list = utils.valid_regexes(this.config.get('helo.checks.regexps', 'list', load_re_file));
@@ -110,7 +110,7 @@ exports.init = function (next, connection, helo) {
 }
 
 exports.should_skip = function (connection, test_name) {
-    if (connection.results.has('helo.checks', '_skip_hooks', test_name)) {
+    if (connection.hello.host && connection.results.has('helo.checks', '_skip_hooks', test_name)) {
         this.logdebug(connection, `SKIPPING: ${test_name}`);
         return true;
     }

diff --git a/test/plugins/helo.checks.js b/test/plugins/helo.checks.js
@@ -301,8 +301,12 @@ describe('helo.checks', () => {
             this.plugin.cfg.reject.big_company=true;
             this.plugin.big_company((rc) => {
                 assert.equal(rc, DENY);
-                assert.ok(this.connection.results.get('helo.checks').fail.length);
-                done();
+                assert.equal(this.connection.results.get('helo.checks').fail.length, 1);
+                this.plugin.big_company((rc) => {
+                    assert.equal(rc, DENY);
+                    assert.ok(this.connection.results.get('helo.checks').fail.length, 2);
+                    done();
+                }, this.connection, test_helo);
             }, this.connection, test_helo);
         })
     })