Fix default EHLO banner formatting

[louis-lau]

Tiny fix, but the default EHLO message seems to have gotten a bit mangled by #2498, this fixes it back to the original.

- 250-mx1.example.com Hello outbound.example.org [0.0.0.0]Haraka is at your service.
+ 250-mx1.example.com Hello outbound.example.org [0.0.0.0], Haraka is at your service.

Checklist:

• [ ] docs updated
• [ ] tests updated
• [ ] Changes updated

[msimerson]

This seems like the wrong way to fix it, as it will also require the user who creates a custom banner to prefix it with ,. Wouldn't this be a better fix?

diff --git a/connection.js b/connection.js
index ef70e0f2..c185bd74 100644
--- a/connection.js
+++ b/connection.js
@@ -850,7 +850,7 @@ class Connection {
             default:
                 // RFC5321 section 4.1.1.1
                 // Hostname/domain should appear after 250
-                this.respond(250, `${this.local.host} Hello ${this.get_remote('host')}${this.ehlo_hello_message}`);
+                this.respond(250, `${this.local.host} Hello ${this.get_remote('host')}, ${this.ehlo_hello_message}`);
         }
     }
     ehlo_respond (retval, msg) {
@@ -883,7 +883,7 @@ class Connection {
                 // Hostname/domain should appear after 250

                 const response = [
-                    `${this.local.host} Hello ${this.get_remote('host')}${this.ehlo_hello_message}`,
+                    `${this.local.host} Hello ${this.get_remote('host')}, ${this.ehlo_hello_message}`,
                     "PIPELINING",
                     "8BITMIME",
                 ];

[louis-lau]

Thought that as well, but the original PR was for an option to hide it entirely. The forced comma wouldn't do that. It's also nice to be able to choose a dash as a separator if you want, for example. But having to add the comma in the config does seem a little counterintuitive

[louis-lau]

But up to you of course :)

[msimerson]

This begs the question of how does one hide that message entirely? This didn't work: echo '' > config/ehlo_hello_message.

Changing this doesn't work either:

--- a/connection.js
+++ b/connection.js
@@ -97,7 +97,7 @@ class Connection {
         this.transaction = null;
         this.tran_count = 0;
         this.capabilities = null;
-        this.ehlo_hello_message = config.get('ehlo_hello_message') || 'Haraka is at your service.';
+        this.ehlo_hello_message = config.get('ehlo_hello_message') ?? 'Haraka is at your service.';
         this.connection_close_message = config.get('connection_close_message') || 'closing connection. Have a jolly good day.';
         this.banner_includes_uuid = !!config.get('banner_includes_uuid');
         this.deny_includes_uuid = config.get('deny_includes_uuid') || null;

because an empty config file returns null. Hmmm.

[louis-lau]

Ha, no idea in that case. Hadn't tested it myself. Guess it's probably fine to statically add the comma then.

[louis-lau]

Edited the commit