From a444d2600b2b4a43cb88a4db27457124ca10df9e Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Thu, 28 Nov 2024 11:49:42 +0100 Subject: [PATCH 01/32] Used skipIf/UnlessDBFeature in test.serializers. --- tests/serializers/test_data.py | 105 ++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/tests/serializers/test_data.py b/tests/serializers/test_data.py index 808db41634..c913c59dd3 100644 --- a/tests/serializers/test_data.py +++ b/tests/serializers/test_data.py @@ -13,7 +13,7 @@ import uuid from django.core import serializers from django.db import connection, models -from django.test import TestCase +from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature from .models import ( Anchor, @@ -256,7 +256,6 @@ uuid_obj = uuid.uuid4() test_data = [ # Format: (data type, PK value, Model Class, data) (data_obj, 1, BinaryData, memoryview(b"\x05\xFD\x00")), - (data_obj, 2, BinaryData, None), (data_obj, 5, BooleanData, True), (data_obj, 6, BooleanData, False), (data_obj, 7, BooleanData, None), @@ -265,7 +264,6 @@ test_data = [ (data_obj, 12, CharData, "None"), (data_obj, 13, CharData, "null"), (data_obj, 14, CharData, "NULL"), - (data_obj, 15, CharData, None), # (We use something that will fit into a latin1 database encoding here, # because that is still the default used on many system setups.) (data_obj, 16, CharData, "\xa5"), @@ -274,13 +272,11 @@ test_data = [ (data_obj, 30, DateTimeData, datetime.datetime(2006, 6, 16, 10, 42, 37)), (data_obj, 31, DateTimeData, None), (data_obj, 40, EmailData, "hovercraft@example.com"), - (data_obj, 41, EmailData, None), (data_obj, 42, EmailData, ""), (data_obj, 50, FileData, "file:///foo/bar/whiz.txt"), # (data_obj, 51, FileData, None), (data_obj, 52, FileData, ""), (data_obj, 60, FilePathData, "/foo/bar/whiz.txt"), - (data_obj, 61, FilePathData, None), (data_obj, 62, FilePathData, ""), (data_obj, 70, DecimalData, decimal.Decimal("12.345")), (data_obj, 71, DecimalData, decimal.Decimal("-12.345")), @@ -304,7 +300,6 @@ test_data = [ (data_obj, 130, PositiveSmallIntegerData, 12), (data_obj, 131, PositiveSmallIntegerData, None), (data_obj, 140, SlugData, "this-is-a-slug"), - (data_obj, 141, SlugData, None), (data_obj, 142, SlugData, ""), (data_obj, 150, SmallData, 12), (data_obj, 151, SmallData, -12), @@ -320,7 +315,6 @@ Several of them. The end.""", ), (data_obj, 161, TextData, ""), - (data_obj, 162, TextData, None), (data_obj, 170, TimeData, datetime.time(10, 42, 37)), (data_obj, 171, TimeData, None), (generic_obj, 200, GenericData, ["Generic Object 1", "tag1", "tag2"]), @@ -388,15 +382,6 @@ The end.""", (pk_obj, 750, SmallPKData, 12), (pk_obj, 751, SmallPKData, -12), (pk_obj, 752, SmallPKData, 0), - ( - pk_obj, - 760, - TextPKData, - """This is a long piece of text. - It contains line breaks. - Several of them. - The end.""", - ), (pk_obj, 770, TimePKData, datetime.time(10, 42, 37)), (pk_obj, 791, UUIDData, uuid_obj), (fk_obj, 792, FKToUUID, uuid_obj), @@ -420,41 +405,11 @@ The end.""", ] -# Because Oracle treats the empty string as NULL, Oracle is expected to fail -# when field.empty_strings_allowed is True and the value is None; skip these -# tests. -if connection.features.interprets_empty_strings_as_nulls: - test_data = [ - data - for data in test_data - if not ( - data[0] == data_obj - and data[2]._meta.get_field("data").empty_strings_allowed - and data[3] is None - ) - ] - - -if not connection.features.supports_index_on_text_field: - test_data = [data for data in test_data if data[2] != TextPKData] - - class SerializerDataTests(TestCase): pass -def serializerTest(self, format): - # FK to an object with PK of 0. This won't work on MySQL without the - # NO_AUTO_VALUE_ON_ZERO SQL mode since it won't let you create an object - # with an autoincrement primary key of 0. - if connection.features.allows_auto_pk_0: - test_data.extend( - [ - (data_obj, 0, Anchor, "Anchor 0"), - (fk_obj, 465, FKData, 0), - ] - ) - +def assert_serializer(self, format, data): # Create all the objects defined in the test data objects = [] instance_count = {} @@ -486,4 +441,60 @@ def serializerTest(self, format): self.assertEqual(count, klass.objects.count()) +def serializerTest(self, format): + assert_serializer(self, format, test_data) + + +@skipUnlessDBFeature("allows_auto_pk_0") +def serializerTestPK0(self, format): + # FK to an object with PK of 0. This won't work on MySQL without the + # NO_AUTO_VALUE_ON_ZERO SQL mode since it won't let you create an object + # with an autoincrement primary key of 0. + data = [ + (data_obj, 0, Anchor, "Anchor 0"), + (fk_obj, 1, FKData, 0), + ] + assert_serializer(self, format, data) + + +@skipIfDBFeature("interprets_empty_strings_as_nulls") +def serializerTestNullValueStingField(self, format): + data = [ + (data_obj, 1, BinaryData, None), + (data_obj, 2, CharData, None), + (data_obj, 3, EmailData, None), + (data_obj, 4, FilePathData, None), + (data_obj, 5, SlugData, None), + (data_obj, 6, TextData, None), + ] + assert_serializer(self, format, data) + + +@skipUnlessDBFeature("supports_index_on_text_field") +def serializerTestTextFieldPK(self, format): + data = [ + ( + pk_obj, + 1, + TextPKData, + """This is a long piece of text. + It contains line breaks. + Several of them. + The end.""", + ), + ] + assert_serializer(self, format, data) + + register_tests(SerializerDataTests, "test_%s_serializer", serializerTest) +register_tests(SerializerDataTests, "test_%s_serializer_pk_0", serializerTestPK0) +register_tests( + SerializerDataTests, + "test_%s_serializer_null_value_string_field", + serializerTestNullValueStingField, +) +register_tests( + SerializerDataTests, + "test_%s_serializer_text_field_pk", + serializerTestTextFieldPK, +) From d783a6f1c0dfbcaab8e953c52ec344f982754b15 Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Thu, 28 Nov 2024 12:07:21 +0100 Subject: [PATCH 02/32] Improved assert_serializer test assertions in test.serializers. --- tests/serializers/test_data.py | 62 ++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/tests/serializers/test_data.py b/tests/serializers/test_data.py index c913c59dd3..f8763f6e42 100644 --- a/tests/serializers/test_data.py +++ b/tests/serializers/test_data.py @@ -10,6 +10,7 @@ forward, backwards and self references. import datetime import decimal import uuid +from collections import namedtuple from django.core import serializers from django.db import connection, models @@ -239,22 +240,22 @@ def inherited_compare(testcase, pk, klass, data): testcase.assertEqual(value, getattr(instance, key)) -# Define some data types. Each data type is -# actually a pair of functions; one to create -# and one to compare objects of that type -data_obj = (data_create, data_compare) -generic_obj = (generic_create, generic_compare) -fk_obj = (fk_create, fk_compare) -m2m_obj = (m2m_create, m2m_compare) -im2m_obj = (im2m_create, im2m_compare) -im_obj = (im_create, im_compare) -o2o_obj = (o2o_create, o2o_compare) -pk_obj = (pk_create, pk_compare) -inherited_obj = (inherited_create, inherited_compare) +# Define some test helpers. Each has a pair of functions: one to create objects and one +# to make assertions against objects of a particular type. +TestHelper = namedtuple("TestHelper", ["create_object", "compare_object"]) +data_obj = TestHelper(data_create, data_compare) +generic_obj = TestHelper(generic_create, generic_compare) +fk_obj = TestHelper(fk_create, fk_compare) +m2m_obj = TestHelper(m2m_create, m2m_compare) +im2m_obj = TestHelper(im2m_create, im2m_compare) +im_obj = TestHelper(im_create, im_compare) +o2o_obj = TestHelper(o2o_create, o2o_compare) +pk_obj = TestHelper(pk_create, pk_compare) +inherited_obj = TestHelper(inherited_create, inherited_compare) uuid_obj = uuid.uuid4() test_data = [ - # Format: (data type, PK value, Model Class, data) + # Format: (test helper, PK value, Model Class, data) (data_obj, 1, BinaryData, memoryview(b"\x05\xFD\x00")), (data_obj, 5, BooleanData, True), (data_obj, 6, BooleanData, False), @@ -410,35 +411,36 @@ class SerializerDataTests(TestCase): def assert_serializer(self, format, data): - # Create all the objects defined in the test data + # Create all the objects defined in the test data. objects = [] - instance_count = {} - for func, pk, klass, datum in test_data: + for test_helper, pk, model, data_value in data: with connection.constraint_checks_disabled(): - objects.extend(func[0](pk, klass, datum)) + objects.extend(test_helper.create_object(pk, model, data_value)) - # Get a count of the number of objects created for each class - for klass in instance_count: - instance_count[klass] = klass.objects.count() + # Get a count of the number of objects created for each model class. + instance_counts = {} + for _, _, model, _ in data: + if model not in instance_counts: + instance_counts[model] = model.objects.count() - # Add the generic tagged objects to the object list + # Add the generic tagged objects to the object list. objects.extend(Tag.objects.all()) - # Serialize the test database + # Serialize the test database. serialized_data = serializers.serialize(format, objects, indent=2) for obj in serializers.deserialize(format, serialized_data): obj.save() - # Assert that the deserialized data is the same - # as the original source - for func, pk, klass, datum in test_data: - func[1](self, pk, klass, datum) + # Assert that the deserialized data is the same as the original source. + for test_helper, pk, model, data_value in data: + with self.subTest(model=model, data_value=data_value): + test_helper.compare_object(self, pk, model, data_value) - # Assert that the number of objects deserialized is the - # same as the number that was serialized. - for klass, count in instance_count.items(): - self.assertEqual(count, klass.objects.count()) + # Assert no new objects were created. + for model, count in instance_counts.items(): + with self.subTest(model=model, count=count): + self.assertEqual(count, model.objects.count()) def serializerTest(self, format): From 81cf690111e49b9cf9d8a3b8a71767f3c8685d5b Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Fri, 29 Nov 2024 21:34:39 +0100 Subject: [PATCH 03/32] Refs #373 -- Fixed CompositePKChecksTests.test_composite_pk_cannot_include_generated_field() test crash on databases with no GeneratedField support. --- tests/composite_pk/test_checks.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/composite_pk/test_checks.py b/tests/composite_pk/test_checks.py index 02a162c31d..58b580ca85 100644 --- a/tests/composite_pk/test_checks.py +++ b/tests/composite_pk/test_checks.py @@ -1,7 +1,7 @@ from django.core import checks from django.db import connection, models from django.db.models import F -from django.test import TestCase +from django.test import TestCase, skipUnlessAnyDBFeature from django.test.utils import isolate_apps @@ -217,16 +217,18 @@ class CompositePKChecksTests(TestCase): ], ) + @skipUnlessAnyDBFeature( + "supports_virtual_generated_columns", + "supports_stored_generated_columns", + ) def test_composite_pk_cannot_include_generated_field(self): - is_oracle = connection.vendor == "oracle" - class Foo(models.Model): pk = models.CompositePrimaryKey("id", "foo") id = models.IntegerField() foo = models.GeneratedField( expression=F("id"), output_field=models.IntegerField(), - db_persist=not is_oracle, + db_persist=connection.features.supports_stored_generated_columns, ) self.assertEqual( From 49761ac99a064236b4280ca55f97a896913109cd Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Sun, 1 Dec 2024 20:55:35 +0100 Subject: [PATCH 04/32] Refs #373 -- Simplified DatabaseIntrospection.get_constraints() tests for composite primary keys. --- tests/composite_pk/tests.py | 61 +++++++------------------------------ 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py index 71522cb836..1a0a327baf 100644 --- a/tests/composite_pk/tests.py +++ b/tests/composite_pk/tests.py @@ -1,5 +1,4 @@ import json -import unittest from uuid import UUID import yaml @@ -35,9 +34,9 @@ class CompositePKTests(TestCase): cls.comment = Comment.objects.create(tenant=cls.tenant, id=1, user=cls.user) @staticmethod - def get_constraints(table): + def get_primary_key_columns(table): with connection.cursor() as cursor: - return connection.introspection.get_constraints(cursor, table) + return connection.introspection.get_primary_key_columns(cursor, table) def test_pk_updated_if_field_updated(self): user = User.objects.get(pk=self.user.pk) @@ -125,53 +124,15 @@ class CompositePKTests(TestCase): with self.assertRaises(IntegrityError): Comment.objects.create(tenant=self.tenant, id=self.comment.id) - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific test") - def test_get_constraints_postgresql(self): - user_constraints = self.get_constraints(User._meta.db_table) - user_pk = user_constraints["composite_pk_user_pkey"] - self.assertEqual(user_pk["columns"], ["tenant_id", "id"]) - self.assertIs(user_pk["primary_key"], True) - - comment_constraints = self.get_constraints(Comment._meta.db_table) - comment_pk = comment_constraints["composite_pk_comment_pkey"] - self.assertEqual(comment_pk["columns"], ["tenant_id", "comment_id"]) - self.assertIs(comment_pk["primary_key"], True) - - @unittest.skipUnless(connection.vendor == "sqlite", "SQLite specific test") - def test_get_constraints_sqlite(self): - user_constraints = self.get_constraints(User._meta.db_table) - user_pk = user_constraints["__primary__"] - self.assertEqual(user_pk["columns"], ["tenant_id", "id"]) - self.assertIs(user_pk["primary_key"], True) - - comment_constraints = self.get_constraints(Comment._meta.db_table) - comment_pk = comment_constraints["__primary__"] - self.assertEqual(comment_pk["columns"], ["tenant_id", "comment_id"]) - self.assertIs(comment_pk["primary_key"], True) - - @unittest.skipUnless(connection.vendor == "mysql", "MySQL specific test") - def test_get_constraints_mysql(self): - user_constraints = self.get_constraints(User._meta.db_table) - user_pk = user_constraints["PRIMARY"] - self.assertEqual(user_pk["columns"], ["tenant_id", "id"]) - self.assertIs(user_pk["primary_key"], True) - - comment_constraints = self.get_constraints(Comment._meta.db_table) - comment_pk = comment_constraints["PRIMARY"] - self.assertEqual(comment_pk["columns"], ["tenant_id", "comment_id"]) - self.assertIs(comment_pk["primary_key"], True) - - @unittest.skipUnless(connection.vendor == "oracle", "Oracle specific test") - def test_get_constraints_oracle(self): - user_constraints = self.get_constraints(User._meta.db_table) - user_pk = next(c for c in user_constraints.values() if c["primary_key"]) - self.assertEqual(user_pk["columns"], ["tenant_id", "id"]) - self.assertEqual(user_pk["primary_key"], 1) - - comment_constraints = self.get_constraints(Comment._meta.db_table) - comment_pk = next(c for c in comment_constraints.values() if c["primary_key"]) - self.assertEqual(comment_pk["columns"], ["tenant_id", "comment_id"]) - self.assertEqual(comment_pk["primary_key"], 1) + def test_get_primary_key_columns(self): + self.assertEqual( + self.get_primary_key_columns(User._meta.db_table), + ["tenant_id", "id"], + ) + self.assertEqual( + self.get_primary_key_columns(Comment._meta.db_table), + ["tenant_id", "comment_id"], + ) def test_in_bulk(self): """ From b8f9f625a19298379dbc16d916694702cd038f52 Mon Sep 17 00:00:00 2001 From: jburns6789 <117755326+jburns6789@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:18:05 -0500 Subject: [PATCH 05/32] Fixed #35915 -- Clarified the empty list case in QueryDict.__getitem__() docs. --- docs/ref/request-response.txt | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index 26fcb5fa08..632e222998 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -554,12 +554,21 @@ a subclass of dictionary. Exceptions are outlined here: .. method:: QueryDict.__getitem__(key) - Returns the value for the given key. If the key has more than one value, - it returns the last value. Raises + Returns the last value for the given key; or an empty list (``[]``) if the + key exists but has no values. Raises ``django.utils.datastructures.MultiValueDictKeyError`` if the key does not exist. (This is a subclass of Python's standard :exc:`KeyError`, so you can stick to catching ``KeyError``.) + .. code-block:: pycon + + >>> q = QueryDict("a=1&a=2&a=3", mutable=True) + >>> q.__getitem__("a") + '3' + >>> q.__setitem__("b", []) + >>> q.__getitem__("b") + [] + .. method:: QueryDict.__setitem__(key, value) Sets the given key to ``[value]`` (a list whose single element is From b0d9c1fe32c74b49bfebf0691530944251cad523 Mon Sep 17 00:00:00 2001 From: SaJH Date: Mon, 9 Sep 2024 23:41:56 +0900 Subject: [PATCH 06/32] Updated docs example to clear cached_property without raising AttributeError. Signed-off-by: SaJH --- docs/ref/utils.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ref/utils.txt b/docs/ref/utils.txt index 33e0fceadf..438a38cea0 100644 --- a/docs/ref/utils.txt +++ b/docs/ref/utils.txt @@ -521,7 +521,7 @@ https://web.archive.org/web/20110718035220/http://diveintomark.org/archives/2004 The cached value can be treated like an ordinary attribute of the instance:: # clear it, requiring re-computation next time it's called - del person.friends # or delattr(person, "friends") + person.__dict__.pop("friends", None) # set a value manually, that will persist on the instance until cleared person.friends = ["Huckleberry Finn", "Tom Sawyer"] From 32b9e00b0c74b3af77c25215e2e2d1254b995355 Mon Sep 17 00:00:00 2001 From: antoliny0919 Date: Fri, 15 Nov 2024 18:31:44 +0900 Subject: [PATCH 07/32] Fixed #35964 -- Cleaned up can_order and can_delete formset examples. --- docs/topics/forms/formsets.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt index 14d4962eb6..a452c7640d 100644 --- a/docs/topics/forms/formsets.txt +++ b/docs/topics/forms/formsets.txt @@ -571,14 +571,12 @@ happen when the user changes these values: ... {"title": "Article #2", "pub_date": datetime.date(2008, 5, 11)}, ... ], ... ) - >>> formset.is_valid() - True >>> for form in formset.ordered_forms: ... print(form.cleaned_data) ... - {'pub_date': datetime.date(2008, 5, 1), 'ORDER': 0, 'title': 'Article #3'} - {'pub_date': datetime.date(2008, 5, 11), 'ORDER': 1, 'title': 'Article #2'} - {'pub_date': datetime.date(2008, 5, 10), 'ORDER': 2, 'title': 'Article #1'} + {'title': 'Article #3', 'pub_date': datetime.date(2008, 5, 1), 'ORDER': 0} + {'title': 'Article #2', 'pub_date': datetime.date(2008, 5, 11), 'ORDER': 1} + {'title': 'Article #1', 'pub_date': datetime.date(2008, 5, 10), 'ORDER': 2} :class:`~django.forms.formsets.BaseFormSet` also provides an :attr:`~django.forms.formsets.BaseFormSet.ordering_widget` attribute and @@ -690,7 +688,7 @@ delete fields you can access them with ``deleted_forms``: ... ], ... ) >>> [form.cleaned_data for form in formset.deleted_forms] - [{'DELETE': True, 'pub_date': datetime.date(2008, 5, 10), 'title': 'Article #1'}] + [{'title': 'Article #1', 'pub_date': datetime.date(2008, 5, 10), 'DELETE': True}] If you are using a :class:`ModelFormSet`, model instances for deleted forms will be deleted when you call From 2f6b096b83c55317c7ceef2d8d5dc3bee33293dc Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 30 Nov 2024 08:03:55 +0000 Subject: [PATCH 08/32] Fixed #35950 -- Restored refreshing of relations when fields deferred. Thank you to Simon Charette and Sarah Boyce for the review. Regression in 73df8b54a2fab53bec4c7573cda5ad8c869c2fd8. --- django/db/models/base.py | 19 ++++++++++--------- docs/releases/5.1.4.txt | 4 ++++ tests/contenttypes_tests/test_fields.py | 9 +++++++++ tests/defer/tests.py | 8 ++++++++ 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index a20e88749f..d948cd2a1c 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -726,12 +726,13 @@ class Model(AltersData, metaclass=ModelBase): if fields is not None: db_instance_qs = db_instance_qs.only(*fields) elif deferred_fields: - fields = { - f.attname - for f in self._meta.concrete_fields - if f.attname not in deferred_fields - } - db_instance_qs = db_instance_qs.only(*fields) + db_instance_qs = db_instance_qs.only( + *{ + f.attname + for f in self._meta.concrete_fields + if f.attname not in deferred_fields + } + ) db_instance = db_instance_qs.get() non_loaded_fields = db_instance.get_deferred_fields() @@ -748,9 +749,9 @@ class Model(AltersData, metaclass=ModelBase): field.delete_cached_value(self) # Clear cached relations. - for field in self._meta.related_objects: - if (fields is None or field.name in fields) and field.is_cached(self): - field.delete_cached_value(self) + for rel in self._meta.related_objects: + if (fields is None or rel.name in fields) and rel.is_cached(self): + rel.delete_cached_value(self) # Clear cached private relations. for field in self._meta.private_fields: diff --git a/docs/releases/5.1.4.txt b/docs/releases/5.1.4.txt index 0c21d99566..44950ac76a 100644 --- a/docs/releases/5.1.4.txt +++ b/docs/releases/5.1.4.txt @@ -12,3 +12,7 @@ Bugfixes * Fixed a crash in ``createsuperuser`` on Python 3.13+ caused by an unhandled ``OSError`` when the username could not be determined (:ticket:`35942`). + +* Fixed a regression in Django 5.1 where relational fields were not updated + when calling ``Model.refresh_from_db()`` on instances with deferred fields + (:ticket:`35950`). diff --git a/tests/contenttypes_tests/test_fields.py b/tests/contenttypes_tests/test_fields.py index ab16324fb6..fc49d59b27 100644 --- a/tests/contenttypes_tests/test_fields.py +++ b/tests/contenttypes_tests/test_fields.py @@ -57,6 +57,15 @@ class GenericForeignKeyTests(TestCase): self.assertIsNot(answer.question, old_question_obj) self.assertEqual(answer.question, old_question_obj) + def test_clear_cached_generic_relation_when_deferred(self): + question = Question.objects.create(text="question") + Answer.objects.create(text="answer", question=question) + answer = Answer.objects.defer("text").get() + old_question_obj = answer.question + # The reverse relation is refreshed even when the text field is deferred. + answer.refresh_from_db() + self.assertIsNot(answer.question, old_question_obj) + class GenericRelationTests(TestCase): def test_value_to_string(self): diff --git a/tests/defer/tests.py b/tests/defer/tests.py index 3945b667ba..989b5c63d7 100644 --- a/tests/defer/tests.py +++ b/tests/defer/tests.py @@ -290,6 +290,14 @@ class TestDefer2(AssertionMixin, TestCase): self.assertEqual(rf2.name, "new foo") self.assertEqual(rf2.value, "new bar") + def test_refresh_when_one_field_deferred(self): + s = Secondary.objects.create() + PrimaryOneToOne.objects.create(name="foo", value="bar", related=s) + s = Secondary.objects.defer("first").get() + p_before = s.primary_o2o + s.refresh_from_db() + self.assertIsNot(s.primary_o2o, p_before) + class InvalidDeferTests(SimpleTestCase): def test_invalid_defer(self): From 871e1ee5ff0b75aee5dd1bd3e88e349ca0ddc60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20Hovm=C3=B6ller?= Date: Tue, 3 Dec 2024 01:54:48 +0100 Subject: [PATCH 09/32] Removed question marks from headings in docs/topics/db/fixtures.txt. --- docs/topics/db/fixtures.txt | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/docs/topics/db/fixtures.txt b/docs/topics/db/fixtures.txt index ac5b34dae0..6066d34f8e 100644 --- a/docs/topics/db/fixtures.txt +++ b/docs/topics/db/fixtures.txt @@ -4,28 +4,25 @@ Fixtures ======== -.. seealso:: - - * :doc:`/howto/initial-data` - -What is a fixture? -================== - A *fixture* is a collection of files that contain the serialized contents of the database. Each fixture has a unique name, and the files that comprise the fixture can be distributed over multiple directories, in multiple applications. -How to produce a fixture? -========================= +.. seealso:: + + * :doc:`/howto/initial-data` + +How to produce a fixture +======================== Fixtures can be generated by :djadmin:`manage.py dumpdata `. It's also possible to generate custom fixtures by directly using :doc:`serialization tools ` or even by handwriting them. -How to use a fixture? -===================== +How to use a fixture +==================== -Fixtures can be used to pre-populate database with data for +Fixtures can be used to pre-populate the database with data for :ref:`tests `: .. code-block:: python @@ -40,8 +37,8 @@ or to provide some :ref:`initial data ` using the django-admin loaddata -Where Django looks for fixtures? -================================ +How fixtures are discovered +=========================== Django will search in these locations for fixtures: @@ -116,8 +113,8 @@ example). .. _MySQL: https://dev.mysql.com/doc/refman/en/constraint-foreign-key.html -How fixtures are saved to the database? -======================================= +How fixtures are saved to the database +====================================== When fixture files are processed, the data is saved to the database as is. Model defined :meth:`~django.db.models.Model.save` methods are not called, and From bddd35cb1a45593935d28dcb08dd69e0bf7c0d2c Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Sat, 24 Aug 2024 12:57:47 -0700 Subject: [PATCH 10/32] Refs #35581 -- Improved reporting for failing tests in mail tests. - Converted HeadersCheckMixin to MailTestsMixin for all shared helpers: - Hoisted assertStartsWith() from BaseEmailBackendTests. - Added matching assertEndsWith(). - Hoisted get_decoded_attachments() from MailTests. - Improved failure reporting in assertMessageHasHeaders(). - Used unittest subTest() to improve handling of compound test cases. - Replaced `assertTrue(test on string)` with custom assertions, so that failure reporting is more informative than `True != False`. --- tests/mail/tests.py | 192 +++++++++++++++++++++++++------------------- 1 file changed, 109 insertions(+), 83 deletions(-) diff --git a/tests/mail/tests.py b/tests/mail/tests.py index 302b746245..ad6ae38445 100644 --- a/tests/mail/tests.py +++ b/tests/mail/tests.py @@ -41,7 +41,7 @@ except ImportError: HAS_AIOSMTPD = False -class HeadersCheckMixin: +class MailTestsMixin: def assertMessageHasHeaders(self, message, headers): """ Asserts that the `message` has all `headers`. @@ -53,17 +53,53 @@ class HeadersCheckMixin: if isinstance(message, bytes): message = message_from_bytes(message) msg_headers = set(message.items()) - self.assertTrue( - headers.issubset(msg_headers), - msg="Message is missing " - "the following headers: %s" % (headers - msg_headers), - ) + if not headers.issubset(msg_headers): + missing = "\n".join(f" {h}: {v}" for h, v in headers - msg_headers) + actual = "\n".join(f" {h}: {v}" for h, v in msg_headers) + raise self.failureException( + f"Expected headers not found in message.\n" + f"Missing headers:\n{missing}\n" + f"Actual headers:\n{actual}" + ) + # In assertStartsWith()/assertEndsWith() failure messages, when truncating + # a long first ("haystack") string, include this many characters beyond the + # length of the second ("needle") string. + START_END_EXTRA_CONTEXT = 15 -class MailTests(HeadersCheckMixin, SimpleTestCase): - """ - Non-backend specific tests. - """ + def assertStartsWith(self, first, second): + if not first.startswith(second): + # Use assertEqual() for failure message with diffs. If first value + # is much longer than second, truncate end and add an ellipsis. + self.longMessage = True + max_len = len(second) + self.START_END_EXTRA_CONTEXT + start_of_first = ( + first + if len(first) <= max_len + else first[:max_len] + ("…" if isinstance(first, str) else b"...") + ) + self.assertEqual( + start_of_first, + second, + "First string doesn't start with the second.", + ) + + def assertEndsWith(self, first, second): + if not first.endswith(second): + # Use assertEqual() for failure message with diffs. If first value + # is much longer than second, truncate start and prepend an ellipsis. + self.longMessage = True + max_len = len(second) + self.START_END_EXTRA_CONTEXT + end_of_first = ( + first + if len(first) <= max_len + else ("…" if isinstance(first, str) else b"...") + first[-max_len:] + ) + self.assertEqual( + end_of_first, + second, + "First string doesn't end with the second.", + ) def get_decoded_attachments(self, django_message): """ @@ -84,6 +120,12 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): return list(iter_attachments()) + +class MailTests(MailTestsMixin, SimpleTestCase): + """ + Non-backend specific tests. + """ + def test_ascii(self): email = EmailMessage( "Subject", "Content", "from@example.com", ["to@example.com"] @@ -303,26 +345,16 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): def test_header_injection(self): msg = "Header values can't contain newlines " - email = EmailMessage( - "Subject\nInjection Test", "Content", "from@example.com", ["to@example.com"] - ) - with self.assertRaisesMessage(BadHeaderError, msg): - email.message() - email = EmailMessage( - gettext_lazy("Subject\nInjection Test"), - "Content", - "from@example.com", - ["to@example.com"], - ) - with self.assertRaisesMessage(BadHeaderError, msg): - email.message() - with self.assertRaisesMessage(BadHeaderError, msg): - EmailMessage( - "Subject", - "Content", - "from@example.com", - ["Name\nInjection test "], - ).message() + cases = [ + {"subject": "Subject\nInjection Test"}, + {"subject": gettext_lazy("Lazy Subject\nInjection Test")}, + {"to": ["Name\nInjection test "]}, + ] + for kwargs in cases: + with self.subTest(case=kwargs): + email = EmailMessage(**kwargs) + with self.assertRaisesMessage(BadHeaderError, msg): + email.message() def test_folding_white_space(self): """ @@ -615,8 +647,8 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): ("Content-Transfer-Encoding", "quoted-printable"), }, ) - self.assertTrue( - payload0.as_bytes().endswith(b"\n\nFirstname S=FCrname is a great guy.") + self.assertEndsWith( + payload0.as_bytes(), b"\n\nFirstname S=FCrname is a great guy." ) # Check the text/html alternative. payload1 = message.get_payload(1) @@ -630,10 +662,9 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): ("Content-Transfer-Encoding", "quoted-printable"), }, ) - self.assertTrue( - payload1.as_bytes().endswith( - b"\n\n

Firstname S=FCrname is a great guy.

" - ) + self.assertEndsWith( + payload1.as_bytes(), + b"\n\n

Firstname S=FCrname is a great guy.

", ) def test_attachments(self): @@ -761,34 +792,38 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): for basename, real_mimetype in files: for mimetype in test_mimetypes: - self.assertEqual(mimetypes.guess_type(basename)[0], real_mimetype) - expected_mimetype = ( - mimetype or real_mimetype or "application/octet-stream" - ) - file_path = Path(__file__).parent / "attachments" / basename - expected_content = file_path.read_bytes() - if expected_mimetype.startswith("text/"): - try: - expected_content = expected_content.decode() - except UnicodeDecodeError: - expected_mimetype = "application/octet-stream" + with self.subTest( + basename=basename, real_mimetype=real_mimetype, mimetype=mimetype + ): + self.assertEqual(mimetypes.guess_type(basename)[0], real_mimetype) + expected_mimetype = ( + mimetype or real_mimetype or "application/octet-stream" + ) + file_path = Path(__file__).parent / "attachments" / basename + expected_content = file_path.read_bytes() + if expected_mimetype.startswith("text/"): + try: + expected_content = expected_content.decode() + except UnicodeDecodeError: + expected_mimetype = "application/octet-stream" - email = EmailMessage() - email.attach_file(file_path, mimetype=mimetype) + email = EmailMessage() + email.attach_file(file_path, mimetype=mimetype) - # Check EmailMessage.attachments. - self.assertEqual(len(email.attachments), 1) - self.assertEqual(email.attachments[0].filename, basename) - self.assertEqual(email.attachments[0].mimetype, expected_mimetype) - self.assertEqual(email.attachments[0].content, expected_content) + # Check EmailMessage.attachments. + self.assertEqual(len(email.attachments), 1) + self.assertEqual(email.attachments[0].filename, basename) + self.assertEqual(email.attachments[0].mimetype, expected_mimetype) + self.assertEqual(email.attachments[0].content, expected_content) - # Check attachments in generated message. - # (The actual content is not checked as variations in platform - # line endings and rfc822 refolding complicate the logic.) - actual_attachment = self.get_decoded_attachments(email)[0] - actual_filename, actual_content, actual_mimetype = actual_attachment - self.assertEqual(actual_filename, basename) - self.assertEqual(actual_mimetype, expected_mimetype) + # Check attachments in the generated message. + # (The actual content is not checked as variations in platform + # line endings and rfc822 refolding complicate the logic.) + attachments = self.get_decoded_attachments(email) + self.assertEqual(len(attachments), 1) + actual = attachments[0] + self.assertEqual(actual.filename, basename) + self.assertEqual(actual.mimetype, expected_mimetype) def test_attach_text_as_bytes(self): msg = EmailMessage() @@ -1207,7 +1242,7 @@ class MailTests(HeadersCheckMixin, SimpleTestCase): @requires_tz_support -class MailTimeZoneTests(SimpleTestCase): +class MailTimeZoneTests(MailTestsMixin, SimpleTestCase): @override_settings( EMAIL_USE_LOCALTIME=False, USE_TZ=True, TIME_ZONE="Africa/Algiers" ) @@ -1216,7 +1251,7 @@ class MailTimeZoneTests(SimpleTestCase): EMAIL_USE_LOCALTIME=False creates a datetime in UTC. """ email = EmailMessage() - self.assertTrue(email.message()["Date"].endswith("-0000")) + self.assertEndsWith(email.message()["Date"], "-0000") @override_settings( EMAIL_USE_LOCALTIME=True, USE_TZ=True, TIME_ZONE="Africa/Algiers" @@ -1226,9 +1261,8 @@ class MailTimeZoneTests(SimpleTestCase): EMAIL_USE_LOCALTIME=True creates a datetime in the local time zone. """ email = EmailMessage() - self.assertTrue( - email.message()["Date"].endswith("+0100") - ) # Africa/Algiers is UTC+1 + # Africa/Algiers is UTC+1 year round. + self.assertEndsWith(email.message()["Date"], "+0100") class PythonGlobalState(SimpleTestCase): @@ -1259,7 +1293,7 @@ class PythonGlobalState(SimpleTestCase): self.assertIn("Content-Transfer-Encoding: base64", txt.as_string()) -class BaseEmailBackendTests(HeadersCheckMixin): +class BaseEmailBackendTests(MailTestsMixin): email_backend = None @classmethod @@ -1267,15 +1301,6 @@ class BaseEmailBackendTests(HeadersCheckMixin): cls.enterClassContext(override_settings(EMAIL_BACKEND=cls.email_backend)) super().setUpClass() - def assertStartsWith(self, first, second): - if not first.startswith(second): - self.longMessage = True - self.assertEqual( - first[: len(second)], - second, - "First string doesn't start with the second.", - ) - def get_mailbox_content(self): raise NotImplementedError( "subclasses of BaseEmailBackendTests must provide a get_mailbox_content() " @@ -1349,13 +1374,14 @@ class BaseEmailBackendTests(HeadersCheckMixin): # send_messages() may take a list or an iterator. emails_lists = ([email1, email2], iter((email1, email2))) for emails_list in emails_lists: - num_sent = mail.get_connection().send_messages(emails_list) - self.assertEqual(num_sent, 2) - messages = self.get_mailbox_content() - self.assertEqual(len(messages), 2) - self.assertEqual(messages[0]["To"], "to-1@example.com") - self.assertEqual(messages[1]["To"], "to-2@example.com") - self.flush_mailbox() + with self.subTest(emails_list=repr(emails_list)): + num_sent = mail.get_connection().send_messages(emails_list) + self.assertEqual(num_sent, 2) + messages = self.get_mailbox_content() + self.assertEqual(len(messages), 2) + self.assertEqual(messages[0]["To"], "to-1@example.com") + self.assertEqual(messages[1]["To"], "to-2@example.com") + self.flush_mailbox() def test_send_verbose_name(self): email = EmailMessage( From 5d7001b578272cd698cd6bfe64f980f00615ce62 Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Sat, 24 Aug 2024 13:41:40 -0700 Subject: [PATCH 11/32] Refs #35581 -- Used modern email parser and helpers in mail tests. - Used modern email API (policy.default) for tests that reparse generated messages, and switched to modern accessors where helpful. - Split get_raw_attachments() helper out of get_decoded_attachments(), and used modern iter_attachments() to avoid finding nested attachments in attached message/* emails. - Stopped using legacy parseaddr. --- tests/mail/tests.py | 103 ++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/tests/mail/tests.py b/tests/mail/tests.py index ad6ae38445..9602368d60 100644 --- a/tests/mail/tests.py +++ b/tests/mail/tests.py @@ -4,10 +4,10 @@ import shutil import socket import sys import tempfile -from email import charset, message_from_binary_file, message_from_bytes -from email.header import Header +from email import charset, message_from_binary_file +from email import message_from_bytes as _message_from_bytes +from email import policy from email.mime.text import MIMEText -from email.utils import parseaddr from io import StringIO from pathlib import Path from smtplib import SMTP, SMTPException @@ -41,6 +41,14 @@ except ImportError: HAS_AIOSMTPD = False +def message_from_bytes(s): + """ + email.message_from_bytes() using modern email.policy.default. + Returns a modern email.message.EmailMessage. + """ + return _message_from_bytes(s, policy=policy.default) + + class MailTestsMixin: def assertMessageHasHeaders(self, message, headers): """ @@ -101,24 +109,35 @@ class MailTestsMixin: "First string doesn't end with the second.", ) - def get_decoded_attachments(self, django_message): + def get_raw_attachments(self, django_message): """ - Encode the specified django.core.mail.message.EmailMessage, then decode - it using Python's email.parser module and, for each attachment of the - message, return a list of tuples with (filename, content, mimetype). + Return a list of the raw attachment parts in the MIME message generated + by serializing django_message and reparsing the result. + + This returns only "top-level" attachments. It will not descend into + message/* attached emails to find nested attachments. """ msg_bytes = django_message.message().as_bytes() - email_message = message_from_bytes(msg_bytes) + message = message_from_bytes(msg_bytes) + return list(message.iter_attachments()) - def iter_attachments(): - for i in email_message.walk(): - if i.get_content_disposition() == "attachment": - filename = i.get_filename() - content = i.get_payload(decode=True) - mimetype = i.get_content_type() - yield filename, content, mimetype + def get_decoded_attachments(self, django_message): + """ + Return a list of decoded attachments resulting from serializing + django_message and reparsing the result. - return list(iter_attachments()) + Each attachment is returned as an EmailAttachment named tuple with + fields filename, content, and mimetype. The content will be decoded + to str for mimetype text/*; retained as bytes for other mimetypes. + """ + return [ + EmailAttachment( + attachment.get_filename(), + attachment.get_content(), + attachment.get_content_type(), + ) + for attachment in self.get_raw_attachments(django_message) + ] class MailTests(MailTestsMixin, SimpleTestCase): @@ -684,7 +703,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): self.assertEqual(msg.attachments[0].mimetype, mime_type) attachments = self.get_decoded_attachments(msg) - self.assertEqual(attachments[0], (file_name, file_content.encode(), mime_type)) + self.assertEqual(attachments[0], (file_name, file_content, mime_type)) def test_attachments_constructor(self): file_name = "example.txt" @@ -706,7 +725,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): self.assertEqual(msg.attachments[0].mimetype, mime_type) attachments = self.get_decoded_attachments(msg) - self.assertEqual(attachments[0], (file_name, file_content.encode(), mime_type)) + self.assertEqual(attachments[0], (file_name, file_content, mime_type)) def test_attachments_constructor_from_tuple(self): file_name = "example.txt" @@ -726,7 +745,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): self.assertEqual(msg.attachments[0].mimetype, mime_type) attachments = self.get_decoded_attachments(msg) - self.assertEqual(attachments[0], (file_name, file_content.encode(), mime_type)) + self.assertEqual(attachments[0], (file_name, file_content, mime_type)) def test_attachments_constructor_omit_mimetype(self): """ @@ -767,10 +786,8 @@ class MailTests(MailTestsMixin, SimpleTestCase): msg = EmailMessage(body="Content") # Unicode in file name msg.attach("une pièce jointe.pdf", b"%PDF-1.4.%...", mimetype="application/pdf") - msg_bytes = msg.message().as_bytes() - message = message_from_bytes(msg_bytes) - payload = message.get_payload() - self.assertEqual(payload[1].get_filename(), "une pièce jointe.pdf") + attachment = self.get_decoded_attachments(msg)[0] + self.assertEqual(attachment.filename, "une pièce jointe.pdf") def test_attach_file(self): """ @@ -830,7 +847,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): msg.attach("file.txt", b"file content") filename, content, mimetype = self.get_decoded_attachments(msg)[0] self.assertEqual(filename, "file.txt") - self.assertEqual(content, b"file content") + self.assertEqual(content, "file content") # (decoded) self.assertEqual(mimetype, "text/plain") def test_attach_utf8_text_as_bytes(self): @@ -842,7 +859,7 @@ class MailTests(MailTestsMixin, SimpleTestCase): msg.attach("file.txt", b"\xc3\xa4") # UTF-8 encoded a umlaut. filename, content, mimetype = self.get_decoded_attachments(msg)[0] self.assertEqual(filename, "file.txt") - self.assertEqual(content, b"\xc3\xa4") + self.assertEqual(content, "ä") # (decoded) self.assertEqual(mimetype, "text/plain") def test_attach_non_utf8_text_as_bytes(self): @@ -1341,10 +1358,9 @@ class BaseEmailBackendTests(MailTestsMixin): num_sent = mail.get_connection().send_messages([email]) self.assertEqual(num_sent, 1) message = self.get_the_message() - self.assertEqual(message["subject"], "=?utf-8?q?Ch=C3=A8re_maman?=") - self.assertEqual( - message.get_payload(decode=True).decode(), "Je t'aime très fort" - ) + self.assertEqual(message["subject"], "Chère maman") + self.assertIn(b"Subject: =?utf-8?q?Ch=C3=A8re_maman?=", message.as_bytes()) + self.assertEqual(message.get_content(), "Je t'aime très fort") def test_send_long_lines(self): """ @@ -1390,8 +1406,10 @@ class BaseEmailBackendTests(MailTestsMixin): ) email.send() message = self.get_the_message() - self.assertEqual( - message["from"], "=?utf-8?q?Firstname_S=C3=BCrname?= " + self.assertEqual(message["from"], "Firstname Sürname ") + self.assertIn( + b"From: =?utf-8?q?Firstname_S=C3=BCrname?= ", + message.as_bytes(), ) def test_plaintext_send_mail(self): @@ -1623,7 +1641,10 @@ class LocmemBackendTests(BaseEmailBackendTests, SimpleTestCase): email_backend = "django.core.mail.backends.locmem.EmailBackend" def get_mailbox_content(self): - return [m.message() for m in mail.outbox] + # Reparse as modern messages to work with shared BaseEmailBackendTests. + # (Once EmailMessage.message() uses Python's modern email API, this + # can be changed back to `[m.message() for m in mail.outbox]`.) + return [message_from_bytes(m.message().as_bytes()) for m in mail.outbox] def flush_mailbox(self): mail.outbox = [] @@ -1702,7 +1723,7 @@ class FileBackendTests(BaseEmailBackendTests, SimpleTestCase): self.assertEqual(len(os.listdir(self.tmp_dir)), 1) with open(os.path.join(self.tmp_dir, os.listdir(self.tmp_dir)[0]), "rb") as fp: - message = message_from_binary_file(fp) + message = message_from_binary_file(fp, policy=policy.default) self.assertEqual(message.get_content_type(), "text/plain") self.assertEqual(message.get("subject"), "Subject") self.assertEqual(message.get("from"), "from@example.com") @@ -1791,17 +1812,13 @@ class SMTPHandler: mail_from = envelope.mail_from message = message_from_bytes(data.rstrip()) - message_addr = parseaddr(message.get("from"))[1] - if mail_from != message_addr: - # According to the spec, mail_from does not necessarily match the - # From header - this is the case where the local part isn't - # encoded, so try to correct that. - lp, domain = mail_from.split("@", 1) - lp = Header(lp, "utf-8").encode() - mail_from = "@".join([lp, domain]) + try: + header_from = message["from"].addresses[0].addr_spec + except (KeyError, IndexError): + header_from = None - if mail_from != message_addr: - return f"553 '{mail_from}' != '{message_addr}'" + if mail_from != header_from: + return f"553 '{mail_from}' != '{header_from}'" self.mailbox.append(message) return "250 OK" From ea34de3bd76fa87e84ae968f09105118ed360afb Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Sat, 24 Aug 2024 13:55:57 -0700 Subject: [PATCH 12/32] Refs #35581 -- Added tests for email parameters, attachments, MIME structure, bcc header, encoding and sending. --- tests/mail/tests.py | 499 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 442 insertions(+), 57 deletions(-) diff --git a/tests/mail/tests.py b/tests/mail/tests.py index 9602368d60..12cbc8e874 100644 --- a/tests/mail/tests.py +++ b/tests/mail/tests.py @@ -7,11 +7,15 @@ import tempfile from email import charset, message_from_binary_file from email import message_from_bytes as _message_from_bytes from email import policy +from email.message import EmailMessage as PyEmailMessage +from email.message import Message as PyMessage +from email.mime.image import MIMEImage from email.mime.text import MIMEText from io import StringIO from pathlib import Path from smtplib import SMTP, SMTPException from ssl import SSLError +from textwrap import dedent from unittest import mock, skipUnless from django.core import mail @@ -139,6 +143,26 @@ class MailTestsMixin: for attachment in self.get_raw_attachments(django_message) ] + def get_message_structure(self, message, level=0): + """ + Return a multiline indented string representation + of the message's MIME content-type structure, e.g.: + + multipart/mixed + multipart/alternative + text/plain + text/html + image/jpg + text/calendar + """ + # Adapted from email.iterators._structure(). + indent = " " * (level * 4) + structure = [f"{indent}{message.get_content_type()}\n"] + if message.is_multipart(): + for subpart in message.get_payload(): + structure.append(self.get_message_structure(subpart, level + 1)) + return "".join(structure) + class MailTests(MailTestsMixin, SimpleTestCase): """ @@ -299,6 +323,20 @@ class MailTests(MailTestsMixin, SimpleTestCase): ).message() self.assertEqual(message.get_all("Cc"), ["foo@example.com"]) + def test_bcc_not_in_headers(self): + """ + A bcc address should be in the recipients, + but not in the (visible) message headers. + """ + email = EmailMessage( + to=["to@example.com"], + bcc=["bcc@example.com"], + ) + message = email.message() + self.assertNotIn("Bcc", message) + self.assertNotIn("bcc@example.com", message.as_string()) + self.assertEqual(email.recipients(), ["to@example.com", "bcc@example.com"]) + def test_reply_to(self): email = EmailMessage( "Subject", @@ -875,6 +913,97 @@ class MailTests(MailTestsMixin, SimpleTestCase): self.assertEqual(content, b"\xff") self.assertEqual(mimetype, "application/octet-stream") + def test_attach_mime_image(self): + """ + EmailMessage.attach() docs: "You can pass it + a single argument that is a MIMEBase instance." + """ + # This also verifies complex attachments with extra header fields. + email = EmailMessage() + image = MIMEImage(b"GIF89a...", "gif") + image["Content-Disposition"] = "inline" + image["Content-ID"] = "" + email.attach(image) + + attachments = self.get_raw_attachments(email) + self.assertEqual(len(attachments), 1) + image_att = attachments[0] + self.assertEqual(image_att.get_content_type(), "image/gif") + self.assertEqual(image_att.get_content_disposition(), "inline") + self.assertEqual(image_att["Content-ID"], "") + self.assertEqual(image_att.get_content(), b"GIF89a...") + self.assertIsNone(image_att.get_filename()) + + def test_attach_mime_image_in_constructor(self): + image = MIMEImage(b"\x89PNG...", "png") + image["Content-Disposition"] = "attachment; filename=test.png" + email = EmailMessage(attachments=[image]) + + attachments = self.get_raw_attachments(email) + self.assertEqual(len(attachments), 1) + image_att = attachments[0] + self.assertEqual(image_att.get_content_type(), "image/png") + self.assertEqual(image_att.get_content(), b"\x89PNG...") + self.assertEqual(image_att.get_content_disposition(), "attachment") + self.assertEqual(image_att.get_filename(), "test.png") + + def test_attach_rfc822_message(self): + """ + EmailMessage.attach() docs: "If you specify a mimetype of message/rfc822, + it will also accept django.core.mail.EmailMessage and email.message.Message." + """ + # django.core.mail.EmailMessage + django_email = EmailMessage("child subject", "child body") + # email.message.Message + py_message = PyMessage() + py_message["Subject"] = "child subject" + py_message.set_payload("child body") + # email.message.EmailMessage + py_email_message = PyEmailMessage() + py_email_message["Subject"] = "child subject" + py_email_message.set_content("child body") + + cases = [ + django_email, + py_message, + py_email_message, + # Should also allow message serialized as str or bytes. + py_message.as_string(), + py_message.as_bytes(), + ] + + for child_message in cases: + with self.subTest(child_type=child_message.__class__): + email = EmailMessage("parent message", "parent body") + email.attach(content=child_message, mimetype="message/rfc822") + self.assertEqual(len(email.attachments), 1) + self.assertIsInstance(email.attachments[0], EmailAttachment) + self.assertEqual(email.attachments[0].mimetype, "message/rfc822") + + # Make sure it is serialized correctly: a message/rfc822 attachment + # whose "body" content (payload) is the "encapsulated" (child) message. + attachments = self.get_raw_attachments(email) + self.assertEqual(len(attachments), 1) + rfc822_attachment = attachments[0] + self.assertEqual(rfc822_attachment.get_content_type(), "message/rfc822") + + attached_message = rfc822_attachment.get_content() + self.assertEqual(attached_message["Subject"], "child subject") + self.assertEqual(attached_message.get_content().rstrip(), "child body") + + # Regression for #18967: Per RFC 2046 5.2.1, "No encoding other + # than '7bit', '8bit', or 'binary' is permitted for the body of + # a 'message/rfc822' entity." (Default CTE is "7bit".) + cte = rfc822_attachment.get("Content-Transfer-Encoding", "7bit") + self.assertIn(cte, ("7bit", "8bit", "binary")) + + # Any properly declared CTE is allowed for the attached message itself + # (including quoted-printable or base64). For the plain ASCII content + # in this test, we'd expect 7bit. + child_cte = attached_message.get("Content-Transfer-Encoding", "7bit") + self.assertEqual(child_cte, "7bit") + self.assertEqual(attached_message.get_content_type(), "text/plain") + def test_attach_mimebase_prohibits_other_params(self): email_msg = EmailMessage() txt = MIMEText("content") @@ -1039,63 +1168,6 @@ class MailTests(MailTestsMixin, SimpleTestCase): s = msg.message().as_string() self.assertIn("Content-Transfer-Encoding: 8bit", s) - def test_dont_base64_encode_message_rfc822(self): - # Ticket #18967 - # Shouldn't use base64 encoding for a child EmailMessage attachment. - # Create a child message first - child_msg = EmailMessage( - "Child Subject", - "Some body of child message", - "bounce@example.com", - ["to@example.com"], - headers={"From": "from@example.com"}, - ) - child_s = child_msg.message().as_string() - - # Now create a parent - parent_msg = EmailMessage( - "Parent Subject", - "Some parent body", - "bounce@example.com", - ["to@example.com"], - headers={"From": "from@example.com"}, - ) - - # Attach to parent as a string - parent_msg.attach(content=child_s, mimetype="message/rfc822") - parent_s = parent_msg.message().as_string() - - # The child message header is not base64 encoded - self.assertIn("Child Subject", parent_s) - - # Feature test: try attaching email.Message object directly to the mail. - parent_msg = EmailMessage( - "Parent Subject", - "Some parent body", - "bounce@example.com", - ["to@example.com"], - headers={"From": "from@example.com"}, - ) - parent_msg.attach(content=child_msg.message(), mimetype="message/rfc822") - parent_s = parent_msg.message().as_string() - - # The child message header is not base64 encoded - self.assertIn("Child Subject", parent_s) - - # Feature test: try attaching Django's EmailMessage object directly to the mail. - parent_msg = EmailMessage( - "Parent Subject", - "Some parent body", - "bounce@example.com", - ["to@example.com"], - headers={"From": "from@example.com"}, - ) - parent_msg.attach(content=child_msg, mimetype="message/rfc822") - parent_s = parent_msg.message().as_string() - - # The child message header is not base64 encoded - self.assertIn("Child Subject", parent_s) - def test_custom_utf8_encoding(self): """A UTF-8 charset with a custom body encoding is respected.""" body = "Body with latin characters: àáä." @@ -1239,6 +1311,121 @@ class MailTests(MailTestsMixin, SimpleTestCase): with self.assertRaisesMessage(ValueError, msg): email_msg.attach_alternative("

content

", None) + def test_mime_structure(self): + """ + Check generated messages have the expected MIME parts and nesting. + """ + html_body = EmailAlternative("

HTML

", "text/html") + image = EmailAttachment("image.gif", b"\x89PNG...", "image/png") + rfc822_attachment = EmailAttachment( + None, EmailMessage(body="text"), "message/rfc822" + ) + cases = [ + # name, email (EmailMessage or subclass), expected structure + ( + "single body", + EmailMessage(body="text"), + """ + text/plain + """, + ), + ( + "single body with attachment", + EmailMessage(body="text", attachments=[image]), + """ + multipart/mixed + text/plain + image/png + """, + ), + ( + "alternative bodies", + EmailMultiAlternatives(body="text", alternatives=[html_body]), + """ + multipart/alternative + text/plain + text/html + """, + ), + ( + "alternative bodies with attachments", + EmailMultiAlternatives( + body="text", alternatives=[html_body], attachments=[image] + ), + """ + multipart/mixed + multipart/alternative + text/plain + text/html + image/png + """, + ), + ( + "alternative bodies with rfc822 attachment", + EmailMultiAlternatives( + body="text", + alternatives=[html_body], + attachments=[rfc822_attachment], + ), + """ + multipart/mixed + multipart/alternative + text/plain + text/html + message/rfc822 + text/plain + """, + ), + ( + "attachment only", + EmailMessage(attachments=[image]), + # Avoid empty text/plain body. + """ + multipart/mixed + image/png + """, + ), + ( + "alternative only", + EmailMultiAlternatives(alternatives=[html_body]), + # Avoid empty text/plain body. + """ + multipart/alternative + text/html + """, + ), + ( + "alternative and attachment only", + EmailMultiAlternatives(alternatives=[html_body], attachments=[image]), + """ + multipart/mixed + multipart/alternative + text/html + image/png + """, + ), + ( + "empty EmailMessage", + EmailMessage(), + """ + text/plain + """, + ), + ( + "empty EmailMultiAlternatives", + EmailMultiAlternatives(), + """ + text/plain + """, + ), + ] + for name, email, expected in cases: + expected = dedent(expected).lstrip() + with self.subTest(name=name): + message = email.message() + structure = self.get_message_structure(message) + self.assertEqual(structure, expected) + def test_body_contains(self): email_msg = EmailMultiAlternatives() email_msg.body = "I am content." @@ -1257,6 +1444,114 @@ class MailTests(MailTestsMixin, SimpleTestCase): email_msg.attach_alternative(b"I am a song.", "audio/mpeg") self.assertIs(email_msg.body_contains("I am content"), True) + def test_all_params_optional(self): + """ + EmailMessage class docs: "All parameters are optional" + """ + email = EmailMessage() + self.assertIsInstance(email.message(), PyMessage) # force serialization. + + email = EmailMultiAlternatives() + self.assertIsInstance(email.message(), PyMessage) # force serialization. + + def test_positional_arguments_order(self): + """ + EmailMessage class docs: "… is initialized with the following parameters + (in the given order, if positional arguments are used)." + """ + connection = mail.get_connection() + email = EmailMessage( + # (If you need to insert/remove/reorder any params here, + # that indicates a breaking change to documented behavior.) + "subject", + "body", + "from@example.com", + ["to@example.com"], + ["bcc@example.com"], + connection, + [EmailAttachment("file.txt", "attachment", "text/plain")], + {"X-Header": "custom header"}, + ["cc@example.com"], + ["reply-to@example.com"], + # (New options can be added below here, ideally as keyword-only args.) + ) + + message = email.message() + self.assertEqual(message.get_all("Subject"), ["subject"]) + self.assertEqual(message.get_all("From"), ["from@example.com"]) + self.assertEqual(message.get_all("To"), ["to@example.com"]) + self.assertEqual(message.get_all("X-Header"), ["custom header"]) + self.assertEqual(message.get_all("Cc"), ["cc@example.com"]) + self.assertEqual(message.get_all("Reply-To"), ["reply-to@example.com"]) + self.assertEqual(message.get_payload(0).get_payload(), "body") + self.assertEqual( + self.get_decoded_attachments(email), + [("file.txt", "attachment", "text/plain")], + ) + self.assertEqual( + email.recipients(), ["to@example.com", "cc@example.com", "bcc@example.com"] + ) + self.assertIs(email.get_connection(), connection) + + def test_all_params_can_be_set_before_send(self): + """ + EmailMessage class docs: "All parameters … can be set at any time + prior to calling the send() method." + """ + # This is meant to verify EmailMessage.__init__() doesn't apply any + # special processing that would be missing for properties set later. + original_connection = mail.get_connection(username="original") + new_connection = mail.get_connection(username="new") + email = EmailMessage( + "original subject", + "original body", + "original-from@example.com", + ["original-to@example.com"], + ["original-bcc@example.com"], + original_connection, + [EmailAttachment("original.txt", "original attachment", "text/plain")], + {"X-Header": "original header"}, + ["original-cc@example.com"], + ["original-reply-to@example.com"], + ) + email.subject = "new subject" + email.body = "new body" + email.from_email = "new-from@example.com" + email.to = ["new-to@example.com"] + email.bcc = ["new-bcc@example.com"] + email.connection = new_connection + email.attachments = [ + ("new1.txt", "new attachment 1", "text/plain"), # plain tuple. + EmailAttachment("new2.txt", "new attachment 2", "text/csv"), + MIMEImage(b"GIF89a...", "gif"), + ] + email.extra_headers = {"X-Header": "new header"} + email.cc = ["new-cc@example.com"] + email.reply_to = ["new-reply-to@example.com"] + + message = email.message() + self.assertEqual(message.get_all("Subject"), ["new subject"]) + self.assertEqual(message.get_all("From"), ["new-from@example.com"]) + self.assertEqual(message.get_all("To"), ["new-to@example.com"]) + self.assertEqual(message.get_all("X-Header"), ["new header"]) + self.assertEqual(message.get_all("Cc"), ["new-cc@example.com"]) + self.assertEqual(message.get_all("Reply-To"), ["new-reply-to@example.com"]) + self.assertEqual(message.get_payload(0).get_payload(), "new body") + self.assertEqual( + self.get_decoded_attachments(email), + [ + ("new1.txt", "new attachment 1", "text/plain"), + ("new2.txt", "new attachment 2", "text/csv"), + (None, b"GIF89a...", "image/gif"), + ], + ) + self.assertEqual( + email.recipients(), + ["new-to@example.com", "new-cc@example.com", "new-bcc@example.com"], + ) + self.assertIs(email.get_connection(), new_connection) + self.assertNotIn("original", message.as_string()) + @requires_tz_support class MailTimeZoneTests(MailTestsMixin, SimpleTestCase): @@ -1806,6 +2101,7 @@ class ConsoleBackendTests(BaseEmailBackendTests, SimpleTestCase): class SMTPHandler: def __init__(self, *args, **kwargs): self.mailbox = [] + self.smtp_envelopes = [] async def handle_DATA(self, server, session, envelope): data = envelope.content @@ -1820,10 +2116,17 @@ class SMTPHandler: if mail_from != header_from: return f"553 '{mail_from}' != '{header_from}'" self.mailbox.append(message) + self.smtp_envelopes.append( + { + "mail_from": envelope.mail_from, + "rcpt_tos": envelope.rcpt_tos, + } + ) return "250 OK" def flush_mailbox(self): self.mailbox[:] = [] + self.smtp_envelopes[:] = [] @skipUnless(HAS_AIOSMTPD, "No aiosmtpd library detected.") @@ -1870,6 +2173,9 @@ class SMTPBackendTests(BaseEmailBackendTests, SMTPBackendTestsBase): def get_mailbox_content(self): return self.smtp_handler.mailbox + def get_smtp_envelopes(self): + return self.smtp_handler.smtp_envelopes + @override_settings( EMAIL_HOST_USER="not empty username", EMAIL_HOST_PASSWORD="not empty password", @@ -2094,6 +2400,85 @@ class SMTPBackendTests(BaseEmailBackendTests, SMTPBackendTestsBase): sent = backend.send_messages([email]) self.assertEqual(sent, 0) + def test_avoids_sending_to_invalid_addresses(self): + """ + Verify invalid addresses can't sneak into SMTP commands through + EmailMessage.all_recipients() (which is distinct from message header fields). + """ + backend = smtp.EmailBackend() + backend.connection = mock.Mock() + for email_address in ( + # Invalid address with two @ signs. + "to@other.com@example.com", + # Invalid address without the quotes. + "to@other.com ", + # Other invalid addresses. + "@", + "to@", + "@example.com", + # CR/NL in addr-spec. (SMTP strips display-name.) + '"evil@example.com\r\nto"@example.com', + "to\nevil@example.com", + ): + with self.subTest(email_address=email_address): + # Use bcc (which is only processed by SMTP backend) to ensure + # error is coming from SMTP backend, not EmailMessage.message(). + email = EmailMessage(bcc=[email_address]) + with self.assertRaisesMessage(ValueError, "Invalid address"): + backend.send_messages([email]) + + def test_encodes_idna_in_smtp_commands(self): + """ + SMTP backend must encode non-ASCII domains for the SMTP envelope + (which can be distinct from the email headers). + """ + email = EmailMessage( + from_email="lists@discussão.example.org", + to=["To Example "], + bcc=["monitor@discussão.example.org"], + headers={ + "From": "Gestor de listas ", + "To": "Discussão Django ", + }, + ) + backend = smtp.EmailBackend() + backend.send_messages([email]) + envelope = self.get_smtp_envelopes()[0] + self.assertEqual(envelope["mail_from"], "lists@xn--discusso-xza.example.org") + self.assertEqual( + envelope["rcpt_tos"], + ["to@xn--p8s937b.example.com", "monitor@xn--discusso-xza.example.org"], + ) + + def test_does_not_reencode_idna(self): + """ + SMTP backend should not downgrade IDNA 2008 to IDNA 2003. + + Django does not currently handle IDNA 2008 encoding, but should retain + it for addresses that have been pre-encoded. + """ + # Test all four EmailMessage attrs accessed by the SMTP email backend. + # These are IDNA 2008 encoded domains that would be different + # in IDNA 2003, from https://www.unicode.org/reports/tr46/#Deviations. + email = EmailMessage( + from_email='"βόλος" ', + to=['"faß" '], + cc=['"ශ්‍රී" '], + bcc=['"نامه‌ای." '], + ) + backend = smtp.EmailBackend() + backend.send_messages([email]) + envelope = self.get_smtp_envelopes()[0] + self.assertEqual(envelope["mail_from"], "from@xn--fa-hia.example.com") + self.assertEqual( + envelope["rcpt_tos"], + [ + "to@xn--10cl1a0b660p.example.com", + "cc@xn--nxasmm1c.example.com", + "bcc@xn--mgba3gch31f060k.example.com", + ], + ) + @skipUnless(HAS_AIOSMTPD, "No aiosmtpd library detected.") class SMTPBackendStoppedServerTests(SMTPBackendTestsBase): From b28438f379049e5ee1a89067e9cc14b7d0da07cf Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Sun, 24 Nov 2024 20:32:44 +0000 Subject: [PATCH 13/32] Refs #35842 -- Fixed handling of quotes in JSONField key lookups on Oracle. --- django/db/backends/sqlite3/features.py | 4 ++ django/db/models/fields/json.py | 23 +++++++++-- tests/model_fields/test_jsonfield.py | 53 ++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/django/db/backends/sqlite3/features.py b/django/db/backends/sqlite3/features.py index 2c1aa32506..2a39005f9f 100644 --- a/django/db/backends/sqlite3/features.py +++ b/django/db/backends/sqlite3/features.py @@ -50,6 +50,10 @@ class DatabaseFeatures(BaseDatabaseFeatures): # The django_format_dtdelta() function doesn't properly handle mixed # Date/DateTime fields and timedeltas. "expressions.tests.FTimeDeltaTests.test_mixed_comparisons1", + # SQLite doesn't parse escaped double quotes in the JSON path notation, + # so it cannot match keys that contains double quotes (#35842). + "model_fields.test_jsonfield.TestQuerying." + "test_lookups_special_chars_double_quotes", } create_test_table_with_composite_primary_key = """ CREATE TABLE test_table_composite_pk ( diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py index 1b219e620c..1898b127ce 100644 --- a/django/db/models/fields/json.py +++ b/django/db/models/fields/json.py @@ -230,10 +230,11 @@ class HasKeyLookup(PostgresOperatorLookup): def as_oracle(self, compiler, connection): sql, params = self.as_sql( - compiler, connection, template="JSON_EXISTS(%s, '%%s')" + compiler, connection, template="JSON_EXISTS(%s, q'\uffff%%s\uffff')" ) # Add paths directly into SQL because path expressions cannot be passed - # as bind variables on Oracle. + # as bind variables on Oracle. Use a custom delimiter to prevent the + # JSON path from escaping the SQL literal. See comment in KeyTransform. return sql % tuple(params), [] def as_postgresql(self, compiler, connection): @@ -362,10 +363,24 @@ class KeyTransform(Transform): json_path = compile_json_path(key_transforms) if connection.features.supports_primitives_in_json_field: sql = ( - "COALESCE(JSON_VALUE(%s, '%s'), JSON_QUERY(%s, '%s' DISALLOW SCALARS))" + "COALESCE(" + "JSON_VALUE(%s, q'\uffff%s\uffff')," + "JSON_QUERY(%s, q'\uffff%s\uffff' DISALLOW SCALARS)" + ")" ) else: - sql = "COALESCE(JSON_QUERY(%s, '%s'), JSON_VALUE(%s, '%s'))" + sql = ( + "COALESCE(" + "JSON_QUERY(%s, q'\uffff%s\uffff')," + "JSON_VALUE(%s, q'\uffff%s\uffff')" + ")" + ) + # Add paths directly into SQL because path expressions cannot be passed + # as bind variables on Oracle. Use a custom delimiter to prevent the + # JSON path from escaping the SQL literal. Each key in the JSON path is + # passed through json.dumps() with ensure_ascii=True (the default), + # which converts the delimiter into the escaped \uffff format. This + # ensures that the delimiter is not present in the JSON path. return sql % ((lhs, json_path) * 2), tuple(params) * 2 def as_postgresql(self, compiler, connection): diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py index ff42b1a14c..4c3dc61176 100644 --- a/tests/model_fields/test_jsonfield.py +++ b/tests/model_fields/test_jsonfield.py @@ -808,6 +808,59 @@ class TestQuerying(TestCase): ) self.assertIs(NullableJSONModel.objects.filter(value__c__lt=5).exists(), False) + def test_lookups_special_chars(self): + test_keys = [ + "CONTROL", + "single'", + "dollar$", + "dot.dot", + "with space", + "back\\slash", + "question?mark", + "user@name", + "emo🤡'ji", + "com,ma", + "curly{{{brace}}}s", + "escape\uffff'seq'\uffffue\uffff'nce", + ] + json_value = {key: "some value" for key in test_keys} + obj = NullableJSONModel.objects.create(value=json_value) + obj.refresh_from_db() + self.assertEqual(obj.value, json_value) + + for key in test_keys: + lookups = { + "has_key": Q(value__has_key=key), + "has_keys": Q(value__has_keys=[key, "CONTROL"]), + "has_any_keys": Q(value__has_any_keys=[key, "does_not_exist"]), + "exact": Q(**{f"value__{key}": "some value"}), + } + for lookup, condition in lookups.items(): + results = NullableJSONModel.objects.filter(condition) + with self.subTest(key=key, lookup=lookup): + self.assertSequenceEqual(results, [obj]) + + def test_lookups_special_chars_double_quotes(self): + test_keys = [ + 'double"', + "m\\i@x. m🤡'a,t{{{ch}}}e?d$\"'es\uffff'ca\uffff'pe", + ] + json_value = {key: "some value" for key in test_keys} + obj = NullableJSONModel.objects.create(value=json_value) + obj.refresh_from_db() + self.assertEqual(obj.value, json_value) + self.assertSequenceEqual( + NullableJSONModel.objects.filter(value__has_keys=test_keys), [obj] + ) + for key in test_keys: + with self.subTest(key=key): + results = NullableJSONModel.objects.filter( + Q(value__has_key=key), + Q(value__has_any_keys=[key, "does_not_exist"]), + Q(**{f"value__{key}": "some value"}), + ) + self.assertSequenceEqual(results, [obj]) + def test_lookup_exclude(self): tests = [ (Q(value__a="b"), [self.objs[0]]), From d2f2a6a6d5cf40ec6dffdc9a195f71bedd72acb4 Mon Sep 17 00:00:00 2001 From: Adam Zapletal Date: Mon, 2 Dec 2024 21:10:43 -0600 Subject: [PATCH 14/32] Refs #21286 -- Enabled ImageField test cases in serializer data tests. This aligns ImageField to be tested in the same way as FileField. The commented-out test also exists for FileField and relates to #10244. --- tests/serializers/models/data.py | 4 ++-- tests/serializers/test_data.py | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/serializers/models/data.py b/tests/serializers/models/data.py index b82e8b4b33..0e08a9b125 100644 --- a/tests/serializers/models/data.py +++ b/tests/serializers/models/data.py @@ -62,8 +62,8 @@ class BigIntegerData(models.Model): data = models.BigIntegerField(null=True) -# class ImageData(models.Model): -# data = models.ImageField(null=True) +class ImageData(models.Model): + data = models.ImageField(null=True) class GenericIPAddressData(models.Model): diff --git a/tests/serializers/test_data.py b/tests/serializers/test_data.py index f8763f6e42..c0f5a38900 100644 --- a/tests/serializers/test_data.py +++ b/tests/serializers/test_data.py @@ -47,6 +47,7 @@ from .models import ( GenericData, GenericIPAddressData, GenericIPAddressPKData, + ImageData, InheritAbstractModel, InheritBaseModel, IntegerData, @@ -291,7 +292,9 @@ test_data = [ (data_obj, 81, IntegerData, -123456789), (data_obj, 82, IntegerData, 0), (data_obj, 83, IntegerData, None), - # (XX, ImageData + (data_obj, 86, ImageData, "file:///foo/bar/whiz.png"), + # (data_obj, 87, ImageData, None), + (data_obj, 88, ImageData, ""), (data_obj, 95, GenericIPAddressData, "fe80:1424:2223:6cff:fe8a:2e8a:2151:abcd"), (data_obj, 96, GenericIPAddressData, None), (data_obj, 110, PositiveBigIntegerData, 9223372036854775807), From 58e548db8b74e3d265a2e94816489cd0caeeaf91 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Fri, 27 Sep 2024 14:13:02 +0100 Subject: [PATCH 15/32] Fixed #35952 -- Used class property for available apps check on TransactionTestCase. --- tests/runtests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/runtests.py b/tests/runtests.py index 516da84768..57d4fcea72 100755 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -32,9 +32,11 @@ else: RemovedInDjango60Warning, RemovedInDjango61Warning, ) + from django.utils.functional import classproperty from django.utils.log import DEFAULT_LOGGING from django.utils.version import PY312, PYPY + try: import MySQLdb except ImportError: @@ -307,12 +309,12 @@ def setup_run_tests(verbosity, start_at, start_after, test_labels=None): apps.set_installed_apps(settings.INSTALLED_APPS) # Force declaring available_apps in TransactionTestCase for faster tests. - def no_available_apps(self): + def no_available_apps(cls): raise Exception( "Please define available_apps in TransactionTestCase and its subclasses." ) - TransactionTestCase.available_apps = property(no_available_apps) + TransactionTestCase.available_apps = classproperty(no_available_apps) TestCase.available_apps = None # Set an environment variable that other code may consult to see if From 49ff1042aa66bb25eda87e9a8ef82f3b0ad4eeba Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:06:23 +0100 Subject: [PATCH 16/32] Fixed CVE-2024-53907 -- Mitigated potential DoS in strip_tags(). Thanks to jiangniao for the report, and Shai Berger and Natalia Bidart for the reviews. --- django/utils/html.py | 10 ++++++++-- docs/releases/4.2.17.txt | 16 ++++++++++++++++ docs/releases/5.0.10.txt | 16 ++++++++++++++++ docs/releases/5.1.4.txt | 16 ++++++++++++++++ tests/utils_tests/test_html.py | 7 +++++++ 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/django/utils/html.py b/django/utils/html.py index b34af183d0..bc336d88a6 100644 --- a/django/utils/html.py +++ b/django/utils/html.py @@ -8,6 +8,7 @@ from collections.abc import Mapping from html.parser import HTMLParser from urllib.parse import parse_qsl, quote, unquote, urlencode, urlsplit, urlunsplit +from django.core.exceptions import SuspiciousOperation from django.utils.deprecation import RemovedInDjango60Warning from django.utils.encoding import punycode from django.utils.functional import Promise, cached_property, keep_lazy, keep_lazy_text @@ -40,6 +41,7 @@ VOID_ELEMENTS = frozenset( ) MAX_URL_LENGTH = 2048 +MAX_STRIP_TAGS_DEPTH = 50 @keep_lazy(SafeString) @@ -211,15 +213,19 @@ def _strip_once(value): @keep_lazy_text def strip_tags(value): """Return the given HTML with all tags stripped.""" - # Note: in typical case this loop executes _strip_once once. Loop condition - # is redundant, but helps to reduce number of executions of _strip_once. value = str(value) + # Note: in typical case this loop executes _strip_once twice (the second + # execution does not remove any more tags). + strip_tags_depth = 0 while "<" in value and ">" in value: + if strip_tags_depth >= MAX_STRIP_TAGS_DEPTH: + raise SuspiciousOperation new_value = _strip_once(value) if value.count("<") == new_value.count("<"): # _strip_once wasn't able to detect more tags. break value = new_value + strip_tags_depth += 1 return value diff --git a/docs/releases/4.2.17.txt b/docs/releases/4.2.17.txt index 5139d7034d..9db07f6da7 100644 --- a/docs/releases/4.2.17.txt +++ b/docs/releases/4.2.17.txt @@ -6,3 +6,19 @@ Django 4.2.17 release notes Django 4.2.17 fixes one security issue with severity "high" and one security issue with severity "moderate" in 4.2.16. + +CVE-2024-53907: Denial-of-service possibility in ``strip_tags()`` +================================================================= + +:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate +certain inputs containing large sequences of nested incomplete HTML entities. +The ``strip_tags()`` method is used to implement the corresponding +:tfilter:`striptags` template filter, which was thus also vulnerable. + +``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser`` +before raising a :exc:`.SuspiciousOperation` exception. + +Remember that absolutely NO guarantee is provided about the results of +``strip_tags()`` being HTML safe. So NEVER mark safe the result of a +``strip_tags()`` call without escaping it first, for example with +:func:`django.utils.html.escape`. diff --git a/docs/releases/5.0.10.txt b/docs/releases/5.0.10.txt index b06c376038..54569516a5 100644 --- a/docs/releases/5.0.10.txt +++ b/docs/releases/5.0.10.txt @@ -6,3 +6,19 @@ Django 5.0.10 release notes Django 5.0.10 fixes one security issue with severity "high" and one security issue with severity "moderate" in 5.0.9. + +CVE-2024-53907: Denial-of-service possibility in ``strip_tags()`` +================================================================= + +:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate +certain inputs containing large sequences of nested incomplete HTML entities. +The ``strip_tags()`` method is used to implement the corresponding +:tfilter:`striptags` template filter, which was thus also vulnerable. + +``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser`` +before raising a :exc:`.SuspiciousOperation` exception. + +Remember that absolutely NO guarantee is provided about the results of +``strip_tags()`` being HTML safe. So NEVER mark safe the result of a +``strip_tags()`` call without escaping it first, for example with +:func:`django.utils.html.escape`. diff --git a/docs/releases/5.1.4.txt b/docs/releases/5.1.4.txt index 44950ac76a..389952efa6 100644 --- a/docs/releases/5.1.4.txt +++ b/docs/releases/5.1.4.txt @@ -7,6 +7,22 @@ Django 5.1.4 release notes Django 5.1.4 fixes one security issue with severity "high", one security issue with severity "moderate", and several bugs in 5.1.3. +CVE-2024-53907: Denial-of-service possibility in ``strip_tags()`` +================================================================= + +:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate +certain inputs containing large sequences of nested incomplete HTML entities. +The ``strip_tags()`` method is used to implement the corresponding +:tfilter:`striptags` template filter, which was thus also vulnerable. + +``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser`` +before raising a :exc:`.SuspiciousOperation` exception. + +Remember that absolutely NO guarantee is provided about the results of +``strip_tags()`` being HTML safe. So NEVER mark safe the result of a +``strip_tags()`` call without escaping it first, for example with +:func:`django.utils.html.escape`. + Bugfixes ======== diff --git a/tests/utils_tests/test_html.py b/tests/utils_tests/test_html.py index b47919df99..dc3768e6fa 100644 --- a/tests/utils_tests/test_html.py +++ b/tests/utils_tests/test_html.py @@ -1,6 +1,7 @@ import os from datetime import datetime +from django.core.exceptions import SuspiciousOperation from django.core.serializers.json import DjangoJSONEncoder from django.test import SimpleTestCase from django.utils.deprecation import RemovedInDjango60Warning @@ -145,12 +146,18 @@ class TestUtilsHtml(SimpleTestCase): ("&h", "alert()h"), (">br>br>br>X", "XX"), + ("<" * 50 + "a>" * 50, ""), ) for value, output in items: with self.subTest(value=value, output=output): self.check_output(strip_tags, value, output) self.check_output(strip_tags, lazystr(value), output) + def test_strip_tags_suspicious_operation(self): + value = "<" * 51 + "a>" * 51, "" + with self.assertRaises(SuspiciousOperation): + strip_tags(value) + def test_strip_tags_files(self): # Test with more lengthy content (also catching performance regressions) for filename in ("strip_tags1.html", "strip_tags2.txt"): From 8f8dc5a1fca7d076e749f307f6573af3512e7e99 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 8 Nov 2024 21:27:31 -0500 Subject: [PATCH 17/32] Fixed CVE-2024-53908 -- Prevented SQL injections in direct HasKeyLookup usage on Oracle. Thanks Seokchan Yoon for the report, and Mariusz Felisiak and Sarah Boyce for the reviews. --- django/db/models/fields/json.py | 56 ++++++++++++++++++---------- docs/releases/4.2.17.txt | 9 +++++ docs/releases/5.0.10.txt | 9 +++++ docs/releases/5.1.4.txt | 9 +++++ tests/model_fields/test_jsonfield.py | 9 +++++ 5 files changed, 73 insertions(+), 19 deletions(-) diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py index 1898b127ce..8d743c436a 100644 --- a/django/db/models/fields/json.py +++ b/django/db/models/fields/json.py @@ -193,20 +193,18 @@ class HasKeyLookup(PostgresOperatorLookup): # Compile the final key without interpreting ints as array elements. return ".%s" % json.dumps(key_transform) - def as_sql(self, compiler, connection, template=None): + def _as_sql_parts(self, compiler, connection): # Process JSON path from the left-hand side. if isinstance(self.lhs, KeyTransform): - lhs, lhs_params, lhs_key_transforms = self.lhs.preprocess_lhs( + lhs_sql, lhs_params, lhs_key_transforms = self.lhs.preprocess_lhs( compiler, connection ) lhs_json_path = compile_json_path(lhs_key_transforms) else: - lhs, lhs_params = self.process_lhs(compiler, connection) + lhs_sql, lhs_params = self.process_lhs(compiler, connection) lhs_json_path = "$" - sql = template % lhs # Process JSON path from the right-hand side. rhs = self.rhs - rhs_params = [] if not isinstance(rhs, (list, tuple)): rhs = [rhs] for key in rhs: @@ -217,25 +215,45 @@ class HasKeyLookup(PostgresOperatorLookup): *rhs_key_transforms, final_key = rhs_key_transforms rhs_json_path = compile_json_path(rhs_key_transforms, include_root=False) rhs_json_path += self.compile_json_path_final_key(final_key) - rhs_params.append(lhs_json_path + rhs_json_path) + yield lhs_sql, lhs_params, lhs_json_path + rhs_json_path + + def _combine_sql_parts(self, parts): # Add condition for each key. if self.logical_operator: - sql = "(%s)" % self.logical_operator.join([sql] * len(rhs_params)) - return sql, tuple(lhs_params) + tuple(rhs_params) + return "(%s)" % self.logical_operator.join(parts) + return "".join(parts) + + def as_sql(self, compiler, connection, template=None): + sql_parts = [] + params = [] + for lhs_sql, lhs_params, rhs_json_path in self._as_sql_parts( + compiler, connection + ): + sql_parts.append(template % (lhs_sql, "%s")) + params.extend(lhs_params + [rhs_json_path]) + return self._combine_sql_parts(sql_parts), tuple(params) def as_mysql(self, compiler, connection): return self.as_sql( - compiler, connection, template="JSON_CONTAINS_PATH(%s, 'one', %%s)" + compiler, connection, template="JSON_CONTAINS_PATH(%s, 'one', %s)" ) def as_oracle(self, compiler, connection): - sql, params = self.as_sql( - compiler, connection, template="JSON_EXISTS(%s, q'\uffff%%s\uffff')" - ) - # Add paths directly into SQL because path expressions cannot be passed - # as bind variables on Oracle. Use a custom delimiter to prevent the - # JSON path from escaping the SQL literal. See comment in KeyTransform. - return sql % tuple(params), [] + # Use a custom delimiter to prevent the JSON path from escaping the SQL + # literal. See comment in KeyTransform. + template = "JSON_EXISTS(%s, q'\uffff%s\uffff')" + sql_parts = [] + params = [] + for lhs_sql, lhs_params, rhs_json_path in self._as_sql_parts( + compiler, connection + ): + # Add right-hand-side directly into SQL because it cannot be passed + # as bind variables to JSON_EXISTS. It might result in invalid + # queries but it is assumed that it cannot be evaded because the + # path is JSON serialized. + sql_parts.append(template % (lhs_sql, rhs_json_path)) + params.extend(lhs_params) + return self._combine_sql_parts(sql_parts), tuple(params) def as_postgresql(self, compiler, connection): if isinstance(self.rhs, KeyTransform): @@ -247,7 +265,7 @@ class HasKeyLookup(PostgresOperatorLookup): def as_sqlite(self, compiler, connection): return self.as_sql( - compiler, connection, template="JSON_TYPE(%s, %%s) IS NOT NULL" + compiler, connection, template="JSON_TYPE(%s, %s) IS NOT NULL" ) @@ -470,9 +488,9 @@ class KeyTransformIsNull(lookups.IsNull): return "(NOT %s OR %s IS NULL)" % (sql, lhs), tuple(params) + tuple(lhs_params) def as_sqlite(self, compiler, connection): - template = "JSON_TYPE(%s, %%s) IS NULL" + template = "JSON_TYPE(%s, %s) IS NULL" if not self.rhs: - template = "JSON_TYPE(%s, %%s) IS NOT NULL" + template = "JSON_TYPE(%s, %s) IS NOT NULL" return HasKeyOrArrayIndex(self.lhs.lhs, self.lhs.key_name).as_sql( compiler, connection, diff --git a/docs/releases/4.2.17.txt b/docs/releases/4.2.17.txt index 9db07f6da7..9a6aee3db6 100644 --- a/docs/releases/4.2.17.txt +++ b/docs/releases/4.2.17.txt @@ -22,3 +22,12 @@ Remember that absolutely NO guarantee is provided about the results of ``strip_tags()`` being HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without escaping it first, for example with :func:`django.utils.html.escape`. + +CVE-2024-53908: Potential SQL injection via ``HasKey(lhs, rhs)`` on Oracle +========================================================================== + +Direct usage of the ``django.db.models.fields.json.HasKey`` lookup on Oracle +was subject to SQL injection if untrusted data was used as a ``lhs`` value. + +Applications that use the :lookup:`has_key ` lookup through +the ``__`` syntax are unaffected. diff --git a/docs/releases/5.0.10.txt b/docs/releases/5.0.10.txt index 54569516a5..ae1fbf99e4 100644 --- a/docs/releases/5.0.10.txt +++ b/docs/releases/5.0.10.txt @@ -22,3 +22,12 @@ Remember that absolutely NO guarantee is provided about the results of ``strip_tags()`` being HTML safe. So NEVER mark safe the result of a ``strip_tags()`` call without escaping it first, for example with :func:`django.utils.html.escape`. + +CVE-2024-53908: Potential SQL injection via ``HasKey(lhs, rhs)`` on Oracle +========================================================================== + +Direct usage of the ``django.db.models.fields.json.HasKey`` lookup on Oracle +was subject to SQL injection if untrusted data was used as a ``lhs`` value. + +Applications that use the :lookup:`has_key ` lookup through +the ``__`` syntax are unaffected. diff --git a/docs/releases/5.1.4.txt b/docs/releases/5.1.4.txt index 389952efa6..e768725688 100644 --- a/docs/releases/5.1.4.txt +++ b/docs/releases/5.1.4.txt @@ -23,6 +23,15 @@ Remember that absolutely NO guarantee is provided about the results of ``strip_tags()`` call without escaping it first, for example with :func:`django.utils.html.escape`. +CVE-2024-53908: Potential SQL injection via ``HasKey(lhs, rhs)`` on Oracle +========================================================================== + +Direct usage of the ``django.db.models.fields.json.HasKey`` lookup on Oracle +was subject to SQL injection if untrusted data was used as a ``lhs`` value. + +Applications that use the :lookup:`has_key ` lookup through +the ``__`` syntax are unaffected. + Bugfixes ======== diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py index 4c3dc61176..09f95ce69f 100644 --- a/tests/model_fields/test_jsonfield.py +++ b/tests/model_fields/test_jsonfield.py @@ -29,6 +29,7 @@ from django.db.models import ( from django.db.models.expressions import RawSQL from django.db.models.fields.json import ( KT, + HasKey, KeyTextTransform, KeyTransform, KeyTransformFactory, @@ -582,6 +583,14 @@ class TestQuerying(TestCase): [expected], ) + def test_has_key_literal_lookup(self): + self.assertSequenceEqual( + NullableJSONModel.objects.filter( + HasKey(Value({"foo": "bar"}, JSONField()), "foo") + ).order_by("id"), + self.objs, + ) + def test_has_key_list(self): obj = NullableJSONModel.objects.create(value=[{"a": 1}, {"b": "x"}]) tests = [ From 828afd782f8bc019401075bd51fad039cc5ceff0 Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:23:59 +0100 Subject: [PATCH 18/32] Added stub release notes for 5.1.5. --- docs/releases/5.1.5.txt | 12 ++++++++++++ docs/releases/index.txt | 1 + 2 files changed, 13 insertions(+) create mode 100644 docs/releases/5.1.5.txt diff --git a/docs/releases/5.1.5.txt b/docs/releases/5.1.5.txt new file mode 100644 index 0000000000..af0dab545c --- /dev/null +++ b/docs/releases/5.1.5.txt @@ -0,0 +1,12 @@ +========================== +Django 5.1.5 release notes +========================== + +*Expected January 7, 2025* + +Django 5.1.5 fixes several bugs in 5.1.4. + +Bugfixes +======== + +* ... diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 536e5917ab..e16ae37f5e 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -32,6 +32,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 5.1.5 5.1.4 5.1.3 5.1.2 From 595cb4a7aeb1ba1770d10d601ce9a2b4e487c46e Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:30:03 +0100 Subject: [PATCH 19/32] Added CVE-2024-53907 and CVE-2024-53908 to security archive. --- docs/releases/security.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/releases/security.txt b/docs/releases/security.txt index b83be59dbb..ed06c7367b 100644 --- a/docs/releases/security.txt +++ b/docs/releases/security.txt @@ -36,6 +36,28 @@ Issues under Django's security process All security issues have been handled under versions of Django's security process. These are listed below. +December 4, 2024 - :cve:`2024-53907` +------------------------------------ + +Potential denial-of-service in django.utils.html.strip_tags(). +`Full description +`__ + +* Django 5.1 :commit:`(patch) ` +* Django 5.0 :commit:`(patch) ` +* Django 4.2 :commit:`(patch) <790eb058b0716c536a2f2e8d1c6d5079d776c22b>` + +December 4, 2024 - :cve:`2024-53908` +------------------------------------ + +Potential SQL injection in HasKey(lhs, rhs) on Oracle. +`Full description +`__ + +* Django 5.1 :commit:`(patch) <6943d61818e63e77b65d8b1ae65941e8f04bd87b>` +* Django 5.0 :commit:`(patch) ` +* Django 4.2 :commit:`(patch) <7376bcbf508883282ffcc0f0fac5cf0ed2d6cbc5>` + September 3, 2024 - :cve:`2024-45231` ------------------------------------- From eb665e076ca3417eb0ac654aed9e9c1853c5af84 Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:51:46 +0100 Subject: [PATCH 20/32] Cleaned up CVE-2024-53907 and CVE-2024-53908 security archive descriptions. --- docs/releases/security.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/releases/security.txt b/docs/releases/security.txt index ed06c7367b..02ea77b54d 100644 --- a/docs/releases/security.txt +++ b/docs/releases/security.txt @@ -39,7 +39,7 @@ process. These are listed below. December 4, 2024 - :cve:`2024-53907` ------------------------------------ -Potential denial-of-service in django.utils.html.strip_tags(). +Potential denial-of-service in ``django.utils.html.strip_tags()``. `Full description `__ @@ -50,7 +50,7 @@ Potential denial-of-service in django.utils.html.strip_tags(). December 4, 2024 - :cve:`2024-53908` ------------------------------------ -Potential SQL injection in HasKey(lhs, rhs) on Oracle. +Potential SQL injection in ``HasKey(lhs, rhs)`` on Oracle. `Full description `__ From 3d508ececbd3b7b652aedc66b0d3d2c7baa4795a Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 5 Dec 2024 08:08:09 +0100 Subject: [PATCH 21/32] Refs #21286 -- Fixed serializers tests if Pillow isn't installed. --- tests/serializers/models/data.py | 13 +++++++++---- tests/serializers/test_data.py | 12 +++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/serializers/models/data.py b/tests/serializers/models/data.py index 0e08a9b125..77625c05e9 100644 --- a/tests/serializers/models/data.py +++ b/tests/serializers/models/data.py @@ -13,6 +13,15 @@ from django.db import models from .base import BaseModel +try: + from PIL import Image # NOQA +except ImportError: + ImageData = None +else: + + class ImageData(models.Model): + data = models.ImageField(null=True) + class BinaryData(models.Model): data = models.BinaryField(null=True) @@ -62,10 +71,6 @@ class BigIntegerData(models.Model): data = models.BigIntegerField(null=True) -class ImageData(models.Model): - data = models.ImageField(null=True) - - class GenericIPAddressData(models.Model): data = models.GenericIPAddressField(null=True) diff --git a/tests/serializers/test_data.py b/tests/serializers/test_data.py index c0f5a38900..bd81ce0c14 100644 --- a/tests/serializers/test_data.py +++ b/tests/serializers/test_data.py @@ -292,9 +292,6 @@ test_data = [ (data_obj, 81, IntegerData, -123456789), (data_obj, 82, IntegerData, 0), (data_obj, 83, IntegerData, None), - (data_obj, 86, ImageData, "file:///foo/bar/whiz.png"), - # (data_obj, 87, ImageData, None), - (data_obj, 88, ImageData, ""), (data_obj, 95, GenericIPAddressData, "fe80:1424:2223:6cff:fe8a:2e8a:2151:abcd"), (data_obj, 96, GenericIPAddressData, None), (data_obj, 110, PositiveBigIntegerData, 9223372036854775807), @@ -408,6 +405,15 @@ The end.""", (data_obj, 1005, LengthModel, 1), ] +if ImageData is not None: + test_data.extend( + [ + (data_obj, 86, ImageData, "file:///foo/bar/whiz.png"), + # (data_obj, 87, ImageData, None), + (data_obj, 88, ImageData, ""), + ] + ) + class SerializerDataTests(TestCase): pass From 28f81a10190de9aa00925156c0005f6c787afeb3 Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Thu, 5 Dec 2024 09:12:29 +0100 Subject: [PATCH 22/32] Refs #373 -- Fixed CompositePrimaryKey tests if yaml isn't installed. --- tests/composite_pk/tests.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py index 1a0a327baf..25e5f2fdd5 100644 --- a/tests/composite_pk/tests.py +++ b/tests/composite_pk/tests.py @@ -1,7 +1,13 @@ import json +import unittest from uuid import UUID -import yaml +try: + import yaml # NOQA + + HAS_YAML = True +except ImportError: + HAS_YAML = False from django import forms from django.core import serializers @@ -252,6 +258,7 @@ class CompositePKFixturesTests(TestCase): }, ) + @unittest.skipUnless(HAS_YAML, "No yaml library detected") def test_serialize_user_yaml(self): users = User.objects.filter(pk=(2, 3)) result = serializers.serialize("yaml", users) From edd74c3417fa3a0b29295012ff31dbe44843303c Mon Sep 17 00:00:00 2001 From: David Smith Date: Sat, 18 Nov 2023 20:36:45 +0000 Subject: [PATCH 23/32] Refs #32819 -- Added id to ErrorList class and template. --- django/forms/forms.py | 5 +- .../jinja2/django/forms/errors/list/ul.html | 2 +- .../django/forms/errors/list/ul.html | 2 +- django/forms/utils.py | 3 +- docs/ref/forms/api.txt | 16 +++- docs/releases/5.2.txt | 4 + .../forms_tests/tests/test_error_messages.py | 9 +- tests/forms_tests/tests/test_forms.py | 96 ++++++++++--------- tests/forms_tests/tests/test_i18n.py | 2 +- tests/model_forms/tests.py | 11 ++- 10 files changed, 91 insertions(+), 59 deletions(-) diff --git a/django/forms/forms.py b/django/forms/forms.py index 452f554e1e..549a3adf6f 100644 --- a/django/forms/forms.py +++ b/django/forms/forms.py @@ -298,7 +298,10 @@ class BaseForm(RenderableFormMixin): error_class="nonfield", renderer=self.renderer ) else: - self._errors[field] = self.error_class(renderer=self.renderer) + self._errors[field] = self.error_class( + renderer=self.renderer, + field_id=self[field].auto_id, + ) self._errors[field].extend(error_list) if field in self.cleaned_data: del self.cleaned_data[field] diff --git a/django/forms/jinja2/django/forms/errors/list/ul.html b/django/forms/jinja2/django/forms/errors/list/ul.html index 752f7c2c8b..59528efccc 100644 --- a/django/forms/jinja2/django/forms/errors/list/ul.html +++ b/django/forms/jinja2/django/forms/errors/list/ul.html @@ -1 +1 @@ -{% if errors %}
    {% for error in errors %}
  • {{ error }}
    • {% endfor %}
{% endif %} +{% if errors %}
    {% for error in errors %}
  • {{ error }}
    • {% endfor %}
{% endif %} diff --git a/django/forms/templates/django/forms/errors/list/ul.html b/django/forms/templates/django/forms/errors/list/ul.html index 57b34ccb88..c28ce8af67 100644 --- a/django/forms/templates/django/forms/errors/list/ul.html +++ b/django/forms/templates/django/forms/errors/list/ul.html @@ -1 +1 @@ -{% if errors %}
    {% for error in errors %}
  • {{ error }}
    • {% endfor %}
{% endif %} \ No newline at end of file +{% if errors %}
    {% for error in errors %}
  • {{ error }}
    • {% endfor %}
{% endif %} \ No newline at end of file diff --git a/django/forms/utils.py b/django/forms/utils.py index f4fbf3e241..d24711d1a0 100644 --- a/django/forms/utils.py +++ b/django/forms/utils.py @@ -147,7 +147,7 @@ class ErrorList(UserList, list, RenderableErrorMixin): template_name_text = "django/forms/errors/list/text.txt" template_name_ul = "django/forms/errors/list/ul.html" - def __init__(self, initlist=None, error_class=None, renderer=None): + def __init__(self, initlist=None, error_class=None, renderer=None, field_id=None): super().__init__(initlist) if error_class is None: @@ -155,6 +155,7 @@ class ErrorList(UserList, list, RenderableErrorMixin): else: self.error_class = "errorlist {}".format(error_class) self.renderer = renderer or get_default_renderer() + self.field_id = field_id def as_data(self): return ValidationError(self.data).error_list diff --git a/docs/ref/forms/api.txt b/docs/ref/forms/api.txt index c6c83dcdfb..4875a1ab72 100644 --- a/docs/ref/forms/api.txt +++ b/docs/ref/forms/api.txt @@ -1025,13 +1025,17 @@ method you're using: Customizing the error list format --------------------------------- -.. class:: ErrorList(initlist=None, error_class=None, renderer=None) +.. class:: ErrorList(initlist=None, error_class=None, renderer=None, field_id=None) By default, forms use ``django.forms.utils.ErrorList`` to format validation errors. ``ErrorList`` is a list like object where ``initlist`` is the list of errors. In addition this class has the following attributes and methods. + .. versionchanged:: 5.2 + + The ``field_id`` argument was added. + .. attribute:: error_class The CSS classes to be used when rendering the error list. Any provided @@ -1043,6 +1047,16 @@ Customizing the error list format Defaults to ``None`` which means to use the default renderer specified by the :setting:`FORM_RENDERER` setting. + .. attribute:: field_id + + .. versionadded:: 5.2 + + An ``id`` for the field for which the errors relate. This allows an + HTML ``id`` attribute to be added in the error template and is useful + to associate the errors with the field. The default template uses the + format ``id="{{ field_id }}_error"`` and a value is provided by + :meth:`.Form.add_error` using the field's :attr:`~.BoundField.auto_id`. + .. attribute:: template_name The name of the template used when calling ``__str__`` or diff --git a/docs/releases/5.2.txt b/docs/releases/5.2.txt index 4b05fd3279..b6c44c43f8 100644 --- a/docs/releases/5.2.txt +++ b/docs/releases/5.2.txt @@ -249,6 +249,10 @@ Forms * The new :class:`~django.forms.TelInput` form widget is for entering telephone numbers and renders as ````. +* The new ``field_id`` argument for :class:`~django.forms.ErrorList` allows an + HTML ``id`` attribute to be added in the error template. See + :attr:`.ErrorList.field_id` for details. + Generic Views ~~~~~~~~~~~~~ diff --git a/tests/forms_tests/tests/test_error_messages.py b/tests/forms_tests/tests/test_error_messages.py index e44c6d6668..f4f5700107 100644 --- a/tests/forms_tests/tests/test_error_messages.py +++ b/tests/forms_tests/tests/test_error_messages.py @@ -249,7 +249,8 @@ class FormsErrorMessagesTestCase(SimpleTestCase, AssertFormErrorsMixin): form1 = TestForm({"first_name": "John"}) self.assertHTMLEqual( str(form1["last_name"].errors), - '
  • This field is required.
', + '
  • ' + "This field is required.
", ) self.assertHTMLEqual( str(form1.errors["__all__"]), @@ -280,7 +281,7 @@ class FormsErrorMessagesTestCase(SimpleTestCase, AssertFormErrorsMixin): f = SomeForm({"field": "