Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15603 closed (fixed)

Don't send HTML emails by default

Reported by: kmtracey Owned by: adrian
Component: Core (Other) Version: 1.3-rc
Severity: Keywords: blocker
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In my experience (gmail accounts) the HTML error traceback emails new with 1.3 are much harder to read than the previous plaintext emails. They also expose far more information (potentially sensitive local variable values) than the previous emails. Proposal is to not send HTML by default, but make it easy to configure for HTML inclusion if that is what is really wanted. Attached patch does this. To get HTML included in the email you'd change your logger config to:

LOGGING = { 
    'version': 1, 
    'disable_existing_loggers': False, 
    'handlers': { 
        'mail_admins': { 
            'level': 'ERROR', 
            'class': 'django.utils.log.AdminEmailHandler', 
            'include_html': True, 
        } 
    }, 
    'loggers': { 
        'django.request':{ 
            'handlers': ['mail_admins'], 
            'level': 'ERROR', 
            'propagate': True, 
        }, 
    } 
}

Only change is the inclusion of the include_html: True on the mail_admins handler definition.

Attachments (1)

logger.diff (2.8 KB) - added by kmtracey 3 years ago.
Updated patch to include doc

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 years ago by kmtracey

  • Owner changed from nobody to kmtracey
  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by kmtracey

Updated patch to include doc

comment:2 Changed 3 years ago by kmtracey

I updated the patch to include some doc. It doesn't go into any detail about pitfalls of sending HTML email, though, nor recommend external packages in place of this built-in mechanism...two things I think were mentioned as things that should be noted in the doc, so suggestions for proper wording for that kind of stuff would be helpful (I had a hard time figuring out how to word that properly...)

comment:3 Changed 3 years ago by russellm

  • Keywords blocker added

We need to address this before we cut 1.3 final.

comment:4 Changed 3 years ago by kmtracey

  • Owner kmtracey deleted

I'm going to have to leave shortly so removing myself as owner.

comment:5 follow-up: Changed 3 years ago by adrian

  • Owner set to adrian

Agreed this is something we need to do. I'll take a look at this patch and commit.

comment:6 in reply to: ↑ 5 Changed 3 years ago by jezdez

Replying to adrian:

Agreed this is something we need to do. I'll take a look at this patch and commit.

Yeah, I believe linking to http://djangopackages.com/grids/g/error-handling/ would be useful in case we try point to more complex error handling tools.

comment:7 Changed 3 years ago by jacob

I don't think that's quite the right target for a link -- Lumberjack and slow-log don't really replace emails the way Sentry does. We probably should create an "error email replacements" grid. Better name, though, please.

comment:8 Changed 3 years ago by adrian

In [15849]:

Corrected email --> e-mail in topics/logging.txt. Refs #15603

comment:9 Changed 3 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

In [15850]:

Fixed #15603 -- Changed the traceback error e-mails not to use HTML by default. It's now configurable with an 'include_html' parameter to AdminEmailHandler. Thanks, kmtracey

comment:10 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.