Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling .join() without parameter causes bad RegExp replacement on bounce #3236

Closed
stanek0j opened this issue Oct 19, 2023 · 2 comments · Fixed by #3237
Closed

Calling .join() without parameter causes bad RegExp replacement on bounce #3236

stanek0j opened this issue Oct 19, 2023 · 2 comments · Fixed by #3237
Labels

Comments

@stanek0j
Copy link
Contributor

Describe the bug

I was testing bouncing and found that when multiple recipients were defined from the same domain, mysterious &undefined; appeared in bounce message.

Expected behavior

Email addresses joined with , like:
Error: Some recipients failed: <[email protected]>, <[email protected]>

Observed behavior

Comma replaced with "undefined":
Error: Some recipients failed: <[email protected]>&undefined; <[email protected]>

Steps To Reproduce

Configure server, enable bouncing, use default templates and send email from your email address (needed to receive the bounce) to multiple non-existing email addresses from the same existing domain.

System Info:

Haraka Haraka.js — Version: 3.0.2
Node v20.5.1
OS Linux test-mailer 6.1.0-10-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.38-1 (2023-07-14) x86_64 GNU/Linux
openssl OpenSSL 3.0.9 30 May 2023 (Library: OpenSSL 3.0.9 30 May 2023)

Additional context

I traced this behavior down to outbound/hmail.js where .join() on line 1065 is used incorrectly:
https://github.com/haraka/Haraka/blob/eb5466f216bcc74bea297ac21e11863c69452ea7/outbound/hmail.js#L1065C1-L1065C1

When .join() is called without a parameter, default of ',' is used, thus creating the escape pattern of /[&,<,>,",',\r,\n]/g.
Correct way should be to supply .join() with empty string like .join('') and therefore the desired pattern /[&<>"'\r\n]/g.

@msimerson
Copy link
Member

Indeed. Is a PR forthcoming?

@msimerson msimerson added the Bug label Oct 19, 2023
@msimerson
Copy link
Member

That code was added in #2446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants