Description
Kyle M Hall (khall)
2020-10-16 11:04:31 UTC
It seems to work for CHECKOUT on master for me. Which notices did you try? Any updates on this Kyle?... I'm going to downgrade it to 'normal' and neither Jonathan nor I can replicate it right now. According to the setting in Koha system preference "NoticeBcc" all notices should be sent to the given e-mail address. But after the update from Koha 19.11.05 to 20.11.03 this stopped working and no e-mails to the BCC-address were created or sent anymore. We now have a second report of this, I am setting a higher priority again. I can't test this as I don't have a system sending email and I believe the BCC is not visible otherwise - could someone try in 20.11 maybe? I'll provide a fix by Monday Are the reporters using multiple addresses on the syspref? In our case there is (and always was) only one address in system preference "NoticeBcc". ...but - not directly related to the "NoticeBcc" problem - there seems to be a problem with script "process_message_queue.pl" if it finds two e-mail addresses in field "message_queue.to_address" which leads to the following message: Invalid 'to' parameter: abc@def.com,xyz@def.comxxx: 11 status returned by "/usr/share/koha/bin/cronjobs/process_message_queue.pl" Ok, I need help here. Specially from email server admins that can see their logs or understand this behavior :-D What I did: # Install postfix (and mutt) in KTD: $ apt install postfix mutt # it removed exim4 # Add a line to /etc/postfix/main.cf $ echo "header_checks = regexp:/etc/postfix/header_checks" >> /etc/postfix/main.cf # Add a header_checks file: $ echo '/.*:/ WARN' > /etc/postfix/header_checks $ postmap /etc/postfix/header_checks # Restart postfix $ service postfix restart # Watch the logs on a terminal: $ tail -f /var/log/mail.* With that setup, I noticed we needed bug 28017 so we can check things without emails failing because of (say) Gmail policy validations. I generated a 'koha' user in KTD: $ adduser koha Then set things so my user has a check-in notice, and set its address to kohadev-koha@localhost Set KohaAdminEmailAddress to root@localhost Set NoticeBcc to koha@localhost Then run: $ kshell k$ misc/cronjobs/process_message_queue.pl -v => SUCCESS: The logs show: Mar 22 17:14:07 kohadevbox postfix/smtpd[63414]: connect from localhost[127.0.0.1] Mar 22 17:14:07 kohadevbox postfix/smtpd[63414]: 8D73D2604A6: client=localhost[127.0.0.1] Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Received: from localhost.localdomain (localhost [127.0.0.1])??by kohadevbox (Postfix) with ESMTP id 8D73D2604A6??for <kohadev-koha@localhost>; Mon, 22 Mar 2021 17:14:07 +0000 (UTC) from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Date: Mon, 22 Mar 2021 17:14:07 +0000 from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header MIME-Version: 1.0 from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Content-Type: text/plain; charset="utf-8" from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Content-Transfer-Encoding: quoted-printable from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header From: root@localhost from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Subject: Check-ins from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Bcc: koha@localhost from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header To: kohadev-koha@localhost from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header X-Mailer: Koha from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: warning: header Message-ID: <16164332470.BFB7954cB.63410@kohadevbox> from localhost[127.0.0.1]; from=<root@localhost> to=<kohadev-koha@localhost> proto=ESMTP helo=<localhost.localdomain> Mar 22 17:14:07 kohadevbox postfix/cleanup[63417]: 8D73D2604A6: message-id=<16164332470.BFB7954cB.63410@kohadevbox> Mar 22 17:14:07 kohadevbox postfix/qmgr[63226]: 8D73D2604A6: from=<root@localhost>, size=1071, nrcpt=1 (queue active) Mar 22 17:14:07 kohadevbox postfix/smtpd[63414]: disconnect from localhost[127.0.0.1] ehlo=1 mail=1 rcpt=1 data=1 quit=1 commands=5 Mar 22 17:14:07 kohadevbox postfix/local[63418]: 8D73D2604A6: to=<kohadev-koha@localhost>, relay=local, delay=0.06, delays=0.05/0.01/0/0, dsn=2.0.0, status=sent (delivered to mailbox) Mar 22 17:14:07 kohadevbox postfix/qmgr[63226]: 8D73D2604A6: removed So the Bcc header is there. I'm not sure why it isn't sending the email afterwards. But I'm sure the header is correctly added. Note: the non-advertised feature of accepting comma separated addresses is still not working. I can solve it on a separate bug report if someone files it. It needs to specify which fields could be multi-address.
> Note: the non-advertised feature of accepting comma separated addresses is
> still not working. I can solve it on a separate bug report if someone files
> it. It needs to specify which fields could be multi-address.
I think officially we don't support this, at least not from the GUI where often you can only get past validation entering one address. Maybe the system preferences would have allowed it in the past, but I am not aware of documentation. The report on the mailing list was a custom script that allows to send to multiple addresses, but if we can make that work again without too much trouble, I think it would be nice.
I've just had a report from a customer that this may also be affecting the reply-to addresses... I'm going to do some digging this morning. Just found out that the Mail::Sendmail lib we used before, does 'Example 2' from the link I pasted above: https://metacpan.org/release/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L331 Created attachment 118659 [details] [review] Bug 26705: Regression tests Created attachment 118660 [details] [review] Bug 26705: Make Koha::Email->send_or_die handle Bcc Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library took the job of extracting Bcc headers (and removing them) to pass them along with the recipients listed on To: and Cc: to the SMTP protocol in the form of RCPT TO: lines. [1] This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior, that is a design decision [2]. This patchset re-introduces the behavior from Mail::Sendmail by overriding the send_or_die method locally (in Koha::Email) and doing the right thing. Unless an explicit {to} parameter is passed, it extracts the recipients from the headers, as Mail::Sendmail does, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Bcc is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The recipients list is correct! No Bcc header sent! 5. Sign off :-D [1] https://metacpan.org/release/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L331 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#Hey,-where's-my-Bcc-support Created attachment 118663 [details] [review] Bug 26705: Regression tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 118664 [details] [review] Bug 26705: Make Koha::Email->send_or_die handle Bcc Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library took the job of extracting Bcc headers (and removing them) to pass them along with the recipients listed on To: and Cc: to the SMTP protocol in the form of RCPT TO: lines. [1] This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior, that is a design decision [2]. This patchset re-introduces the behavior from Mail::Sendmail by overriding the send_or_die method locally (in Koha::Email) and doing the right thing. Unless an explicit {to} parameter is passed, it extracts the recipients from the headers, as Mail::Sendmail does, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Bcc is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The recipients list is correct! No Bcc header sent! 5. Sign off :-D [1] https://metacpan.org/release/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L331 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#Hey,-where's-my-Bcc-support Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 118665 [details] [review] Bug 26705: (QA follow-up) Clarify test result Simple output string improvement to fix the copy/paste of the previous tests comment string. Nice one, well done tracking it down. Signing off Email::MIME::Header needs to be added to our dependencies. (In reply to Kyle M Hall from comment #18) > Email::MIME::Header needs to be added to our dependencies. The deps are Email::Stuffer <- Email::MIME > Email::MIME::Header we could explicitly require Email::MIME, but it is already a dependency for Email::Stuffer. It appears that the problem is that stretch has Email::Mime::Header 1.937 in packages, and buster has 1.949. Unit tests fail on 1.937 but pass on 1.949. (In reply to Kyle M Hall from comment #20) > It appears that the problem is that stretch has Email::Mime::Header 1.937 in > packages, and buster has 1.949. Unit tests fail on 1.937 but pass on 1.949. Gosh, I love having tests for everything new we add! Created attachment 118704 [details] [review] Bug 26705: (QA follow-up) Clarify test result Simple output string improvement to fix the copy/paste of the previous tests comment string. Created attachment 118705 [details] [review] Bug 26705: (QA follow-up) Do not rely on latest Email::MIME This patch makes the implementation use lower-level methods to deal with headers. So the Email::MIME library in Debian 9 works. Params are sorted for testing purposes. To test: 1. Apply this patch 2. Run the tests => SUCCESS: Things pass in D9+ 3. Sign off :-D Thanks Kyle! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Martin Renvoize from comment #16) > Created attachment 118665 [details] [review] [review] > Bug 26705: (QA follow-up) Clarify test result > > Simple output string improvement to fix the copy/paste of the previous > tests comment string. Thanks for that! Created attachment 118713 [details] [review] Bug 26705: Regression tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 118714 [details] [review] Bug 26705: Make Koha::Email->send_or_die handle Bcc Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library took the job of extracting Bcc headers (and removing them) to pass them along with the recipients listed on To: and Cc: to the SMTP protocol in the form of RCPT TO: lines. [1] This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior, that is a design decision [2]. This patchset re-introduces the behavior from Mail::Sendmail by overriding the send_or_die method locally (in Koha::Email) and doing the right thing. Unless an explicit {to} parameter is passed, it extracts the recipients from the headers, as Mail::Sendmail does, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Bcc is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The recipients list is correct! No Bcc header sent! 5. Sign off :-D [1] https://metacpan.org/release/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L331 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#Hey,-where's-my-Bcc-support Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 118715 [details] [review] Bug 26705: (QA follow-up) Clarify test result Simple output string improvement to fix the copy/paste of the previous tests comment string. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 118716 [details] [review] Bug 26705: (QA follow-up) Do not rely on latest Email::MIME This patch makes the implementation use lower-level methods to deal with headers. So the Email::MIME library in Debian 9 works. Params are sorted for testing purposes. To test: 1. Apply this patch 2. Run the tests => SUCCESS: Things pass in D9+ 3. Sign off :-D Thanks Kyle! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.05 Doesn't apply cleanly to 20.05, please rebase for backport. |