fix requests not being sent to the cc recipients

From a SMTP pov, the To and Cc header are not read. The real recipient is stated at the begining of a send mail transaction. It's up to the client to collect this recipient list. Sendmail does this by reading the To and Cc header. smtplib doesn't do this when being invoked with an explicit To array. This commit adds the Cc people to the recipient array. The issue has been introduced by the switch from sendmail to smtplib.

added 1 commit

• 052ed3ce - fix requests not being sent to the cc recipients

Compare with previous version

added 1 commit

• c99ec9ba - fix requests not being sent to the cc recipients

Compare with previous version

added 1 commit

• 8c41e1e6 - fix requests not being sent to the cc recipients

Compare with previous version

changed the description

added 1 commit

• 04eeaf57 - fix requests not being sent to the cc recipients

Compare with previous version

added 2 commits

• 56d6dbaf - a
• 744a4d9f - remove sendmail option

Compare with previous version

added 2 commits

• 3c96d85a - fix requests not being sent to the cc recipients
• 33b4fd92 - remove sendmail option

Compare with previous version

The more "obvious" solution would be to call smtplib with to_addrs=None, but then we'd implicitly send mails to Cc'ed people len(To:) times. For mails with Cc set, we currently have only one To recipient, but that might change in the future. I think that this more explicit approach is less error prone.

Thanks Frederik; please run make check with your patches. The sendmail option is used by the test suite and cannot be removed.

I also think this is exactly what I intended to do in c4f4ac51 (Deliver emails to Cc in smtplib code path, 2020-08-27); but now, looking at it again, it seems like I forgot to replace to by deliver_to in the server.sendmail() line. Or, in other words, is your patch equivalent to substituting to with deliver_to in the server.sendmail() line?

I thought about replacing to with deliver_to too, but we iterate over the recipient list, so we'd send the mail multiple times.

In all cases with Cc non-empty, there's only one recipient.

Long-term, I'm open to redesigning this; the problem is that multiple recipients are supported (for each of which we want to send separate emails) whereas there's only one shared Cc field.

added 1 commit

• ccf11e57 - fix requests not being sent to the cc recipients

Compare with previous version

This enables sendmail again and adds a small instruction to the sendmail portion to make it compatible for cases where there is only one rcpt if cc is set.

Doesn't this change the behavior to exposing all recipients in the To header if sendmail is not used? I'd much prefer fixing the problem in production by a straightforward low-risk (single line?) bug fix; we can have the conversation about redesigning the function in a followup patch.

If we don't want to expose the recipients, we shouldn't use To: or Cc: for those cases then. I'll open a separate MR for the single line fix.

mentioned in merge request !7 (closed)

We can discuss the advantages/disadvantages and other implications of using Bcc vs. individual emails for notifications -- but that's definitely way outside the scope of a bug fix for this particular issue.

Thanks for analyzing the issue, preparing another PR and sorry for the extra work!

So what shall we do with this MR?