[Bug 28729] New: Return-path Header not correctly set in emails
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Bug ID: 28729 Summary: Return-path Header not correctly set in emails Change sponsored?: --- Product: Koha Version: 20.05 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: System Administration Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com We have been testing the email setup in Koha comparing our 18.11 installations with a 20.11 one. In those tests we see an issue with the Return-Path header. The Return-Path header should be using (if set) the * ReturnpathDefault system preference * Return-Path from library configuration But in 20.11 we see that it's set using * KohaAdminEmailAddress * branch email This is quite an issue. Wer are using specific and different email addresses for each of those settings: * KohaAdminEmailAddress / branch email * ReturnPathDefault / return-path for library * ReplytoDefault / reply to for library Something between versions has changed to cause the change in the Return-Path header. Maybe another side effect of the SMTP library change? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Tests were done using the cart email (directly sent) and the checkout notice (goes through message_queue). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|20.05 |20.11 CC| |jonathan.druart+koha@gmail. | |com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Return-path Header not |Return-path header not |correctly set in emails |correctly set in emails -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What's the expected behaviour? For each of the headers! :) Because it's confusing. I can confirm the bug in master: KohaAdminEmailAddress => root@example.org ReturnPathDefault => ReturnPathDefault@default.org No emails defined at library level """ From: root@example.org Subject: Checkouts To: edna@example.org Sender: ReturnPathDefault@default.org """ My proposed fix would be: diff --git a/C4/Letters.pm b/C4/Letters.pm index 177a4f51f67..ca02293c492 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1372,6 +1372,7 @@ sub _send_message_by_email { my $from_address = $message->{'from_address'} || $branch_email + || $branch_returnpath || C4::Context->preference('KohaAdminEmailAddress'); if( !$from_address ) { _set_message_status({ But I am not sure what should be used for "From" and "Sender". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|20.11 |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- However I don't find which commit could have caused a change on the "From" (is that what we are talking about)? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|master |20.11 CC| |christian.stelzenmueller@bs | |z-bw.de, | |ulrich.kleiber@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jonathan, thx for checking. The change is not in the From header, the change is the Return-Path header. As far as I understand the issue: We have never set the Return-Path as such, but we have used "Sender". See: https://stackoverflow.com/questions/19334223/how-can-we-set-return-path-in-a...) In the emails sent, this resulted in Sender and Return-Path headers being set to the same email address - the one configured in our Return-Path configuration settings. As we are on 18.11 at the moment, there is a bit of a version gap in my testing. With the changes to STMP configuration in mind, we have tested 20.11 more closely. There the change appears: Now Sender is still correct, but Return-Path shows the information from From (branchemail, KohaAdminEmailAddress). The Return-Path header appears on the first line from the other headers in my testing, so can be missed easily when looking at the other headers that appear further below. Sender and Return-Path should always match. They did, but not anymore. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
My proposed fix would be:
diff --git a/C4/Letters.pm b/C4/Letters.pm index 177a4f51f67..ca02293c492 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1372,6 +1372,7 @@ sub _send_message_by_email { my $from_address = $message->{'from_address'} || $branch_email + || $branch_returnpath || C4::Context->preference('KohaAdminEmailAddress'); if( !$from_address ) { _set_message_status({
But I am not sure what should be used for "From" and "Sender".
This change would be wrong - the From address correct with branchemail or KohaAdminEmailAddress. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I was going to add: I suspect the change is more likely to be caused by the change of Perl module than a Koha code change, but I don't have prove. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #6)
I was going to add: I suspect the change is more likely to be caused by the change of Perl module than a Koha code change, but I don't have prove.
I didn't find traces of Return-path being set in Mail::Sendmail. That's why I'm a bit confused about what changed. On the other hand, it is correct to set the return path correctly :-). If you want me to do that, I can just do it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Could it be that it was not set, so the MTA did it - and it is set with the new library now, but with From if not set explicitly? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- In some documentation I read that the MTA copy Sender if no Return-Path is set, which. That could be a possible explanation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
On the other hand, it is correct to set the return path correctly :-). If you want me to do that, I can just do it.
Please do! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Sender and Return-path are envelope headers. So the email has a From, but the envelope also says 'This is sent by Sender on behalf of From, and in the event of a delivery failure, please send the failure to Return-path'. My perception is that ReturnpathDefault/$library->returnpath should be used to set the Return-path, and the Sender. Unless we want to be incredibly flexible and split them. But that'd be out of the scope of the bug, and probably that should be solved by sysadmins with some rules on the MTA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Tomás Cohen Arazi from comment #11)
Sender and Return-path are envelope headers. So the email has a From, but the envelope also says 'This is sent by Sender on behalf of From, and in the event of a delivery failure, please send the failure to Return-path'.
My perception is that ReturnpathDefault/$library->returnpath should be used to set the Return-path, and the Sender.
I agree - Sender and Return-path should be set to the same address.
Unless we want to be incredibly flexible and split them. But that'd be out of the scope of the bug, and probably that should be solved by sysadmins with some rules on the MTA.
This would be a change of behaviour and I see no use case where this would be necessary. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |normal Summary|Return-path header not |Return-path header not set |correctly set in emails |in emails Version|20.11 |unspecified --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- So if I understand correctly this is a new enhancement request, not a regression? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #13)
So if I understand correctly this is a new enhancement request, not a regression?
No, it's a regression and a bug. We have configuration options for the Return-Path in Koha that now are no longer used/functional. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Any library relying on the return-path paramters might be likely to no longer receive emails bouncing back. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Looks like we never set the "Return-Path" from Koha and haven't found anything related in the Mail::Sendmail library. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #16)
Looks like we never set the "Return-Path" from Koha and haven't found anything related in the Mail::Sendmail library.
But it worked. Just because we haven't found out why it changed yet, doesn't mean this isn't a regression. Because the behaviour change can be proved. The Return-path functionality was developed, tested and agreed on in bug 9350 in 2014. I can promise it worked when added to Koha and has worked since then at least up to 18.11 but probably in later versions as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Tried on 18.11.x ---------- MESSAGE FOLLOWS ---------- Sender: ReturnPathDefault@default.org Mime-Version: 1.0 Charset: utf8 Content-Transfer-Encoding: quoted-printable Subject: =?UTF-8?B?Q2hlY2tvdXRz?= Content-Type: text/plain; charset="UTF-8" Date: Wed, 28 Jul 2021 12:59:39 +0000 To: edna@example.org From: root@example.org X-Peer: 127.0.0.1 The following items have been checked out: E Street shuffle=20 Thank you for visiting Centerville.= ------------ END MESSAGE ------------ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Did you check the MTA config? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #20 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- Hi Jonathan, the configuration of the MTA was not changed when upgrading to 20.11. It is the same on all our Koha Servers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #21 from David Cook <dcook@prosentient.com.au> --- I wouldn't be surprised if this were related to Bug 26705. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #21)
I wouldn't be surprised if this were related to Bug 26705.
I mean... not the same problem/fix, but that the new library might be messing with things. I could be totally wrong though. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Koha::Email (19.11) if ( C4::Context->preference('ReturnpathDefault') ) { $params->{sender} ||= C4::Context->preference('ReturnpathDefault'); } Mail::Sendmail If you wish to use an envelope sender address different than the From: address, set $mail{Sender} in your %mail hash MTAs will copy the Sender to Return-path But in the meantime master has been changed quite a bit. So the solution of this problem must be found in these changes? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Bug 22343 ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #24)
Bug 22343 ?
This one was pushed to 20.11 not backported. I think that the problem is there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
Something between versions has changed to cause the change in the Return-Path header. Maybe another side effect of the SMTP library change?
That's what I was suspecting all along :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #27 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Comment 18 shows that Koha didn't set Return-path in previous versions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #28 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- Maybe in Email.pm the envelope-from should be set to the return-path. Koha 20.11 --- Email.pm 2021-07-29 11:07:24.560924469 +0200 +++ Email.pm.patched 2021-07-29 11:49:01.108795689 +0200 @@ -76,9 +76,6 @@ my ( $self, $params ) = @_; my $args = {}; - $args->{from} = $params->{from} || C4::Context->preference('KohaAdminEmailAddress'); - Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) - unless Email::Valid->address( -address => $args->{from}, -fqdn => 0 ); # from is mandatory $args->{subject} = $params->{subject} // ''; @@ -117,6 +114,9 @@ ); } + $args->{from} = $addresses->{sender} || C4::Context->preference('ReturnpathDefault') || $params->{from} || C4::Context->preference('KohaAdminEmailAddress'); + Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) + unless Email::Valid->address( -address => $args->{from}, -fqdn => 0 ); # from is mandatory $args->{cc} = $addresses->{cc} if $addresses->{cc}; $args->{bcc} = $addresses->{bcc} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #29 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- The previous patch does not work, because it unfortunately also changes the header-from. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #27)
Comment 18 shows that Koha didn't set Return-path in previous versions.
Sorry, but I have to disagree. We tested the initial development in 2014 and we have just tested with 18.11 and the Return-Path was definitely set correctly. I think it might depend on MTA and how you test the email. Local testing might be limited here. How did you test? The issue we see now is also not that it's not set, but it's set to the wrong address, which is even worse. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Marcel de Rooy from comment #23)
Koha::Email (19.11)
if ( C4::Context->preference('ReturnpathDefault') ) { $params->{sender} ||= C4::Context->preference('ReturnpathDefault'); }
Mail::Sendmail If you wish to use an envelope sender address different than the From: address, set $mail{Sender} in your %mail hash
MTAs will copy the Sender to Return-path
But in the meantime master has been changed quite a bit. So the solution of this problem must be found in these changes?
Thx for digging this up. It's also the explanation I found so far. The strange thing now is that we see the wrong setting in Return-Path. Instead of using the information from Sender it appears that From is used for setting the Return-Path. I would suspect that this is not done by the MTA that hasn't changed then, but there shoudl be a visible change in the Koha email format too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #18)
Tried on 18.11.x
---------- MESSAGE FOLLOWS ---------- Sender: ReturnPathDefault@default.org Mime-Version: 1.0 Charset: utf8 Content-Transfer-Encoding: quoted-printable Subject: =?UTF-8?B?Q2hlY2tvdXRz?= Content-Type: text/plain; charset="UTF-8" Date: Wed, 28 Jul 2021 12:59:39 +0000 To: edna@example.org From: root@example.org X-Peer: 127.0.0.1
The following items have been checked out: E Street shuffle=20 Thank you for visiting Centerville.=
------------ END MESSAGE ------------
So this proves that if the MTA copies Sender to Return-path, Koha indirectly sets Return-path :) It is not enough to just look at headers leaving Koha; also look at what happens between sender and receiving side. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
So this proves that if the MTA copies Sender to Return-path, Koha indirectly sets Return-path :) It is not enough to just look at headers leaving Koha; also look at what happens between sender and receiving side.
Thx for the test - could you maybe repeat on master/20.11 for checking on differences? If the MTA hasn't changed like in our case... Koha might send something slightly different? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #34 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- With this patch only the envelope-from is changed. After that, emailing under 20.11 behaves much like emailing under 18.11. At least that's true for our installations. It's not pretty and I don't know if it covers everything or has any negative side effects. I hope this helps with further troubleshooting. --- Email.pm 2021-07-29 11:07:24.560924469 +0200 +++ Email.pm.patched 2021-07-29 16:33:26.636468041 +0200 @@ -173,6 +173,9 @@ $args->{to} = \@recipients; } + $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress'); + Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) + unless Email::Valid->address( -address => $args->{from}, -fqdn => 0 ); # from is mandatory $self->SUPER::send_or_die($args); } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Ulrich Kleiber from comment #34)
With this patch only the envelope-from is changed. After that, emailing under 20.11 behaves much like emailing under 18.11. At least that's true for our installations.
It's not pretty and I don't know if it covers everything or has any negative side effects. I hope this helps with further troubleshooting.
--- Email.pm 2021-07-29 11:07:24.560924469 +0200 +++ Email.pm.patched 2021-07-29 16:33:26.636468041 +0200 @@ -173,6 +173,9 @@ $args->{to} = \@recipients; }
+ $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress'); + Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) + unless Email::Valid->address( -address => $args->{from}, -fqdn => 0 ); # from is mandatory $self->SUPER::send_or_die($args); }
The default for Return-Path should be preference ReturnpathDefault. Is that still used then? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #36 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- If no branchreturnpath exists, sender and thus envelope-from will be set to ReturnpathDefault. Maybe we should use $args->{from} ||= $self->email->header("Sender"); instead of $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress'); in the upper patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #37 from Ulrich Kleiber <ulrich.kleiber@bsz-bw.de> --- The described change of the patch in my last comment is incorrect. Please ignore. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
The default for Return-Path should be preference ReturnpathDefault. Is that still used then?
As we copy the Sender information that behaves correctly in my testing, I think that is the case. You can find this line further above in Email.pm create() $addresses->{sender} ||= C4::Context->preference('ReturnpathDefault') if C4::Context->preference('ReturnpathDefault'); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK.. I've dug through this a bit.. man the Email::Stuffer and friends and Mail::Sendmail docs are hard to understand. In short, I think 'Sender' and 'From' are basically reversed between the two sets of modules.. I would say we need to set 'From' to 'returnpath' info (Thus setting the envelope MAIL FROM' used in the mail server connection and then if we have a more specific 'From' we want in the actual mail we set that in the 'Sender' slot. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126436 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126436&action=edit Bug 28729: Reverse 'sender' and 'from' Having read through the docs for both Email::Stuffer (Email::Sender) and Mail::Sendmail, I believe the meaning of 'from' and 'Sender' has reversed between the two modules. Email::Sendmail interpreted 'from' as the from address of the mail and used Sender as an override for the envelope when it was passed. However, in Email::Stuffer the 'from' is set at the envelope level and thus sets the connection 'MAIL FROM' which is subsequently used as the return-path by the mail server. It then uses 'Sender' as the message from address if one is passed in as a header. This patch reverses our handling of those attribute in Koha::Email. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|tomascohen@gmail.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Martin, thanks a lot for looking into this! We'll try to test your fix, this is the patch we have been running with meanwhile: Set the envelope-from and therefore the return-path to sender. --- Koha/Email.pm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Koha/Email.pm b/Koha/Email.pm index 9c077a6b84..068f8d9004 100644 --- a/Koha/Email.pm +++ b/Koha/Email.pm @@ -173,6 +173,9 @@ sub send_or_die { $args->{to} = \@recipients; } + $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress'); + Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from}) + unless Email::Valid->address( -address => $args->{from}, -fqdn => 0 ); # from is mandatory $self->SUPER::send_or_die($args); } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126436|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #42 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I changed my mind.. patches inbound -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126526&action=edit Bug 28729: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126527&action=edit Bug 28729: Make Koha::Email->send_or_die handle returnpath Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library would extract the from address from the Sender header to pass along in the MAIL FROM: field of the SMTP protocol [1]. This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior as it expects the envelope to be passed and falls back to extracting the 'From' header when said envelope is not found [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 {from} parameter is passed, it extracts the MAIL FROM envelope from the Sender header, as Mail::Sendmail did, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the unit tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Sender is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The from parameter is correct! No Sender header sent! 5. Sign off :-D [1] https://metacpan.org/dist/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L284 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#envelope-informat... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Wish you'd submitted that before I dug through it and came up with basically the same. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #46 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Actually, we did - comment#34. I missed it earlier too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|tomascohen@gmail.com |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126526|0 |1 is obsolete| | --- Comment #47 from David Nind <david@davidnind.com> --- Created attachment 127016 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127016&action=edit Bug 28729: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126527|0 |1 is obsolete| | --- Comment #48 from David Nind <david@davidnind.com> --- Created attachment 127017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127017&action=edit Bug 28729: Make Koha::Email->send_or_die handle returnpath Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library would extract the from address from the Sender header to pass along in the MAIL FROM: field of the SMTP protocol [1]. This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior as it expects the envelope to be passed and falls back to extracting the 'From' header when said envelope is not found [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 {from} parameter is passed, it extracts the MAIL FROM envelope from the Sender header, as Mail::Sendmail did, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the unit tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Sender is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The from parameter is correct! No Sender header sent! 5. Sign off :-D [1] https://metacpan.org/dist/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L284 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#envelope-informat... Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Resuming tomorrow, dont worry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29365 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29365 [Bug 29365] Koha::Email issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127017|0 |1 is obsolete| | --- Comment #51 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127085&action=edit Bug 28729: Make Koha::Email->send_or_die handle returnpath Koha used to rely on Mail::Sendmail for sending emails. As an SMTP client, the library would extract the from address from the Sender header to pass along in the MAIL FROM: field of the SMTP protocol [1]. This was overlooked when we moved to Email::Stuffer/Email::Simple and there's a different behavior as it expects the envelope to be passed and falls back to extracting the 'From' header when said envelope is not found [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 {from} parameter is passed, it extracts the MAIL FROM envelope from the Sender header, as Mail::Sendmail did, and calls $self->SUPER::send_or_die with the right parameters. To test: 1. Apply the unit tests 2. Run: $ kshell k$ prove t/Koha/Email.t => FAIL: Sender is not handled correctly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The from parameter is correct! No Sender header sent! 5. Sign off :-D [1] https://metacpan.org/dist/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L284 [2] https://metacpan.org/pod/Email::Sender::Manual::QuickStart#envelope-informat... Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> AMENDED (SHORTENED) - my @headers = $self->email->header_str_pairs; - foreach my $pair ( pairs @headers ) { - my ( $header, $value ) = @$pair; - $args->{from} = $value if $header eq 'Sender'; - } - - # Remove the Sender header - $self->email->header_str_set('Sender'); + $args->{from} = $self->email->header_str('Sender'); + $self->email->header_str_set('Sender'); # remove Sender header Tested with same results (scrambled domains): From: noreply@mydevserver.com Cc: marcel@email.nl To: test@somewhere.nl Reply-To: bieb@mydevserver.com Return-Path: postmaster@mydevserver.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127016|0 |1 is obsolete| | --- Comment #52 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127086&action=edit Bug 28729: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #53 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- The fix works fine on itself, but it feels like a fix on the wrong place. And a fix that more or less assumes that we are not calling send_or_die with the from parameter; this seems to the case in the current codebase where we only pass transport. We should be fixing create too (origin of this problem). I will postpone these things to 29365. The test passes, could be improved too. This serves as a minimal fix, allowing backports. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #54 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We cover ReturnPathDefault but do not fallback to Admin like here: + $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress'); I am keeping it in blocked over the weekend.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #55 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Marcel de Rooy from comment #53)
The fix works fine on itself, but it feels like a fix on the wrong place. And a fix that more or less assumes that we are not calling send_or_die with the from parameter; this seems to the case in the current codebase where we only pass transport.
Not sure what you mean here.. we account for 'from' being passed... the key challenge is understanding that the 'from' you might pass send_or_die is the envelope address and not the email from.
We should be fixing create too (origin of this problem). I will postpone these things to 29365. The test passes, could be improved too.
This serves as a minimal fix, allowing backports.
I'm not sure what you mean about fixing create? Sender is a bad name for this parameter and it was used due to a weird internal workaround provided by Mail::Sendmail.. I don't know that you could easily move the fix into create.. short of create producing the wrapper and returning a structure that's all nested one level deeper. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #56 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 127094 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127094&action=edit Bug 28729: Additions to POD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #57 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize from comment #55)
(In reply to Marcel de Rooy from comment #53)
The fix works fine on itself, but it feels like a fix on the wrong place. And a fix that more or less assumes that we are not calling send_or_die with the from parameter; this seems to the case in the current codebase where we only pass transport.
Not sure what you mean here.. we account for 'from' being passed... the key challenge is understanding that the 'from' you might pass send_or_die is the envelope address and not the email from.
We should be fixing create too (origin of this problem). I will postpone these things to 29365. The test passes, could be improved too.
This serves as a minimal fix, allowing backports.
I'm not sure what you mean about fixing create? Sender is a bad name for this parameter and it was used due to a weird internal workaround provided by Mail::Sendmail.. I don't know that you could easily move the fix into create.. short of create producing the wrapper and returning a structure that's all nested one level deeper.
Lets see on the new report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA --- Comment #58 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #54)
We cover ReturnPathDefault but do not fallback to Admin like here:
+ $args->{from} = $self->email->header("Sender") || C4::Context->preference('KohaAdminEmailAddress');
I am keeping it in blocked over the weekend..
Dont think we should do that here. If you keep the pref ReturnpathDefault empty, return path falls back correctly to sender. And we already use AdminEmail as a fallback. The fix is fine as-is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22343 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22343 [Bug 22343] Add configuration options for SMTP servers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 --- Comment #59 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Version(s)|21.11.00 |21.11.00,21.05.05 released in| | Status|Pushed to master |Pushed to stable --- Comment #60 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|21.11.00,21.05.05 |21.11.00,21.05.05,20.11.12 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #61 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED CC| |victor@tuxayo.net Resolution|--- |FIXED --- Comment #62 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Missing dependencies for 20.05.x, it shouldn't be affected, no backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28729 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36115 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org