From e295033144e3085abaf9277d1bb0a6436ce73e01 Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Fri, 14 Feb 2025 10:58:01 -0800 Subject: [PATCH] Fixed #36138 -- Changed ADMINS and MANAGERS settings to lists of strings. Previously, the ADMINS and MANAGERS settings were lists of (name, address) tuples (where the name had been unused). Deprecated use of tuples. Updated settings value sanity checks, and changed from ValueError to ImproperlyConfigured. --- django/core/mail/__init__.py | 25 +++++++- .../contributing/writing-documentation.txt | 10 +-- docs/internals/deprecation.txt | 3 + docs/ref/settings.txt | 12 +++- docs/releases/6.0.txt | 5 ++ tests/logging_tests/tests.py | 18 +++--- tests/mail/test_sendtestemail.py | 10 +-- tests/mail/tests.py | 63 ++++++++++++++----- tests/middleware/tests.py | 2 +- tests/view_tests/tests/test_debug.py | 6 +- 10 files changed, 102 insertions(+), 52 deletions(-) diff --git a/django/core/mail/__init__.py b/django/core/mail/__init__.py index 374d459c52..e822f1d2a2 100644 --- a/django/core/mail/__init__.py +++ b/django/core/mail/__init__.py @@ -2,7 +2,10 @@ Tools for sending email. """ +import warnings + from django.conf import settings +from django.core.exceptions import ImproperlyConfigured # Imported for backwards compatibility and for the sake # of a cleaner namespace. These symbols used to be in @@ -21,6 +24,8 @@ from django.core.mail.message import ( make_msgid, ) from django.core.mail.utils import DNS_NAME, CachedDnsName +from django.utils.deprecation import RemovedInDjango70Warning +from django.utils.functional import Promise from django.utils.module_loading import import_string __all__ = [ @@ -132,14 +137,28 @@ def _send_server_message( if not recipients: return - if not all(isinstance(a, (list, tuple)) and len(a) == 2 for a in recipients): - raise ValueError(f"The {setting_name} setting must be a list of 2-tuples.") + # RemovedInDjango70Warning. + if all(isinstance(a, (list, tuple)) and len(a) == 2 for a in recipients): + warnings.warn( + f"Using (name, address) pairs in the {setting_name} setting is deprecated." + " Replace with a list of email address strings.", + RemovedInDjango70Warning, + stacklevel=2, + ) + recipients = [a[1] for a in recipients] + + if not isinstance(recipients, (list, tuple)) or not all( + isinstance(address, (str, Promise)) for address in recipients + ): + raise ImproperlyConfigured( + f"The {setting_name} setting must be a list of email address strings." + ) mail = EmailMultiAlternatives( subject="%s%s" % (settings.EMAIL_SUBJECT_PREFIX, subject), body=message, from_email=settings.SERVER_EMAIL, - to=[a[1] for a in recipients], + to=recipients, connection=connection, ) if html_message: diff --git a/docs/internals/contributing/writing-documentation.txt b/docs/internals/contributing/writing-documentation.txt index 10b7edbca8..6aaf8bd0d9 100644 --- a/docs/internals/contributing/writing-documentation.txt +++ b/docs/internals/contributing/writing-documentation.txt @@ -606,15 +606,7 @@ example: Default: ``[]`` (Empty list) - A list of all the people who get code error notifications. When - ``DEBUG=False`` and a view raises an exception, Django will email these people - with the full exception information. Each member of the list should be a tuple - of (Full name, email address). Example:: - - [("John", "john@example.com"), ("Mary", "mary@example.com")] - - Note that Django will email *all* of these people whenever an error happens. - See :doc:`/howto/error-reporting` for more information. + A list of all the people who get code error notifications... This marks up the following header as the "canonical" target for the setting ``ADMINS``. This means any time I talk about ``ADMINS``, diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 1ac2c291e0..24831951a7 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -28,6 +28,9 @@ details on these changes. * The ``URLIZE_ASSUME_HTTPS`` transitional setting will be removed. +* Support for setting the ``ADMINS`` or ``MANAGERS`` settings to a list of + (name, address) tuples will be removed. + .. _deprecation-removed-in-6.1: 6.1 diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index ab82b539b0..e6fa3f4221 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -52,9 +52,13 @@ A list of all the people who get code error notifications. When is configured in :setting:`LOGGING` (done by default), Django emails these people the details of exceptions raised in the request/response cycle. -Each item in the list should be a tuple of (Full name, email address). Example:: +Each item in the list should be an email address string. Example:: - [("John", "john@example.com"), ("Mary", "mary@example.com")] + ADMINS = ["john@example.com", '"Ng, Mary" '] + +.. versionchanged:: 6.0 + + In older versions, required a list of (name, address) tuples. .. setting:: ALLOWED_HOSTS @@ -2074,6 +2078,10 @@ A list in the same format as :setting:`ADMINS` that specifies who should get broken link notifications when :class:`~django.middleware.common.BrokenLinkEmailsMiddleware` is enabled. +.. versionchanged:: 6.0 + + In older versions, required a list of (name, address) tuples. + .. setting:: MEDIA_ROOT ``MEDIA_ROOT`` diff --git a/docs/releases/6.0.txt b/docs/releases/6.0.txt index 2cbab2dace..f651274dfe 100644 --- a/docs/releases/6.0.txt +++ b/docs/releases/6.0.txt @@ -328,6 +328,11 @@ Miscellaneous * ``URLIZE_ASSUME_HTTPS`` transitional setting is deprecated. +* Setting :setting:`ADMINS` or :setting:`MANAGERS` to a list of (name, address) + tuples is deprecated. Set to a list of email address strings instead. Django + never used the name portion. To include a name, format the address string as + ``'"Name"
'`` or use Python's :func:`email.utils.formataddr`. + Features removed in 6.0 ======================= diff --git a/tests/logging_tests/tests.py b/tests/logging_tests/tests.py index e58109fb78..c8de1e415e 100644 --- a/tests/logging_tests/tests.py +++ b/tests/logging_tests/tests.py @@ -248,7 +248,7 @@ class AdminEmailHandlerTest(SimpleTestCase): self.assertTrue(admin_email_handler.connection().fail_silently) @override_settings( - ADMINS=[("whatever admin", "admin@example.com")], + ADMINS=["admin@example.com"], EMAIL_SUBJECT_PREFIX="-SuperAwesomeSubject-", ) def test_accepts_args(self): @@ -280,7 +280,7 @@ class AdminEmailHandlerTest(SimpleTestCase): admin_email_handler.filters = orig_filters @override_settings( - ADMINS=[("whatever admin", "admin@example.com")], + ADMINS=["admin@example.com"], EMAIL_SUBJECT_PREFIX="-SuperAwesomeSubject-", INTERNAL_IPS=["127.0.0.1"], ) @@ -319,7 +319,7 @@ class AdminEmailHandlerTest(SimpleTestCase): admin_email_handler.filters = orig_filters @override_settings( - ADMINS=[("admin", "admin@example.com")], + ADMINS=["admin@example.com"], EMAIL_SUBJECT_PREFIX="", DEBUG=False, ) @@ -341,7 +341,7 @@ class AdminEmailHandlerTest(SimpleTestCase): self.assertEqual(mail.outbox[0].subject, expected_subject) @override_settings( - ADMINS=[("admin", "admin@example.com")], + ADMINS=["admin@example.com"], DEBUG=False, ) def test_uses_custom_email_backend(self): @@ -372,7 +372,7 @@ class AdminEmailHandlerTest(SimpleTestCase): admin_email_handler.email_backend = orig_email_backend @override_settings( - ADMINS=[("whatever admin", "admin@example.com")], + ADMINS=["admin@example.com"], ) def test_emit_non_ascii(self): """ @@ -393,7 +393,7 @@ class AdminEmailHandlerTest(SimpleTestCase): self.assertIn("Report at %s" % url_path, msg.body) @override_settings( - MANAGERS=[("manager", "manager@example.com")], + MANAGERS=["manager@example.com"], DEBUG=False, ) def test_customize_send_mail_method(self): @@ -435,7 +435,7 @@ class AdminEmailHandlerTest(SimpleTestCase): admin_email_handler = self.get_admin_email_handler(self.logger) self.assertEqual(admin_email_handler.reporter_class, ExceptionReporter) - @override_settings(ADMINS=[("A.N.Admin", "admin@example.com")]) + @override_settings(ADMINS=["admin@example.com"]) def test_custom_exception_reporter_is_used(self): record = self.logger.makeRecord( "name", logging.ERROR, "function", "lno", "message", None, None @@ -449,7 +449,7 @@ class AdminEmailHandlerTest(SimpleTestCase): msg = mail.outbox[0] self.assertEqual(msg.body, "message\n\ncustom traceback text") - @override_settings(ADMINS=[("admin", "admin@example.com")]) + @override_settings(ADMINS=["admin@example.com"]) def test_emit_no_form_tag(self): """HTML email doesn't contain forms.""" handler = AdminEmailHandler(include_html=True) @@ -567,7 +567,7 @@ class SecurityLoggerTest(LoggingAssertionMixin, SimpleTestCase): ) @override_settings( - ADMINS=[("admin", "admin@example.com")], + ADMINS=["admin@example.com"], DEBUG=False, ) def test_suspicious_email_admins(self): diff --git a/tests/mail/test_sendtestemail.py b/tests/mail/test_sendtestemail.py index c871f1a299..c7195ba9a1 100644 --- a/tests/mail/test_sendtestemail.py +++ b/tests/mail/test_sendtestemail.py @@ -4,14 +4,8 @@ from django.test import SimpleTestCase, override_settings @override_settings( - ADMINS=( - ("Admin", "admin@example.com"), - ("Admin and Manager", "admin_and_manager@example.com"), - ), - MANAGERS=( - ("Manager", "manager@example.com"), - ("Admin and Manager", "admin_and_manager@example.com"), - ), + ADMINS=["admin@example.com", "admin_and_manager@example.com"], + MANAGERS=["manager@example.com", "admin_and_manager@example.com"], ) class SendTestEmailManagementCommand(SimpleTestCase): """ diff --git a/tests/mail/tests.py b/tests/mail/tests.py index 950ec7fa35..8180887a8a 100644 --- a/tests/mail/tests.py +++ b/tests/mail/tests.py @@ -19,6 +19,7 @@ from textwrap import dedent from unittest import mock, skipUnless from django.core import mail +from django.core.exceptions import ImproperlyConfigured from django.core.mail import ( DNS_NAME, BadHeaderError, @@ -35,6 +36,7 @@ from django.core.mail.backends import console, dummy, filebased, locmem, smtp from django.core.mail.message import sanitize_address from django.test import SimpleTestCase, override_settings from django.test.utils import requires_tz_support +from django.utils.deprecation import RemovedInDjango70Warning from django.utils.translation import gettext_lazy try: @@ -1139,7 +1141,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): @override_settings( EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend", - ADMINS=[("nobody", "nobody@example.com")], + ADMINS=["nobody@example.com"], ) def test_connection_arg_mail_admins(self): mail.outbox = [] @@ -1152,7 +1154,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): @override_settings( EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend", - MANAGERS=[("nobody", "nobody@example.com")], + MANAGERS=["nobody@example.com"], ) def test_connection_arg_mail_managers(self): mail.outbox = [] @@ -1779,13 +1781,13 @@ class BaseEmailBackendTests(MailTestsMixin): def test_mail_admins_and_managers(self): tests = ( - # The ADMINS and MANAGERS settings are lists of (name, address) tuples. - [("Name, Full", "test@example.com")], + # The ADMINS and MANAGERS settings are lists of email strings. + ['"Name, Full" '], # Lists and tuples are interchangeable. - [["Name, Full", "test@example.com"], ["ignored", "other@example.com"]], - (("", "test@example.com"), ("", "other@example.com")), + ["test@example.com", "other@example.com"], + ("test@example.com", "other@example.com"), # Lazy strings are supported. - [(gettext_lazy("Name, Full"), gettext_lazy("test@example.com"))], + [gettext_lazy("test@example.com")], ) for setting, mail_func in ( ("ADMINS", mail_admins), @@ -1799,10 +1801,10 @@ class BaseEmailBackendTests(MailTestsMixin): ): mail_func("subject", "content") message = self.get_the_message() - expected_to = ", ".join([str(address) for _, address in value]) + expected_to = ", ".join([str(address) for address in value]) self.assertEqual(message.get_all("to"), [expected_to]) - @override_settings(MANAGERS=[("nobody", "nobody@example.com")]) + @override_settings(MANAGERS=["nobody@example.com"]) def test_html_mail_managers(self): """Test html_message argument to mail_managers""" mail_managers("Subject", "Content", html_message="HTML Content") @@ -1817,7 +1819,7 @@ class BaseEmailBackendTests(MailTestsMixin): self.assertEqual(message.get_payload(1).get_payload(), "HTML Content") self.assertEqual(message.get_payload(1).get_content_type(), "text/html") - @override_settings(ADMINS=[("nobody", "nobody@example.com")]) + @override_settings(ADMINS=["nobody@example.com"]) def test_html_mail_admins(self): """Test html_message argument to mail_admins""" mail_admins("Subject", "Content", html_message="HTML Content") @@ -1833,8 +1835,8 @@ class BaseEmailBackendTests(MailTestsMixin): self.assertEqual(message.get_payload(1).get_content_type(), "text/html") @override_settings( - ADMINS=[("nobody", "nobody+admin@example.com")], - MANAGERS=[("nobody", "nobody+manager@example.com")], + ADMINS=["nobody+admin@example.com"], + MANAGERS=["nobody+manager@example.com"], ) def test_manager_and_admin_mail_prefix(self): """ @@ -1859,13 +1861,40 @@ class BaseEmailBackendTests(MailTestsMixin): mail_func("hi", "there") self.assertEqual(self.get_mailbox_content(), []) + # RemovedInDjango70Warning. + def test_deprecated_admins_managers_tuples(self): + tests = ( + [("nobody", "nobody@example.com"), ("other", "other@example.com")], + [["nobody", "nobody@example.com"], ["other", "other@example.com"]], + ) + for setting, mail_func in ( + ("ADMINS", mail_admins), + ("MANAGERS", mail_managers), + ): + msg = ( + f"Using (name, address) pairs in the {setting} setting is deprecated." + " Replace with a list of email address strings." + ) + for value in tests: + self.flush_mailbox() + with ( + self.subTest(setting=setting, value=value), + self.settings(**{setting: value}), + ): + with self.assertWarnsMessage(RemovedInDjango70Warning, msg): + mail_func("subject", "content") + message = self.get_the_message() + expected_to = ", ".join([str(address) for _, address in value]) + self.assertEqual(message.get_all("to"), [expected_to]) + def test_wrong_admins_managers(self): tests = ( "test@example.com", gettext_lazy("test@example.com"), - ("test@example.com",), - ["test@example.com", "other@example.com"], - ("test@example.com", "other@example.com"), + # RemovedInDjango70Warning: uncomment these cases when support for + # deprecated (name, address) tuples is removed. + # [("nobody", "nobody@example.com"), ("other", "other@example.com")], + # [["nobody", "nobody@example.com"], ["other", "other@example.com"]], [("name", "test", "example.com")], [("Name