Opened 5 years ago

Closed 5 years ago

Last modified 5 months ago

#17281 closed Bug (fixed)

AdminErrorHandler silently fails if the log message contains newlines

Reported by: Russell Keith-Magee Owned by: marw85
Component: Core (Other) Version:
Severity: Normal Keywords:
Cc: glencoates, iacobcatalin@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

If you have loggers configured with an django.utils.log.AdminEmailHandler, and you log a message that has a newline in it, the log message is handled by the logger, but is silently discarded.

For example if you sent the following:

logger.error('This is a\n test message')

The log message will be correctly written to any text-based log handlers or console log handlers, but the AdminEmailHandler silently fails.

This is because the subject of an email can't contain newlines.

Given that the AdminEmailHandler is intended as a mechanism for reporting serious errors, it's pretty bad that it can fail silently due to the contents of the message it's reporting. The "subject" of the log email should be cleansed of newlines before the call to mail_admins is made.

This cleansing could also be performed in the mail utilities themselves. However, I'm not convinced this is the right place; the general mail tools have "fail_silently" as an option, so it would be possible to handle this error in a better way. Failing is not an option during logging, so we should make sure that the messages to be sent are appropriately cleansed.

Attachments (2)

patch.diff (1.3 KB) - added by elbarto 5 years ago.
django-17281.2.diff (3.1 KB) - added by marw85 5 years ago.
added tests, escape characters instead of truncate on newline

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by elbarto

Owner: changed from nobody to elbarto

comment:2 Changed 5 years ago by glencoates

Cc: glencoates added

I would guess that in many cases the bit of an error message after the first newline is a stack trace, and that in that case you probably don't want all that stuff in the email subject (it will be in the body anyway). It would be good if AdminEmailHandler defined a make_subject( record ) method which strips everything after the first newline, but allows the user to easily override that behaviour in a custom handler class if they choose.

Changed 5 years ago by elbarto

Attachment: patch.diff added

comment:3 Changed 5 years ago by elbarto

Has patch: set

I've followed the glencoates indications in order to create the patch.

comment:4 Changed 5 years ago by marw85

Needs tests: set
Owner: changed from elbarto to marw85
Status: newassigned

Changed 5 years ago by marw85

Attachment: django-17281.2.diff added

added tests, escape characters instead of truncate on newline

comment:5 Changed 5 years ago by Wiktor

I checked and the problem exists in django 1.3.1. The supplied patch fixes it.

comment:6 Changed 5 years ago by marw85

Needs tests: unset

comment:7 Changed 5 years ago by Tomek Paczkowski

Triage Stage: AcceptedReady for checkin

I've reviewed the patch and it looks very good.

comment:8 Changed 5 years ago by Catalin Iacob

Cc: iacobcatalin@… added

comment:9 Changed 5 years ago by Julien Phalip

Resolution: fixed
Status: assignedclosed

In [17501]:

Fixed #17281 -- Prevented AdminErrorHandler from silently failing if the log message contains newlines. Thanks to Russell Keith-Magee for the report and to Bartolome Sanchez Salado and Marcin Wróbel for the patch.

comment:10 Changed 5 months ago by Claude Paroz <claude@…>

In c3e10869:

Stopped truncating AdminEmailHandler message subjects

Refs #26572, #17281. The RFC doesn't limit total length, just the line length
which is already taken care of by Python itself.
Thanks Tim Graham for the review.

Note: See TracTickets for help on using tickets.
Back to Top