Opened 7 years ago

Last modified 6 weeks ago

#28598 assigned Cleanup/optimization

BCC addresses are ignored in the console and file email backends

Reported by: zngr Owned by: Josh Schneier
Component: Core (Mail) Version: 1.11
Severity: Normal Keywords: mail, bcc
Cc: Ülgen Sarıkavak Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Hi there,

we noticed during development that the bcc header line is not printed in the console (and filebased since it inherits from console) EmailBackend (django.core.mail.backends.{console|filebased}.EmailBackend). It seems like this issue has been reported before, e.g. in #18582, however there was no specific solution. It looks like a design decision, however there is no documentation (that I found) about it.

In my opinion, it would be nice to have the BCC line printed in those backends. As the documentation says, those backends are not intended for use in production, which makes it an ideal tool for development and testing. However that requires that the backend behaves just as a regular (smtp) backend would and display the email message exactly as it would have been sent.

If you decide not to fix this, please add a note to the documentation to help developers avoid a sleepless night because they really can't get BCC to work in their mail function ;)

Best,
zngr

Change History (21)

comment:1 by Tim Graham, 7 years ago

Summary: Django ignores BCC in console and filebased EmailBackendBCC addresses are ignored in the console and file email backends
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

I'm not sure which solution is best, but I'll accept the ticket as an indication to do something (either a code change or document the limitation).

comment:2 by Stefan Schneider, 7 years ago

Owner: changed from nobody to Stefan Schneider
Status: newassigned

comment:3 by Josh Schneier, 6 years ago

Owner: changed from Stefan Schneider to Josh Schneier

What we display is the MIME message that the end-user receives, BCC is passed in the RCPT TO command to the SMTP server. I can't think of a decent way to include that as well so I'm going to just document the behavior.

PR

comment:4 by Carlton Gibson, 6 years ago

I think theres's an easy-enough™ fix available here, rather than just documenting that the BCC list won't be shown.

The current console backend does this:

for message in email_messages:
    self.write_message(message)
    ...

The simple addition is to also write message.recipients() at this point.

If we add and document a format_message() hook to console.EmailBackend, with a stub implementation...

def format_message(self, message):
    return message

# ... then in send_messages()...
for message in email_messages:
    self.write_message(self.format_message(message))
    ...

... then users would be free to subclass the either the console or file backends in order to display the BCC list.

comment:5 by Josh Schneier, 6 years ago

I thought about including message.recipients(), my hesitation is that it will double up the From and CC pieces since currently we show the MIME which already includes that component. I wonder if adding .format_message() isn't a case of YAGNI or over engineering.

I do agree that the proposed documentation patch is ugly, it makes specific call outs to technical details that don't feel necessary.

What do you think about also documenting the fact that filebased inherits from console? Having to double up docs is unfortunate.

Currently write_message also includes '*' * 79, is that part of the overridable interface as well in your solution?

comment:6 by Carlton Gibson, 6 years ago

I wonder if adding .format_message() isn't a case of YAGNI or over engineering.

Well, I guess most people won't need it, but that this ticket exists suggest some will. Adding a hook to allow subclassing at least allows those people to address their issue.
(Just saying in the docs that "BCCs won't be included" isn't great IMO: it offers me no way forward.)

... it will double up ...

I wouldn't worry about this. If people want to do some set operations in their subclass to get just the BCCs then they're welcome to.
All we're doing is providing the hook.

Currently write_message also includes '*' * 79...

I'd leave that where it is. If someone wants to override write_message() as well then they're welcome.

comment:7 by Josh Schneier, 6 years ago

To be clear write_message takes in an EmailMessage, not a string. Curious what you think about documenting write_message as the hook given that?

comment:8 by Carlton Gibson, 6 years ago

...write_message takes in an EmailMessage, not a string.

Yes, that's right. So they'd need to be some adjustment.

The trouble with write_message() as the hook is that you need to essentially reimplement it (or copy and paste the whole thing) in order to adjust the formatting.
As such it's more of a wontfix solution.

The idea I'm trying to communicate is that we add a format_message() hook, which does just that, separate from the existing write_message() method, which would just be responsible for the write() calls, message dividers (etc).

Last edited 6 years ago by Carlton Gibson (previous) (diff)

comment:9 by Carlton Gibson, 6 years ago

There is (of course) the wontfix scenario.

The "Defining a custom email backend" section begins thus:

If you need to change how emails are sent you can write your own email backend...

People wanting BCCs for console or file based backends could just do this. We might say it's already documented...

comment:10 by Josh Schneier, 6 years ago

I have a cursory implementation of your solution which is simple enough.

My hesitation is that this and the other opened issue seems more like users wanting to ensure that their code is doing precisely what they are telling it to do and then not realizing that BCC is not included in the actual MIME text which is what we are printing (I speak from experience). Or that they want to be able to access it and expect to see everything since these backends are explicitly for development.

How about we add the hook and document BCC explicitly or that the default implementations return the MIME text?

The no-BCC-in-MIME is very much part of the black box that is the email spec which we shouldn't be reproducing in the docs but it is confusing when encountered in this context.

comment:11 by Josh Schneier, 6 years ago

I updated the PR with a provisional patch. I am also okaying wontfix-ing this.

comment:12 by Carlton Gibson, 6 years ago

Hi Josh. Thanks for updating the PR. It's looking good.

... it is confusing when encountered in this context.

Right. OK. I think you might have convinced me. :-)

What's your thought here: are you keen to add the (small?) amount of set logic needed to calculate the BCC list and add that to the formatted message for these backends?

comment:13 by Josh Schneier, 6 years ago

Right. OK. I think you might have convinced me. :-)
What's your thought here: are you keen to add the (small?) amount of set logic needed to calculate the BCC list and add that to the formatted message for these backends?

Is there any sort of backwards compatibility guarantee on the output?

comment:14 by Muesliflyer, 5 years ago

I re-opened issue #18582 because I don't understand why it has been closed as invalid. The issue persists.

I suspect that a fix to #18582 would also fix this issue. Is it really an issue of the backends or is it an issue of EmailMessage.message() missing a line of code for bcc?

comment:15 by Mariusz Felisiak, 5 years ago

Muesliflyer, please don't reopen old tickets. If you have something to add about displaying Bcc addresses in the console you can leave your comments here.

comment:16 by Jacob Walls, 4 years ago

Has patch: set
Needs tests: set
Patch needs improvement: set

comment:17 by Michael, 2 years ago

I thought my email sending was not working, stepped code all the way to when send_mail is called and sure enough its getting a valid BCC address, but its not being printed. Then I starting googling and found this issue. I recommend printing out the the BCC in console/file email backends, as thats what people expect, and they are tools for debugging.

comment:18 by Mariusz Felisiak, 2 years ago

Michael, feel-free to prepare a patch.

comment:19 by Michael, 2 years ago

Mariusz Felisiak

Hi Mariusz, is what I came up with, do you the solution is good enough for the Django Core? If it is a will create a patch out of it

import re
from io import StringIO

from django.core.mail.backends.console import EmailBackend


class WithBccEmailBackend(EmailBackend):
    """Email backend that writes messages to console instead of sending them, by defaul
    the Django Console back end does not print the Bcc fields, with just predixes
    the original output with the Bcc field.
    """

    def write_message(self, message):
        if message.bcc:
            self.stream_backup = self.stream
            self.stream = StringIO()
            super().write_message(message)
            content = self.stream.getvalue()
            bcc = f"Bcc: {', '.join(message.bcc)}\n"
            new_content = re.sub(r'(Reply-To: |Date: )', bcc + r'\g<1>', content, 1)
            self.stream = self.stream_backup
            self.stream.write(new_content)
        else:
            super().write_message(message)

in reply to:  19 comment:20 by Mariusz Felisiak, 2 years ago

Replying to Michael:

Hi Mariusz, is what I came up with, do you the solution is good enough for the Django Core? If it is a will create a patch out of it

As far as I'm aware we don't need a new backend, rather we should extend the existing EmailBackend. Have you seen the previous patch?

comment:21 by Ülgen Sarıkavak, 6 weeks ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top