From 9e27650e6a228ad41787724b71daafaae0bc71ef Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Mon, 3 Feb 2020 09:58:56 +0100 Subject: [PATCH 01/28] [3.0.x] Post-release version bump. --- django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/__init__.py b/django/__init__.py index 69d61206b7ec..0dc4cc19e8eb 100644 --- a/django/__init__.py +++ b/django/__init__.py @@ -1,6 +1,6 @@ from django.utils.version import get_version -VERSION = (3, 0, 3, 'final', 0) +VERSION = (3, 0, 4, 'alpha', 0) __version__ = get_version(VERSION) From 5d18016a077afa3e47874762d5c7f087ac3407a9 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Mon, 3 Feb 2020 10:11:34 +0100 Subject: [PATCH 02/28] [3.0.x] Added CVE-2020-7471 to security archive. Backport of d8b2ccbbb846328a0938347dc70cb2e603164d9a from master --- docs/releases/security.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/releases/security.txt b/docs/releases/security.txt index 6e0c29223d14..76991cb23a2b 100644 --- a/docs/releases/security.txt +++ b/docs/releases/security.txt @@ -1055,3 +1055,16 @@ Versions affected * Django 3.0 :commit:`(patch) <302a4ff1e8b1c798aab97673909c7a3dfda42c26>` * Django 2.2 :commit:`(patch) <4d334bea06cac63dc1272abcec545b85136cca0e>` * Django 1.11 :commit:`(patch) ` + +February 3, 2020 - :cve:`2020-7471` +----------------------------------- + +Potential SQL injection via ``StringAgg(delimiter)``. `Full description +`__ + +Versions affected +~~~~~~~~~~~~~~~~~ + +* Django 3.0 :commit:`(patch) <505826b469b16ab36693360da9e11fd13213421b>` +* Django 2.2 :commit:`(patch) ` +* Django 1.11 :commit:`(patch) <001b0634cd309e372edb6d7d95d083d02b8e37bd>` From 8aaa7a2960c7d82c7d2c5e1598a2fea65ed6f092 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Mon, 3 Feb 2020 10:23:54 +0100 Subject: [PATCH 03/28] [3.0.x] Added stub release notes for 3.0.4. Backport of 273918c25b203d32a7922bc7c3610e4a089fe931 from master --- docs/releases/3.0.4.txt | 7 +++++++ docs/releases/index.txt | 1 + 2 files changed, 8 insertions(+) create mode 100644 docs/releases/3.0.4.txt diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt new file mode 100644 index 000000000000..e5198c71cad8 --- /dev/null +++ b/docs/releases/3.0.4.txt @@ -0,0 +1,7 @@ +========================== +Django 3.0.4 release notes +========================== + +*Expected March 2, 2020* + +Django 3.0.4 fixes several bugs in 3.0.3. diff --git a/docs/releases/index.txt b/docs/releases/index.txt index d648751611f7..37d9f8a3ff6a 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -25,6 +25,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 3.0.4 3.0.3 3.0.2 3.0.1 From 8ed5ac4eb42b036144072e94d88873c1eaef09cb Mon Sep 17 00:00:00 2001 From: Vibhu Agarwal Date: Sat, 1 Feb 2020 22:28:18 +0530 Subject: [PATCH 04/28] [3.0.x] Fixed #31222 -- Fixed typo in docs/internals/contributing/bugs-and-features.txt. Backport of 1a09708dcb2f60ad5ca0a75b8de9619356f74ff6 from master --- AUTHORS | 1 + docs/internals/contributing/bugs-and-features.txt | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index c2e281208027..85ae591fc9cc 100644 --- a/AUTHORS +++ b/AUTHORS @@ -895,6 +895,7 @@ answer newbie questions, and generally made Django that much better: valtron Vasiliy Stavenko Vasil Vangelovski + Vibhu Agarwal Victor Andrée viestards.lists@gmail.com Viktor Danyliuk diff --git a/docs/internals/contributing/bugs-and-features.txt b/docs/internals/contributing/bugs-and-features.txt index 89c11c3a326b..a85f68932687 100644 --- a/docs/internals/contributing/bugs-and-features.txt +++ b/docs/internals/contributing/bugs-and-features.txt @@ -91,9 +91,8 @@ part of that. Here are some tips on how to make a request most effectively: * Make sure the feature actually requires changes in Django's core. If your idea can be developed as an independent application or module — for instance, you want to support another database engine — we'll probably - suggest that you to develop it independently. Then, if your project - gathers sufficient community support, we may consider it for inclusion in - Django. + suggest that you develop it independently. Then, if your project gathers + sufficient community support, we may consider it for inclusion in Django. * First request the feature on the |django-developers| list, not in the ticket tracker. It'll get read more closely if it's on the mailing list. From 0bf1330fe41e9a8ce04806c764fd5c90e1bafe5d Mon Sep 17 00:00:00 2001 From: Vibhu Agarwal Date: Tue, 4 Feb 2020 00:46:06 +0530 Subject: [PATCH 05/28] [3.0.x] Fixed #31226 -- Fixed typo in docs/internals/contributing/writing-code/submitting-patches.txt. Backport of 6f9ecc23f676e7a6f25d1a6cf830fe638a1eb589 from master --- docs/internals/contributing/writing-code/submitting-patches.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/contributing/writing-code/submitting-patches.txt b/docs/internals/contributing/writing-code/submitting-patches.txt index 9f8acd9cd01b..3f5c4618fd58 100644 --- a/docs/internals/contributing/writing-code/submitting-patches.txt +++ b/docs/internals/contributing/writing-code/submitting-patches.txt @@ -158,7 +158,7 @@ the ticket for opinions. Deprecating a feature ===================== -There are a couple reasons that code in Django might be deprecated: +There are a couple of reasons that code in Django might be deprecated: * If a feature has been improved or modified in a backwards-incompatible way, the old feature or behavior will be deprecated. From 9b3634866583da8f1bf1710ba509d151d0fcbe11 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Wed, 5 Feb 2020 11:46:14 +0000 Subject: [PATCH 06/28] [3.0.x] Improved grammar in 3.0 release notes for SECURE_CONTENT_TYPE_NOSNIFF change. Backport of de1924e0e7499535f05298c46d9983fd1640d4b4 from master --- docs/releases/3.0.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/releases/3.0.txt b/docs/releases/3.0.txt index f133c58654b0..50c98ade58d0 100644 --- a/docs/releases/3.0.txt +++ b/docs/releases/3.0.txt @@ -330,9 +330,8 @@ Security uses frames of itself, you will need to explicitly set ``X_FRAME_OPTIONS = 'SAMEORIGIN'`` for them to continue working. -* :setting:`SECURE_CONTENT_TYPE_NOSNIFF` setting now defaults to ``True``. With - the enabled :setting:`SECURE_CONTENT_TYPE_NOSNIFF`, the - :class:`~django.middleware.security.SecurityMiddleware` sets the +* :setting:`SECURE_CONTENT_TYPE_NOSNIFF` now defaults to ``True``. With this + enabled, :class:`~django.middleware.security.SecurityMiddleware` sets the :ref:`x-content-type-options` header on all responses that do not already have it. From 5aaec00606a69ef4c4548501bb878a401c6ee109 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Mon, 10 Feb 2020 08:13:31 +0100 Subject: [PATCH 07/28] [3.0.x] Added "Bugfixes" section to release notes for 3.0.4. Backport of 932bd794b29c24536b6277b61175bd11ca9be234 from master --- docs/releases/3.0.4.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index e5198c71cad8..57675fa9a8fb 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -5,3 +5,8 @@ Django 3.0.4 release notes *Expected March 2, 2020* Django 3.0.4 fixes several bugs in 3.0.3. + +Bugfixes +======== + +* ... From dc0dfd1dacfb26a3e7bcbe7aaf7658282cae3b54 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Mon, 10 Feb 2020 08:18:58 +0100 Subject: [PATCH 08/28] [3.0.x] Added stub release notes for 2.2.11. Backport of 7e8339748cc199b4a13513891d9ac4f1e4794588 from master --- docs/releases/2.2.11.txt | 12 ++++++++++++ docs/releases/index.txt | 1 + 2 files changed, 13 insertions(+) create mode 100644 docs/releases/2.2.11.txt diff --git a/docs/releases/2.2.11.txt b/docs/releases/2.2.11.txt new file mode 100644 index 000000000000..5aaa5deab0c6 --- /dev/null +++ b/docs/releases/2.2.11.txt @@ -0,0 +1,12 @@ +=========================== +Django 2.2.11 release notes +=========================== + +*Expected March 2, 2020* + +Django 2.2.11 fixes a data loss bug in 2.2.10. + +Bugfixes +======== + +* ... diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 37d9f8a3ff6a..8598dfeb2926 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -36,6 +36,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 2.2.11 2.2.10 2.2.9 2.2.8 From 7540b7eb318b552a4324adccf5d64d39fad2ee12 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Thu, 6 Feb 2020 17:59:20 -0800 Subject: [PATCH 09/28] [3.0.x] Fixed #31253 -- Fixed data loss possibility when using caching from async code. Case missed in a415ce70bef6d91036b00dd2c8544aed7aeeaaed. Backport of e3f6e18513224c8ad081e5a19da641f49b0b43da from master --- django/core/cache/__init__.py | 4 ++-- docs/releases/3.0.4.txt | 3 ++- tests/async/tests.py | 13 +++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/django/core/cache/__init__.py b/django/core/cache/__init__.py index a6b956fdf2f9..735b83e94f5b 100644 --- a/django/core/cache/__init__.py +++ b/django/core/cache/__init__.py @@ -12,7 +12,7 @@ See docs/topics/cache.txt for information on the public API. """ -from threading import local +from asgiref.local import Local from django.conf import settings from django.core import signals @@ -61,7 +61,7 @@ class CacheHandler: Ensure only one instance of each alias exists per thread. """ def __init__(self): - self._caches = local() + self._caches = Local() def __getitem__(self, alias): try: diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index 57675fa9a8fb..dac985394788 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -9,4 +9,5 @@ Django 3.0.4 fixes several bugs in 3.0.3. Bugfixes ======== -* ... +* Fixed a data loss possibility when using caching from async code + (:ticket:`31253`). diff --git a/tests/async/tests.py b/tests/async/tests.py index f42e54907559..86ed504c5721 100644 --- a/tests/async/tests.py +++ b/tests/async/tests.py @@ -4,6 +4,7 @@ from asgiref.sync import async_to_sync +from django.core.cache import DEFAULT_CACHE_ALIAS, caches from django.core.exceptions import SynchronousOnlyOperation from django.test import SimpleTestCase from django.utils.asyncio import async_unsafe @@ -11,6 +12,18 @@ from .models import SimpleModel +@skipIf(sys.platform == 'win32' and (3, 8, 0) < sys.version_info < (3, 8, 1), 'https://bugs.python.org/issue38563') +class CacheTest(SimpleTestCase): + def test_caches_local(self): + @async_to_sync + async def async_cache(): + return caches[DEFAULT_CACHE_ALIAS] + + cache_1 = async_cache() + cache_2 = async_cache() + self.assertIs(cache_1, cache_2) + + @skipIf(sys.platform == 'win32' and (3, 8, 0) < sys.version_info < (3, 8, 1), 'https://bugs.python.org/issue38563') class DatabaseConnectionTest(SimpleTestCase): """A database connection cannot be used in an async context.""" From 22c25bea54dcad23c76d26bb0bfb8d56eeedeef2 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Fri, 7 Feb 2020 11:30:26 +0100 Subject: [PATCH 10/28] [3.0.x] Reverted "Fixed #30565 -- Closed HttpResponse when wsgi.file_wrapper closes file-like object." This reverts commit cce47ff65a4dd3786c049ec14ee889e128ca7de9. Backport of 549445519ce90cc5c1e3f981853cc0c67725f3ed from master --- django/http/response.py | 28 ---------------- docs/ref/request-response.txt | 3 +- tests/responses/test_fileresponse.py | 49 ---------------------------- 3 files changed, 1 insertion(+), 79 deletions(-) diff --git a/django/http/response.py b/django/http/response.py index c39519c4bb3b..04efbd6bef34 100644 --- a/django/http/response.py +++ b/django/http/response.py @@ -241,9 +241,6 @@ def make_bytes(self, value): # The WSGI server must call this method upon completion of the request. # See http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html - # When wsgi.file_wrapper is used, the WSGI server instead calls close() - # on the file-like object. Django ensures this method is called in this - # case by replacing self.file_to_stream.close() with a wrapped version. def close(self): for closable in self._closable_objects: try: @@ -400,39 +397,14 @@ def __init__(self, *args, as_attachment=False, filename='', **kwargs): self.filename = filename super().__init__(*args, **kwargs) - def _wrap_file_to_stream_close(self, filelike): - """ - Wrap the file-like close() with a version that calls - FileResponse.close(). - """ - closing = False - filelike_close = getattr(filelike, 'close', lambda: None) - - def file_wrapper_close(): - nonlocal closing - # Prevent an infinite loop since FileResponse.close() tries to - # close the objects in self._closable_objects. - if closing: - return - closing = True - try: - filelike_close() - finally: - self.close() - - filelike.close = file_wrapper_close - def _set_streaming_content(self, value): if not hasattr(value, 'read'): self.file_to_stream = None return super()._set_streaming_content(value) self.file_to_stream = filelike = value - # Add to closable objects before wrapping close(), since the filelike - # might not have close(). if hasattr(filelike, 'close'): self._closable_objects.append(filelike) - self._wrap_file_to_stream_close(filelike) value = iter(lambda: filelike.read(self.block_size), b'') self.set_headers(filelike) super()._set_streaming_content(value) diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index 45d4bab4188c..6afb1803892d 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -867,8 +867,7 @@ Methods .. method:: HttpResponse.close() This method is called at the end of the request directly by the WSGI - server, or when the WSGI server closes the file-like object, if - `wsgi.file_wrapper`_ is used for the request. + server. .. method:: HttpResponse.write(content) diff --git a/tests/responses/test_fileresponse.py b/tests/responses/test_fileresponse.py index a6bcb584660f..e77df4513a20 100644 --- a/tests/responses/test_fileresponse.py +++ b/tests/responses/test_fileresponse.py @@ -80,52 +80,3 @@ def test_unicode_attachment(self): response['Content-Disposition'], "attachment; filename*=utf-8''%E7%A5%9D%E6%82%A8%E5%B9%B3%E5%AE%89.odt" ) - - def test_file_to_stream_closes_response(self): - # Closing file_to_stream calls FileResponse.close(), even when - # file-like object doesn't have a close() method. - class FileLike: - def read(self): - pass - - class FileLikeWithClose(FileLike): - def __init__(self): - self.closed = False - - def close(self): - self.closed = True - - for filelike_cls in (FileLike, FileLikeWithClose): - with self.subTest(filelike_cls=filelike_cls.__name__): - filelike = filelike_cls() - response = FileResponse(filelike) - self.assertFalse(response.closed) - # Object with close() is added to the list of closable. - if hasattr(filelike, 'closed'): - self.assertEqual(response._closable_objects, [filelike]) - else: - self.assertEqual(response._closable_objects, []) - file_to_stream = response.file_to_stream - file_to_stream.close() - if hasattr(filelike, 'closed'): - self.assertTrue(filelike.closed) - self.assertTrue(response.closed) - - def test_file_to_stream_closes_response_on_error(self): - # Closing file_to_stream calls FileResponse.close(), even when - # closing file-like raises exceptions. - class FileLikeWithRaisingClose: - def read(self): - pass - - def close(self): - raise RuntimeError() - - filelike = FileLikeWithRaisingClose() - response = FileResponse(filelike) - self.assertFalse(response.closed) - self.assertEqual(response._closable_objects, [filelike]) - file_to_stream = response.file_to_stream - with self.assertRaises(RuntimeError): - file_to_stream.close() - self.assertTrue(response.closed) From 4e8d6a1bafbbebc0720c5d1ed751521356364fe9 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Fri, 7 Feb 2020 12:55:59 +0100 Subject: [PATCH 11/28] [3.0.x] Fixed #31240 -- Properly closed FileResponse when wsgi.file_wrapper is used. Thanks to Oskar Persson for the report. Backport of 41a3b3d18647b258331104520e76f977406c590d from master --- django/core/handlers/base.py | 2 +- django/core/handlers/wsgi.py | 4 ++++ django/http/response.py | 12 +++++++----- docs/releases/3.0.4.txt | 3 +++ tests/builtin_server/tests.py | 26 ++++++++++++++++++++++++++ tests/builtin_server/urls.py | 7 +++++++ tests/builtin_server/views.py | 15 +++++++++++++++ 7 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 tests/builtin_server/urls.py create mode 100644 tests/builtin_server/views.py diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index 2304e7761d71..a7b8e61f60dc 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -73,7 +73,7 @@ def get_response(self, request): # Setup default url resolver for this thread set_urlconf(settings.ROOT_URLCONF) response = self._middleware_chain(request) - response._closable_objects.append(request) + response._resource_closers.append(request.close) if response.status_code >= 400: log_response( '%s: %s', response.reason_phrase, request.path, diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index cb740e5c5058..4dff1a299b84 100644 --- a/django/core/handlers/wsgi.py +++ b/django/core/handlers/wsgi.py @@ -141,6 +141,10 @@ def __call__(self, environ, start_response): ] start_response(status, response_headers) if getattr(response, 'file_to_stream', None) is not None and environ.get('wsgi.file_wrapper'): + # If `wsgi.file_wrapper` is used the WSGI server does not call + # .close on the response, but on the file wrapper. Patch it to use + # response.close instead which takes care of closing all files. + response.file_to_stream.close = response.close response = environ['wsgi.file_wrapper'](response.file_to_stream, response.block_size) return response diff --git a/django/http/response.py b/django/http/response.py index 04efbd6bef34..c33feb97c43b 100644 --- a/django/http/response.py +++ b/django/http/response.py @@ -40,7 +40,7 @@ def __init__(self, content_type=None, status=None, reason=None, charset=None): # the header (required for working with legacy systems) and the header # value. Both the name of the header and its value are ASCII strings. self._headers = {} - self._closable_objects = [] + self._resource_closers = [] # This parameter is set by the handler. It's necessary to preserve the # historical behavior of request_finished. self._handler_class = None @@ -242,11 +242,13 @@ def make_bytes(self, value): # The WSGI server must call this method upon completion of the request. # See http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html def close(self): - for closable in self._closable_objects: + for closer in self._resource_closers: try: - closable.close() + closer() except Exception: pass + # Free resources that were still referenced. + self._resource_closers.clear() self.closed = True signals.request_finished.send(sender=self._handler_class) @@ -377,7 +379,7 @@ def _set_streaming_content(self, value): # Ensure we can never iterate on "value" more than once. self._iterator = iter(value) if hasattr(value, 'close'): - self._closable_objects.append(value) + self._resource_closers.append(value.close) def __iter__(self): return self.streaming_content @@ -404,7 +406,7 @@ def _set_streaming_content(self, value): self.file_to_stream = filelike = value if hasattr(filelike, 'close'): - self._closable_objects.append(filelike) + self._resource_closers.append(filelike.close) value = iter(lambda: filelike.read(self.block_size), b'') self.set_headers(filelike) super()._set_streaming_content(value) diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index dac985394788..c24d8f7a6a85 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -11,3 +11,6 @@ Bugfixes * Fixed a data loss possibility when using caching from async code (:ticket:`31253`). + +* Fixed a regression in Django 3.0 that caused a file response using a + temporary file to be closed incorrectly (:ticket:`31240`). diff --git a/tests/builtin_server/tests.py b/tests/builtin_server/tests.py index 879a93bc08cb..71e261ddcc24 100644 --- a/tests/builtin_server/tests.py +++ b/tests/builtin_server/tests.py @@ -4,6 +4,11 @@ from unittest import TestCase from wsgiref import simple_server +from django.core.servers.basehttp import get_internal_wsgi_application +from django.test import RequestFactory, override_settings + +from .views import FILE_RESPONSE_HOLDER + # If data is too large, socket will choke, so write chunks no larger than 32MB # at a time. The rationale behind the 32MB can be found in #5596#comment:4. MAX_SOCKET_CHUNK_SIZE = 32 * 1024 * 1024 # 32 MB @@ -89,6 +94,27 @@ def test_file_wrapper_no_sendfile(self): self.assertEqual(handler.stdout.getvalue().splitlines()[-1], b'Hello World!') self.assertEqual(handler.stderr.getvalue(), b'') + @override_settings(ROOT_URLCONF='builtin_server.urls') + def test_file_response_closing(self): + """ + View returning a FileResponse properly closes the file and http + response when file_wrapper is used. + """ + env = RequestFactory().get('/fileresponse/').environ + handler = FileWrapperHandler(None, BytesIO(), BytesIO(), env) + handler.run(get_internal_wsgi_application()) + # Sendfile is used only when file_wrapper has been used. + self.assertTrue(handler._used_sendfile) + # Fetch the original response object. + self.assertIn('response', FILE_RESPONSE_HOLDER) + response = FILE_RESPONSE_HOLDER['response'] + # The response and file buffers are closed. + self.assertIs(response.closed, True) + buf1, buf2 = FILE_RESPONSE_HOLDER['buffers'] + self.assertIs(buf1.closed, True) + self.assertIs(buf2.closed, True) + FILE_RESPONSE_HOLDER.clear() + class WriteChunkCounterHandler(ServerHandler): """ diff --git a/tests/builtin_server/urls.py b/tests/builtin_server/urls.py new file mode 100644 index 000000000000..c26366f1e681 --- /dev/null +++ b/tests/builtin_server/urls.py @@ -0,0 +1,7 @@ +from django.urls import path + +from . import views + +urlpatterns = [ + path('fileresponse/', views.file_response), +] diff --git a/tests/builtin_server/views.py b/tests/builtin_server/views.py new file mode 100644 index 000000000000..be7c7e94ab8f --- /dev/null +++ b/tests/builtin_server/views.py @@ -0,0 +1,15 @@ +from io import BytesIO + +from django.http import FileResponse + +FILE_RESPONSE_HOLDER = {} + + +def file_response(request): + f1 = BytesIO(b"test1") + f2 = BytesIO(b"test2") + response = FileResponse(f1) + response._resource_closers.append(f2.close) + FILE_RESPONSE_HOLDER['response'] = response + FILE_RESPONSE_HOLDER['buffers'] = (f1, f2) + return response From 8faaaf4e719531f2fd7f390a8af33ef2458f5427 Mon Sep 17 00:00:00 2001 From: Abhijeet Viswa Date: Sat, 8 Feb 2020 10:22:09 +0530 Subject: [PATCH 12/28] [3.0.x] Fixed #31246 -- Fixed locking models in QuerySet.select_for_update(of=()) for related fields and parent link fields with multi-table inheritance. Partly regression in 0107e3d1058f653f66032f7fd3a0bd61e96bf782. Backport of 1712a76b9dfda1ef220395e62ea87079da8c9f6c from master --- AUTHORS | 1 + django/db/models/sql/compiler.py | 37 ++++++++++++++---------- docs/releases/2.2.11.txt | 6 +++- docs/releases/3.0.4.txt | 6 ++++ tests/select_for_update/models.py | 6 +++- tests/select_for_update/tests.py | 48 +++++++++++++++++++++++++++---- 6 files changed, 81 insertions(+), 23 deletions(-) diff --git a/AUTHORS b/AUTHORS index 85ae591fc9cc..2431fe61e14c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -9,6 +9,7 @@ answer newbie questions, and generally made Django that much better: Aaron Swartz Aaron T. Myers Abeer Upadhyay + Abhijeet Viswa Abhinav Patil Abhishek Gautam Adam Allred diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index be2d590d8469..18365f1d7520 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -972,19 +972,34 @@ def get_select_for_update_of_arguments(self): the query. """ def _get_parent_klass_info(klass_info): - return ( - { + for parent_model, parent_link in klass_info['model']._meta.parents.items(): + parent_list = parent_model._meta.get_parent_list() + yield { 'model': parent_model, 'field': parent_link, 'reverse': False, 'select_fields': [ select_index for select_index in klass_info['select_fields'] - if self.select[select_index][0].target.model == parent_model + # Selected columns from a model or its parents. + if ( + self.select[select_index][0].target.model == parent_model or + self.select[select_index][0].target.model in parent_list + ) ], } - for parent_model, parent_link in klass_info['model']._meta.parents.items() - ) + + def _get_first_selected_col_from_model(klass_info): + """ + Find the first selected column from a model. If it doesn't exist, + don't lock a model. + + select_fields is filled recursively, so it also contains fields + from the parent models. + """ + for select_index in klass_info['select_fields']: + if self.select[select_index][0].target.model == klass_info['model']: + return self.select[select_index][0] def _get_field_choices(): """Yield all allowed field paths in breadth-first search order.""" @@ -1013,14 +1028,7 @@ def _get_field_choices(): for name in self.query.select_for_update_of: klass_info = self.klass_info if name == 'self': - # Find the first selected column from a base model. If it - # doesn't exist, don't lock a base model. - for select_index in klass_info['select_fields']: - if self.select[select_index][0].target.model == klass_info['model']: - col = self.select[select_index][0] - break - else: - col = None + col = _get_first_selected_col_from_model(klass_info) else: for part in name.split(LOOKUP_SEP): klass_infos = ( @@ -1040,8 +1048,7 @@ def _get_field_choices(): if klass_info is None: invalid_names.append(name) continue - select_index = klass_info['select_fields'][0] - col = self.select[select_index][0] + col = _get_first_selected_col_from_model(klass_info) if col is not None: if self.connection.features.select_for_update_of_column: result.append(self.compile(col)[0]) diff --git a/docs/releases/2.2.11.txt b/docs/releases/2.2.11.txt index 5aaa5deab0c6..b14d961ac36a 100644 --- a/docs/releases/2.2.11.txt +++ b/docs/releases/2.2.11.txt @@ -9,4 +9,8 @@ Django 2.2.11 fixes a data loss bug in 2.2.10. Bugfixes ======== -* ... +* Fixed a data loss possibility in the + :meth:`~django.db.models.query.QuerySet.select_for_update`. When using + related fields or parent link fields with :ref:`multi-table-inheritance` in + the ``of`` argument, the corresponding models were not locked + (:ticket:`31246`). diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index c24d8f7a6a85..7be1ed15cee6 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -14,3 +14,9 @@ Bugfixes * Fixed a regression in Django 3.0 that caused a file response using a temporary file to be closed incorrectly (:ticket:`31240`). + +* Fixed a data loss possibility in the + :meth:`~django.db.models.query.QuerySet.select_for_update`. When using + related fields or parent link fields with :ref:`multi-table-inheritance` in + the ``of`` argument, the corresponding models were not locked + (:ticket:`31246`). diff --git a/tests/select_for_update/models.py b/tests/select_for_update/models.py index c84f9ad6b29a..305e8cac490b 100644 --- a/tests/select_for_update/models.py +++ b/tests/select_for_update/models.py @@ -1,7 +1,11 @@ from django.db import models -class Country(models.Model): +class Entity(models.Model): + pass + + +class Country(Entity): name = models.CharField(max_length=30) diff --git a/tests/select_for_update/tests.py b/tests/select_for_update/tests.py index 0bb21972d10b..3622a95c11a7 100644 --- a/tests/select_for_update/tests.py +++ b/tests/select_for_update/tests.py @@ -113,7 +113,10 @@ def test_for_update_sql_generated_of(self): )) features = connections['default'].features if features.select_for_update_of_column: - expected = ['select_for_update_person"."id', 'select_for_update_country"."id'] + expected = [ + 'select_for_update_person"."id', + 'select_for_update_country"."entity_ptr_id', + ] else: expected = ['select_for_update_person', 'select_for_update_country'] expected = [connection.ops.quote_name(value) for value in expected] @@ -137,13 +140,29 @@ def test_for_update_sql_model_inheritance_ptr_generated_of(self): if connection.features.select_for_update_of_column: expected = [ 'select_for_update_eucountry"."country_ptr_id', - 'select_for_update_country"."id', + 'select_for_update_country"."entity_ptr_id', ] else: expected = ['select_for_update_eucountry', 'select_for_update_country'] expected = [connection.ops.quote_name(value) for value in expected] self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected)) + @skipUnlessDBFeature('has_select_for_update_of') + def test_for_update_sql_related_model_inheritance_generated_of(self): + with transaction.atomic(), CaptureQueriesContext(connection) as ctx: + list(EUCity.objects.select_related('country').select_for_update( + of=('self', 'country'), + )) + if connection.features.select_for_update_of_column: + expected = [ + 'select_for_update_eucity"."id', + 'select_for_update_eucountry"."country_ptr_id', + ] + else: + expected = ['select_for_update_eucity', 'select_for_update_eucountry'] + expected = [connection.ops.quote_name(value) for value in expected] + self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected)) + @skipUnlessDBFeature('has_select_for_update_of') def test_for_update_sql_model_inheritance_nested_ptr_generated_of(self): with transaction.atomic(), CaptureQueriesContext(connection) as ctx: @@ -153,13 +172,29 @@ def test_for_update_sql_model_inheritance_nested_ptr_generated_of(self): if connection.features.select_for_update_of_column: expected = [ 'select_for_update_eucity"."id', - 'select_for_update_country"."id', + 'select_for_update_country"."entity_ptr_id', ] else: expected = ['select_for_update_eucity', 'select_for_update_country'] expected = [connection.ops.quote_name(value) for value in expected] self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected)) + @skipUnlessDBFeature('has_select_for_update_of') + def test_for_update_sql_multilevel_model_inheritance_ptr_generated_of(self): + with transaction.atomic(), CaptureQueriesContext(connection) as ctx: + list(EUCountry.objects.select_for_update( + of=('country_ptr', 'country_ptr__entity_ptr'), + )) + if connection.features.select_for_update_of_column: + expected = [ + 'select_for_update_country"."entity_ptr_id', + 'select_for_update_entity"."id', + ] + else: + expected = ['select_for_update_country', 'select_for_update_entity'] + expected = [connection.ops.quote_name(value) for value in expected] + self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected)) + @skipUnlessDBFeature('has_select_for_update_of') def test_for_update_of_followed_by_values(self): with transaction.atomic(): @@ -264,7 +299,8 @@ def test_unrelated_of_argument_raises_error(self): msg = ( 'Invalid field name(s) given in select_for_update(of=(...)): %s. ' 'Only relational fields followed in the query are allowed. ' - 'Choices are: self, born, born__country.' + 'Choices are: self, born, born__country, ' + 'born__country__entity_ptr.' ) invalid_of = [ ('nonexistent',), @@ -307,13 +343,13 @@ def test_model_inheritance_of_argument_raises_error_ptr_in_choices(self): ) with self.assertRaisesMessage( FieldError, - msg % 'country, country__country_ptr', + msg % 'country, country__country_ptr, country__country_ptr__entity_ptr', ): with transaction.atomic(): EUCity.objects.select_related( 'country', ).select_for_update(of=('name',)).get() - with self.assertRaisesMessage(FieldError, msg % 'country_ptr'): + with self.assertRaisesMessage(FieldError, msg % 'country_ptr, country_ptr__entity_ptr'): with transaction.atomic(): EUCountry.objects.select_for_update(of=('name',)).get() From bbec01c1520059aa518c30781bbd64a9469c2c30 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Thu, 6 Feb 2020 22:55:17 +0900 Subject: [PATCH 13/28] [3.0.x] Fixed #31241 -- Clarified porting translations of the Django docs to docs.djangoproject.com. Backport of c25a8c77d72b6d3a1942a699796224f4a2caf543 from master --- docs/internals/contributing/localizing.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/internals/contributing/localizing.txt b/docs/internals/contributing/localizing.txt index f0d89f900194..8942ea742a89 100644 --- a/docs/internals/contributing/localizing.txt +++ b/docs/internals/contributing/localizing.txt @@ -82,3 +82,9 @@ huge undertaking to complete entirely (you have been warned!). We use the same `Transifex tool `_. The translations will appear at ``https://docs.djangoproject.com//`` when at least the ``docs/intro/*`` files are fully translated in your language. + +Once translations are published, updated versions from Transifex will be +irregularly ported to the `django/django-docs-translations +`_ repository and to the +documentation website. Only translations for the latest stable Django release +are updated. From bcf58e3e705cf0feaf29768a3539b241d8f12ec8 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Thu, 13 Feb 2020 16:01:43 +0100 Subject: [PATCH 14/28] [3.0.x] Fixed #31270 -- Doc'd RedirectView.get_redirect_url() arguments. Backport of 2ab97af3528352e8f3ea4aa725b863822442d5ed from master --- docs/ref/class-based-views/base.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/ref/class-based-views/base.txt b/docs/ref/class-based-views/base.txt index 85e3c8bd4b76..e5a3ca19636e 100644 --- a/docs/ref/class-based-views/base.txt +++ b/docs/ref/class-based-views/base.txt @@ -265,6 +265,10 @@ MRO is an acronym for Method Resolution Order. Constructs the target URL for redirection. + The ``args`` and ``kwargs`` arguments are positional and/or keyword + arguments :ref:`captured from the URL pattern + `, respectively. + The default implementation uses :attr:`url` as a starting string and performs expansion of ``%`` named parameters in that string using the named groups captured in the URL. From 2448b3182c75cce5d22dd80be0fb5cba6104c364 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Tue, 18 Feb 2020 11:45:12 +0100 Subject: [PATCH 15/28] [3.0.x] Fixed #31271 -- Preserved ordering when unifying query parameters on Oracle. This caused misplacing parameters in logged SQL queries. Regression in 79065b55a70cd220820a260a1c54851b7be0615a. Thanks Hans Aarne Liblik for the report. Backport of 2a038521c4eabdc5f6d5026d3dd6d22868e329cd from master --- django/db/backends/oracle/base.py | 5 ++++- docs/releases/3.0.4.txt | 3 +++ tests/backends/tests.py | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index ef33d9fad7cd..86650a894ea5 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -497,7 +497,10 @@ def _fix_for_params(self, query, params, unify_by_values=False): # params_dict = {0.75: ':arg0', 2: ':arg1', 'sth': ':arg2'} # args = [':arg0', ':arg1', ':arg0', ':arg2', ':arg0'] # params = {':arg0': 0.75, ':arg1': 2, ':arg2': 'sth'} - params_dict = {param: ':arg%d' % i for i, param in enumerate(set(params))} + params_dict = { + param: ':arg%d' % i + for i, param in enumerate(dict.fromkeys(params)) + } args = [params_dict[param] for param in params] params = {value: key for key, value in params_dict.items()} query = query % tuple(args) diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index 7be1ed15cee6..216bc29b0e26 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -20,3 +20,6 @@ Bugfixes related fields or parent link fields with :ref:`multi-table-inheritance` in the ``of`` argument, the corresponding models were not locked (:ticket:`31246`). + +* Fixed a regression in Django 3.0 that caused misplacing parameters in logged + SQL queries on Oracle (:ticket:`31271`). diff --git a/tests/backends/tests.py b/tests/backends/tests.py index da20d94442e3..2a5019919226 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -79,6 +79,10 @@ def test_last_executed_query(self): for qs in ( Article.objects.filter(pk=1), Article.objects.filter(pk__in=(1, 2), reporter__pk=3), + Article.objects.filter( + pk=1, + reporter__pk=9, + ).exclude(reporter__pk__in=[2, 1]), ): sql, params = qs.query.sql_with_params() cursor = qs.query.get_compiler(DEFAULT_DB_ALIAS).execute_sql(CURSOR) From 611d1c114835310a0cd963e20f3e9782c34db847 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 18 Feb 2020 10:48:19 +0100 Subject: [PATCH 16/28] [3.0.x] Fixed #31282 -- Corrected RelatedManager docs for using add/remove/set with PKs. Backport of 3bbf9a489afc689eff2f4a0b84af196aa1ef51e7 from master --- docs/ref/models/relations.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/ref/models/relations.txt b/docs/ref/models/relations.txt index 67a6a419408a..21d65c033dfa 100644 --- a/docs/ref/models/relations.txt +++ b/docs/ref/models/relations.txt @@ -66,8 +66,8 @@ Related objects reference Using ``add()`` on a relation that already exists won't duplicate the relation, but it will still trigger signals. - ``add()`` also accepts the field the relation points to as an argument. - The above example can be rewritten as ``b.entry_set.add(234)``. + For many-to-many relationships ``add()`` accepts either model instances + or field values, normally primary keys, as the ``*objs`` argument. Use the ``through_defaults`` argument to specify values for the new :ref:`intermediate model ` instance(s), if @@ -131,9 +131,9 @@ Related objects reference :data:`~django.db.models.signals.m2m_changed` signal if you wish to execute custom code when a relationship is deleted. - Similarly to :meth:`add()`, ``remove()`` also accepts the field the - relation points to as an argument. The above example can be rewritten - as ``b.entry_set.remove(234)``. + For many-to-many relationships ``remove()`` accepts either model + instances or field values, normally primary keys, as the ``*objs`` + argument. For :class:`~django.db.models.ForeignKey` objects, this method only exists if ``null=True``. If the related field can't be set to ``None`` @@ -195,9 +195,9 @@ Related objects reference race conditions. For instance, new objects may be added to the database in between the call to ``clear()`` and the call to ``add()``. - Similarly to :meth:`add()`, ``set()`` also accepts the field the - relation points to as an argument. The above example can be rewritten - as ``e.related_set.set([obj1.pk, obj2.pk, obj3.pk])``. + For many-to-many relationships ``set()`` accepts a list of either model + instances or field values, normally primary keys, as the ``objs`` + argument. Use the ``through_defaults`` argument to specify values for the new :ref:`intermediate model ` instance(s), if From a73489f162003472708887fac9b98987af6464fd Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Mon, 17 Feb 2020 16:24:34 +0100 Subject: [PATCH 17/28] [3.0.x] Fixed #30040 -- Used default permission name in docs examples to avoid confusion. Backport of b7795d7673f51daf288ac80616ef69b05918ca6b from master --- docs/topics/auth/default.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 9f38fb70e459..e593f37e490b 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -727,13 +727,13 @@ The ``permission_required`` decorator from django.contrib.auth.decorators import permission_required - @permission_required('polls.can_vote') + @permission_required('polls.add_choice') def my_view(request): ... Just like the :meth:`~django.contrib.auth.models.User.has_perm` method, permission names take the form ``"."`` - (i.e. ``polls.can_vote`` for a permission on a model in the ``polls`` + (i.e. ``polls.add_choice`` for a permission on a model in the ``polls`` application). The decorator may also take an iterable of permissions, in which case the @@ -744,7 +744,7 @@ The ``permission_required`` decorator from django.contrib.auth.decorators import permission_required - @permission_required('polls.can_vote', login_url='/loginpage/') + @permission_required('polls.add_choice', login_url='/loginpage/') def my_view(request): ... @@ -763,7 +763,7 @@ The ``permission_required`` decorator from django.contrib.auth.decorators import login_required, permission_required @login_required - @permission_required('polls.can_vote', raise_exception=True) + @permission_required('polls.add_choice', raise_exception=True) def my_view(request): ... @@ -789,9 +789,9 @@ To apply permission checks to :doc:`class-based views from django.contrib.auth.mixins import PermissionRequiredMixin class MyView(PermissionRequiredMixin, View): - permission_required = 'polls.can_vote' + permission_required = 'polls.add_choice' # Or multiple of permissions: - permission_required = ('polls.can_open', 'polls.can_edit') + permission_required = ('polls.view_choice', 'polls.change_choice') You can set any of the parameters of :class:`~django.contrib.auth.mixins.AccessMixin` to customize the handling @@ -1621,9 +1621,9 @@ the logged-in user has any permissions in the ``foo`` app:: Evaluating a two-level-attribute lookup as a boolean is a proxy to :meth:`User.has_perm() `. For example, -to check if the logged-in user has the permission ``foo.can_vote``:: +to check if the logged-in user has the permission ``foo.add_vote``:: - {% if perms.foo.can_vote %} + {% if perms.foo.add_vote %} Here's a more complete example of checking permissions in a template: @@ -1631,10 +1631,10 @@ Here's a more complete example of checking permissions in a template: {% if perms.foo %}

You have permission to do something in the foo app.

- {% if perms.foo.can_vote %} + {% if perms.foo.add_vote %}

You can vote!

{% endif %} - {% if perms.foo.can_drive %} + {% if perms.foo.add_driving %}

You can drive!

{% endif %} {% else %} @@ -1647,7 +1647,7 @@ For example: .. code-block:: html+django {% if 'foo' in perms %} - {% if 'foo.can_vote' in perms %} + {% if 'foo.add_vote' in perms %}

In lookup works, too.

{% endif %} {% endif %} From 80e6639e22be464cc8b042450f505293a617967a Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 20 Feb 2020 14:05:47 +0000 Subject: [PATCH 18/28] [3.0.x] Fixed #31182 -- Adjusted release notes for ASGI support. Backport of a6b3938afc0204093b5356ade2be30b461a698c5 from master --- docs/releases/3.0.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/releases/3.0.txt b/docs/releases/3.0.txt index 50c98ade58d0..51ca584ecb5d 100644 --- a/docs/releases/3.0.txt +++ b/docs/releases/3.0.txt @@ -54,6 +54,11 @@ This is in addition to our existing WSGI support. Django intends to support both for the foreseeable future. Async features will only be available to applications that run under ASGI, however. +At this stage async support only applies to the outer ASGI application. +Internally everything remains synchronous. Asynchronous middleware, views, etc. +are not yet supported. You can, however, use ASGI middleware around Django's +application, allowing you to combine Django with other ASGI frameworks. + There is no need to switch your applications over unless you want to start experimenting with asynchronous code, but we have :doc:`documentation on deploying with ASGI ` if From 0193a1630eca5c55721c04562f24c6f17464f901 Mon Sep 17 00:00:00 2001 From: Matheus Cunha Motta Date: Sun, 23 Feb 2020 19:15:48 -0300 Subject: [PATCH 19/28] [3.0.x] Fixed #31303 -- Removed outdated note about symmetrical intermediate table for self-referential ManyToManyField. Follow up to 87b1ad6e7351464c60e751b483d9dfce3a2d3382. Backport of 0352a44dd61c19bebf0c0b305dbbc3f710ff9945 from master --- docs/topics/db/models.txt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt index 75a46485acdc..9fab17872e25 100644 --- a/docs/topics/db/models.txt +++ b/docs/topics/db/models.txt @@ -499,11 +499,6 @@ There are a few restrictions on the intermediate model: must also specify ``through_fields`` as above, or a validation error will be raised. -* When defining a many-to-many relationship from a model to - itself, using an intermediary model, you *must* use - :attr:`symmetrical=False ` (see - :ref:`the model field reference `). - Now that you have set up your :class:`~django.db.models.ManyToManyField` to use your intermediary model (``Membership``, in this case), you're ready to start creating some many-to-many relationships. You do this by creating instances of From ae6c6f9110a0bd30a72c5bc5067920ba2725dc6b Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 26 Feb 2020 12:00:52 +0100 Subject: [PATCH 20/28] [3.0.x] Removed hint from fields.E310 message in system check docs. This is the only documented hint. Backport of 667f784baab31f11d2469e5d22bbdc2390dbc030 from master --- docs/ref/checks.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 1289ffe1ab28..289defa5ba1b 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -211,8 +211,7 @@ Related fields * **fields.E309**: Reverse query name ```` must not contain ``'__'``. * **fields.E310**: No subset of the fields ````, ````, ... on - model ```` is unique. Add ``unique=True`` on any of those fields or - add at least a subset of them to a unique_together constraint. + model ```` is unique. * **fields.E311**: ```` must set ``unique=True`` because it is referenced by a ``ForeignKey``. * **fields.E312**: The ``to_field`` ```` doesn't exist on the From 59ac25c93b228cd4ef34d4f36fbfbab3d9f6b4ad Mon Sep 17 00:00:00 2001 From: Andrey Doroschenko Date: Thu, 27 Feb 2020 08:26:52 +0300 Subject: [PATCH 21/28] [3.0.x] Fixed #31313 -- Fixed is_upperclass() example in enumeration types docs. Backport of f1016814d84b1423cfe0df85644c9870a6bc6b41 from master --- docs/ref/models/fields.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index dfd9f9b665ee..f997921148a3 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -204,7 +204,10 @@ choices in a concise way:: ) def is_upperclass(self): - return self.year_in_school in {YearInSchool.JUNIOR, YearInSchool.SENIOR} + return self.year_in_school in { + self.YearInSchool.JUNIOR, + self.YearInSchool.SENIOR, + } These work similar to :mod:`enum` from Python's standard library, but with some modifications: From 16cacdcb3f7856df5454b648503374de150fa245 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Thu, 27 Feb 2020 00:34:37 -0500 Subject: [PATCH 22/28] [3.0.x] Fixed #31312 -- Properly ordered temporal subtraction params on MySQL. Regression in 9bcbcd599abac91ea853b2fe10b784ba32df043e. Thanks rick2ricks for the report. Backport of 41ebe60728a15aa273f4d70de92f5246a89c3d4e from master --- django/db/backends/mysql/operations.py | 2 +- docs/releases/3.0.4.txt | 4 ++++ tests/expressions/tests.py | 23 ++++++++++++++++++++--- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/django/db/backends/mysql/operations.py b/django/db/backends/mysql/operations.py index 241e6e2135a3..b3225fafa07f 100644 --- a/django/db/backends/mysql/operations.py +++ b/django/db/backends/mysql/operations.py @@ -287,7 +287,7 @@ def subtract_temporals(self, internal_type, lhs, rhs): "((TIME_TO_SEC(%(lhs)s) * 1000000 + MICROSECOND(%(lhs)s)) -" " (TIME_TO_SEC(%(rhs)s) * 1000000 + MICROSECOND(%(rhs)s)))" ) % {'lhs': lhs_sql, 'rhs': rhs_sql}, tuple(lhs_params) * 2 + tuple(rhs_params) * 2 - params = (*lhs_params, *rhs_params) + params = (*rhs_params, *lhs_params) return "TIMESTAMPDIFF(MICROSECOND, %s, %s)" % (rhs_sql, lhs_sql), params def explain_query_prefix(self, format=None, **options): diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index 216bc29b0e26..1da9a2255202 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -23,3 +23,7 @@ Bugfixes * Fixed a regression in Django 3.0 that caused misplacing parameters in logged SQL queries on Oracle (:ticket:`31271`). + +* Fixed a regression in Django 3.0.3 that caused misplacing parameters of SQL + queries when subtracting ``DateField`` or ``DateTimeField`` expressions on + MySQL (:ticket:`31312`). diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index 005e9150e70f..bcd2f93177c8 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -1221,13 +1221,13 @@ def setUpTestData(cls): # e0: started same day as assigned, zero duration end = stime + delta0 - e0 = Experiment.objects.create( + cls.e0 = Experiment.objects.create( name='e0', assigned=sday, start=stime, end=end, completed=end.date(), estimated_time=delta0, ) cls.deltas.append(delta0) - cls.delays.append(e0.start - datetime.datetime.combine(e0.assigned, midnight)) - cls.days_long.append(e0.completed - e0.assigned) + cls.delays.append(cls.e0.start - datetime.datetime.combine(cls.e0.assigned, midnight)) + cls.days_long.append(cls.e0.completed - cls.e0.assigned) # e1: started one day after assigned, tiny duration, data # set so that end time has no fractional seconds, which @@ -1434,6 +1434,23 @@ def test_date_subquery_subtraction(self): ).filter(difference=datetime.timedelta()) self.assertTrue(queryset.exists()) + @skipUnlessDBFeature('supports_temporal_subtraction') + def test_date_case_subtraction(self): + queryset = Experiment.objects.annotate( + date_case=Case( + When(Q(name='e0'), then=F('completed')), + output_field=DateField(), + ), + completed_value=Value( + self.e0.completed, + output_field=DateField(), + ), + difference=ExpressionWrapper( + F('date_case') - F('completed_value'), output_field=DurationField(), + ), + ).filter(difference=datetime.timedelta()) + self.assertEqual(queryset.get(), self.e0) + @skipUnlessDBFeature('supports_temporal_subtraction') def test_time_subtraction(self): Time.objects.create(time=datetime.time(12, 30, 15, 2345)) From 94e192a580374259d9e52432f007fd2b49a8672d Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 27 Feb 2020 20:18:53 +0100 Subject: [PATCH 23/28] [3.0.x] Refs #31312 -- Fixed FTimeDeltaTests.test_date_case_subtraction() test. Follow up to 16cacdcb3f7856df5454b648503374de150fa245. --- tests/expressions/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index bcd2f93177c8..e9c4f5f3ac87 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -1439,14 +1439,14 @@ def test_date_case_subtraction(self): queryset = Experiment.objects.annotate( date_case=Case( When(Q(name='e0'), then=F('completed')), - output_field=DateField(), + output_field=models.DateField(), ), completed_value=Value( self.e0.completed, - output_field=DateField(), + output_field=models.DateField(), ), difference=ExpressionWrapper( - F('date_case') - F('completed_value'), output_field=DurationField(), + F('date_case') - F('completed_value'), output_field=models.DurationField(), ), ).filter(difference=datetime.timedelta()) self.assertEqual(queryset.get(), self.e0) From 5320ba98f3d253afcaa76b4b388a8982f87d4f1a Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Sat, 29 Feb 2020 20:35:11 +0100 Subject: [PATCH 24/28] [3.0.x] Removed outdated note about not supporting partial indexes by Django. Supported since a906c9898284a9aecb5f48bdc534e9c1273864a6. Backport of a49c2b6bf098eb48c07641f60dba9be78c6cc92f from master --- docs/ref/migration-operations.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index f0436a136674..7cc9bd550a8d 100644 --- a/docs/ref/migration-operations.txt +++ b/docs/ref/migration-operations.txt @@ -250,8 +250,7 @@ Special Operations .. class:: RunSQL(sql, reverse_sql=None, state_operations=None, hints=None, elidable=False) Allows running of arbitrary SQL on the database - useful for more advanced -features of database backends that Django doesn't support directly, like -partial indexes. +features of database backends that Django doesn't support directly. ``sql``, and ``reverse_sql`` if provided, should be strings of SQL to run on the database. On most database backends (all but PostgreSQL), Django will From 4977f2084ec828c1214817e0a7a82ff96cba7863 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 3 Mar 2020 08:05:27 +0000 Subject: [PATCH 25/28] [3.0.x] Documented default value of InlineModelAdmin.extra. Backport of 3bd29a8a973e2bb11b00666458344aeab5684a39 from master --- docs/ref/contrib/admin/index.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 75d8245499f7..6a2bfbad3d93 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -2295,7 +2295,7 @@ The ``InlineModelAdmin`` class adds or customizes: .. attribute:: InlineModelAdmin.extra This controls the number of extra forms the formset will display in - addition to the initial forms. See the + addition to the initial forms. Defaults to 3. See the :doc:`formsets documentation ` for more information. From c5cfaad2f1f08b31ba04b9534f1a46a6ef1003bf Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Mon, 2 Mar 2020 13:20:36 +0100 Subject: [PATCH 26/28] [3.0.x] Fixed #31150 -- Included subqueries that reference related fields in GROUP BY clauses. Thanks Johannes Hoppe for the report. Regression in fb3f034f1c63160c0ff13c609acd01c18be12f80. Co-authored-by: Simon Charette Backport of 7b8fa1653fde578ab3a496d9974ab1d4261b8b26 from master --- django/db/models/expressions.py | 15 ++++++++++++++- docs/releases/3.0.4.txt | 3 +++ tests/aggregation/tests.py | 27 +++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 3adec334f703..2733fbada93c 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -6,6 +6,7 @@ from django.core.exceptions import EmptyResultSet, FieldError from django.db import connection from django.db.models import fields +from django.db.models.constants import LOOKUP_SEP from django.db.models.query_utils import Q from django.db.utils import NotSupportedError from django.utils.deconstruct import deconstructible @@ -558,6 +559,14 @@ def as_sql(self, *args, **kwargs): 'only be used in a subquery.' ) + def resolve_expression(self, *args, **kwargs): + col = super().resolve_expression(*args, **kwargs) + # FIXME: Rename possibly_multivalued to multivalued and fix detection + # for non-multivalued JOINs (e.g. foreign key fields). This should take + # into account only many-to-many and one-to-many relationships. + col.possibly_multivalued = LOOKUP_SEP in self.name + return col + def relabeled_clone(self, relabels): return self @@ -744,6 +753,7 @@ def as_sql(self, compiler, connection): class Col(Expression): contains_column_references = True + possibly_multivalued = False def __init__(self, alias, target, output_field=None): if output_field is None: @@ -1068,7 +1078,10 @@ def as_sql(self, compiler, connection, template=None, **extra_context): def get_group_by_cols(self, alias=None): if alias: return [Ref(alias, self)] - return self.query.get_external_cols() + external_cols = self.query.get_external_cols() + if any(col.possibly_multivalued for col in external_cols): + return [self] + return external_cols class Exists(Subquery): diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index 1da9a2255202..ad9752addb69 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -27,3 +27,6 @@ Bugfixes * Fixed a regression in Django 3.0.3 that caused misplacing parameters of SQL queries when subtracting ``DateField`` or ``DateTimeField`` expressions on MySQL (:ticket:`31312`). + +* Fixed a regression in Django 3.0 that didn't include subqueries spanning + multivalued relations in the ``GROUP BY`` clause (:ticket:`31150`). diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index dc745e4138fb..342507da010c 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -1,6 +1,7 @@ import datetime import re from decimal import Decimal +from unittest import skipIf from django.core.exceptions import FieldError from django.db import connection @@ -1189,6 +1190,26 @@ def test_aggregation_subquery_annotation_values(self): }, ]) + @skipUnlessDBFeature('supports_subqueries_in_group_by') + @skipIf( + connection.vendor == 'mysql', + 'GROUP BY optimization does not work properly when ONLY_FULL_GROUP_BY ' + 'mode is enabled on MySQL, see #31331.', + ) + def test_aggregation_subquery_annotation_multivalued(self): + """ + Subquery annotations must be included in the GROUP BY if they use + potentially multivalued relations (contain the LOOKUP_SEP). + """ + subquery_qs = Author.objects.filter( + pk=OuterRef('pk'), + book__name=OuterRef('book__name'), + ).values('pk') + author_qs = Author.objects.annotate( + subquery_id=Subquery(subquery_qs), + ).annotate(count=Count('book')) + self.assertEqual(author_qs.count(), Author.objects.count()) + def test_aggregation_order_by_not_selected_annotation_values(self): result_asc = [ self.b4.pk, @@ -1247,6 +1268,7 @@ def test_group_by_exists_annotation(self): ).annotate(total=Count('*')) self.assertEqual(dict(has_long_books_breakdown), {True: 2, False: 3}) + @skipUnlessDBFeature('supports_subqueries_in_group_by') def test_aggregation_subquery_annotation_related_field(self): publisher = Publisher.objects.create(name=self.a9.name, num_awards=2) book = Book.objects.create( @@ -1266,3 +1288,8 @@ def test_aggregation_subquery_annotation_related_field(self): contact_publisher__isnull=False, ).annotate(count=Count('authors')) self.assertSequenceEqual(books_qs, [book]) + # FIXME: GROUP BY doesn't need to include a subquery with + # non-multivalued JOINs, see Col.possibly_multivalued (refs #31150): + # with self.assertNumQueries(1) as ctx: + # self.assertSequenceEqual(books_qs, [book]) + # self.assertEqual(ctx[0]['sql'].count('SELECT'), 2) From 26a5cf834526e291db00385dd33d319b8271fc4c Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Mon, 24 Feb 2020 14:46:28 +0100 Subject: [PATCH 27/28] [3.0.x] Fixed CVE-2020-9402 -- Properly escaped tolerance parameter in GIS functions and aggregates on Oracle. Thanks to Norbert Szetei for the report. --- django/contrib/gis/db/models/aggregates.py | 14 ++++++-- django/contrib/gis/db/models/functions.py | 14 ++++---- docs/releases/1.11.29.txt | 13 ++++++++ docs/releases/2.2.11.txt | 10 ++++-- docs/releases/3.0.4.txt | 10 ++++-- docs/releases/index.txt | 1 + tests/gis_tests/distapp/tests.py | 31 ++++++++++++++++++ tests/gis_tests/geoapp/tests.py | 38 +++++++++++++++++++++- 8 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 docs/releases/1.11.29.txt diff --git a/django/contrib/gis/db/models/aggregates.py b/django/contrib/gis/db/models/aggregates.py index 993d9f91fcb3..9d169275c59b 100644 --- a/django/contrib/gis/db/models/aggregates.py +++ b/django/contrib/gis/db/models/aggregates.py @@ -1,6 +1,7 @@ from django.contrib.gis.db.models.fields import ( ExtentField, GeometryCollectionField, GeometryField, LineStringField, ) +from django.db.models import Value from django.db.models.aggregates import Aggregate from django.utils.functional import cached_property @@ -27,9 +28,16 @@ def as_sql(self, compiler, connection, function=None, **extra_context): ) def as_oracle(self, compiler, connection, **extra_context): - tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05) - template = None if self.is_extent else '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))' - return self.as_sql(compiler, connection, template=template, tolerance=tolerance, **extra_context) + if not self.is_extent: + tolerance = self.extra.get('tolerance') or getattr(self, 'tolerance', 0.05) + clone = self.copy() + clone.set_source_expressions([ + *self.get_source_expressions(), + Value(tolerance), + ]) + template = '%(function)s(SDOAGGRTYPE(%(expressions)s))' + return clone.as_sql(compiler, connection, template=template, **extra_context) + return self.as_sql(compiler, connection, **extra_context) def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False): c = super().resolve_expression(query, allow_joins, reuse, summarize, for_save) diff --git a/django/contrib/gis/db/models/functions.py b/django/contrib/gis/db/models/functions.py index a79067a45ad8..1b0a2a427ba5 100644 --- a/django/contrib/gis/db/models/functions.py +++ b/django/contrib/gis/db/models/functions.py @@ -111,12 +111,14 @@ class OracleToleranceMixin: tolerance = 0.05 def as_oracle(self, compiler, connection, **extra_context): - tol = self.extra.get('tolerance', self.tolerance) - return self.as_sql( - compiler, connection, - template="%%(function)s(%%(expressions)s, %s)" % tol, - **extra_context - ) + tolerance = Value(self._handle_param( + self.extra.get('tolerance', self.tolerance), + 'tolerance', + NUMERIC_TYPES, + )) + clone = self.copy() + clone.set_source_expressions([*self.get_source_expressions(), tolerance]) + return clone.as_sql(compiler, connection, **extra_context) class Area(OracleToleranceMixin, GeoFunc): diff --git a/docs/releases/1.11.29.txt b/docs/releases/1.11.29.txt new file mode 100644 index 000000000000..d37f3ffc0dac --- /dev/null +++ b/docs/releases/1.11.29.txt @@ -0,0 +1,13 @@ +============================ +Django 1.11.29 release notes +============================ + +*March 4, 2020* + +Django 1.11.29 fixes a security issue in 1.11.29. + +CVE-2020-9402: Potential SQL injection via ``tolerance`` parameter in GIS functions and aggregates on Oracle +============================================================================================================ + +GIS functions and aggregates on Oracle were subject to SQL injection, +using a suitably crafted ``tolerance``. diff --git a/docs/releases/2.2.11.txt b/docs/releases/2.2.11.txt index b14d961ac36a..9738ef4470a9 100644 --- a/docs/releases/2.2.11.txt +++ b/docs/releases/2.2.11.txt @@ -2,9 +2,15 @@ Django 2.2.11 release notes =========================== -*Expected March 2, 2020* +*March 4, 2020* -Django 2.2.11 fixes a data loss bug in 2.2.10. +Django 2.2.11 fixes a security issue and a data loss bug in 2.2.10. + +CVE-2020-9402: Potential SQL injection via ``tolerance`` parameter in GIS functions and aggregates on Oracle +============================================================================================================ + +GIS functions and aggregates on Oracle were subject to SQL injection, +using a suitably crafted ``tolerance``. Bugfixes ======== diff --git a/docs/releases/3.0.4.txt b/docs/releases/3.0.4.txt index ad9752addb69..647e59374911 100644 --- a/docs/releases/3.0.4.txt +++ b/docs/releases/3.0.4.txt @@ -2,9 +2,15 @@ Django 3.0.4 release notes ========================== -*Expected March 2, 2020* +*March 4, 2020* -Django 3.0.4 fixes several bugs in 3.0.3. +Django 3.0.4 fixes a security issue and several bugs in 3.0.3. + +CVE-2020-9402: Potential SQL injection via ``tolerance`` parameter in GIS functions and aggregates on Oracle +============================================================================================================ + +GIS functions and aggregates on Oracle were subject to SQL injection, +using a suitably crafted ``tolerance``. Bugfixes ======== diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 8598dfeb2926..f95aee459e88 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -96,6 +96,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 1.11.29 1.11.28 1.11.27 1.11.26 diff --git a/tests/gis_tests/distapp/tests.py b/tests/gis_tests/distapp/tests.py index 2cdd0e8f0eec..4e2c95ee18bc 100644 --- a/tests/gis_tests/distapp/tests.py +++ b/tests/gis_tests/distapp/tests.py @@ -434,6 +434,37 @@ def test_distance_function_d_lookup(self): ).filter(d=D(m=1)) self.assertTrue(qs.exists()) + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_distance_function_tolerance_escaping(self): + qs = Interstate.objects.annotate( + d=Distance( + Point(500, 500, srid=3857), + Point(0, 0, srid=3857), + tolerance='0.05) = 1 OR 1=1 OR (1+1', + ), + ).filter(d=D(m=1)).values('pk') + msg = 'The tolerance parameter has the wrong type' + with self.assertRaisesMessage(TypeError, msg): + qs.exists() + + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_distance_function_tolerance(self): + # Tolerance is greater than distance. + qs = Interstate.objects.annotate( + d=Distance( + Point(0, 0, srid=3857), + Point(1, 1, srid=3857), + tolerance=1.5, + ), + ).filter(d=0).values('pk') + self.assertIs(qs.exists(), True) + @skipIfDBFeature("supports_distance_geodetic") @skipUnlessDBFeature("has_Distance_function") def test_distance_function_raw_result_d_lookup(self): diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index 47d16434a56e..be007041a5ad 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -9,7 +9,7 @@ MultiPoint, MultiPolygon, Point, Polygon, fromstr, ) from django.core.management import call_command -from django.db import NotSupportedError, connection +from django.db import DatabaseError, NotSupportedError, connection from django.test import TestCase, skipUnlessDBFeature from ..utils import ( @@ -564,6 +564,42 @@ def test_unionagg(self): qs = City.objects.filter(name='NotACity') self.assertIsNone(qs.aggregate(Union('point'))['point__union']) + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_unionagg_tolerance(self): + City.objects.create( + point=fromstr('POINT(-96.467222 32.751389)', srid=4326), + name='Forney', + ) + tx = Country.objects.get(name='Texas').mpoly + # Tolerance is greater than distance between Forney and Dallas, that's + # why Dallas is ignored. + forney_houston = GEOSGeometry( + 'MULTIPOINT(-95.363151 29.763374, -96.467222 32.751389)', + srid=4326, + ) + self.assertIs( + forney_houston.equals( + City.objects.filter(point__within=tx).aggregate( + Union('point', tolerance=32000), + )['point__union'], + ), + True, + ) + + @unittest.skipUnless( + connection.vendor == 'oracle', + 'Oracle supports tolerance paremeter.', + ) + def test_unionagg_tolerance_escaping(self): + tx = Country.objects.get(name='Texas').mpoly + with self.assertRaises(DatabaseError): + City.objects.filter(point__within=tx).aggregate( + Union('point', tolerance='0.05))), (((1'), + ) + def test_within_subquery(self): """ Using a queryset inside a geo lookup is working (using a subquery) From c2250cfb80e27cdf8d098428824da2800a18cadf Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 4 Mar 2020 09:18:28 +0100 Subject: [PATCH 28/28] [3.0.x] Bumped version for 3.0.4 release. --- django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/__init__.py b/django/__init__.py index 0dc4cc19e8eb..443917dac4b7 100644 --- a/django/__init__.py +++ b/django/__init__.py @@ -1,6 +1,6 @@ from django.utils.version import get_version -VERSION = (3, 0, 4, 'alpha', 0) +VERSION = (3, 0, 4, 'final', 0) __version__ = get_version(VERSION)