From 2fb5a51fa3ac276efc7121ec9de91f092a986104 Mon Sep 17 00:00:00 2001 From: Bouke Haarsma Date: Tue, 15 Oct 2013 22:36:49 +0200 Subject: [PATCH] Fixed #18659 -- Deprecated request.REQUEST and MergeDict Thanks Aymeric Augustin for the suggestion. --- django/contrib/admin/options.py | 12 ++++++++---- django/contrib/auth/admin.py | 3 ++- django/contrib/auth/tests/test_views.py | 1 - django/contrib/auth/views.py | 9 ++++++--- django/core/handlers/wsgi.py | 3 +++ django/utils/datastructures.py | 2 ++ django/views/i18n.py | 2 +- docs/internals/deprecation.txt | 4 ++++ docs/ref/request-response.txt | 3 +++ docs/releases/1.7.txt | 15 +++++++++++++++ tests/deprecation/tests.py | 22 +++++++++++++++++++++- tests/forms_tests/tests/test_forms.py | 10 +++++++--- tests/test_client_regress/views.py | 20 ++++++++++++++------ tests/utils_tests/test_datastructures.py | 2 +- 14 files changed, 87 insertions(+), 21 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 3b02ac020c9..e27672773c1 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1284,8 +1284,10 @@ class ModelAdmin(BaseModelAdmin): context = dict(self.admin_site.each_context(), title=_('Add %s') % force_text(opts.verbose_name), adminform=adminForm, - is_popup=IS_POPUP_VAR in request.REQUEST, - to_field=request.REQUEST.get(TO_FIELD_VAR), + is_popup=(IS_POPUP_VAR in request.POST or + IS_POPUP_VAR in request.GET), + to_field=request.POST.get(TO_FIELD_VAR, + request.GET.get(TO_FIELD_VAR)), media=media, inline_admin_formsets=inline_admin_formsets, errors=helpers.AdminErrorList(form, formsets), @@ -1357,8 +1359,10 @@ class ModelAdmin(BaseModelAdmin): adminform=adminForm, object_id=object_id, original=obj, - is_popup=IS_POPUP_VAR in request.REQUEST, - to_field=request.REQUEST.get(TO_FIELD_VAR), + is_popup=(IS_POPUP_VAR in request.POST or + IS_POPUP_VAR in request.GET), + to_field=request.POST.get(TO_FIELD_VAR, + request.GET.get(TO_FIELD_VAR)), media=media, inline_admin_formsets=inline_admin_formsets, errors=helpers.AdminErrorList(form, formsets), diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index ff08f417983..8514f3bfcf4 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -144,7 +144,8 @@ class UserAdmin(admin.ModelAdmin): 'adminForm': adminForm, 'form_url': form_url, 'form': form, - 'is_popup': IS_POPUP_VAR in request.REQUEST, + 'is_popup': (IS_POPUP_VAR in request.POST or + IS_POPUP_VAR in request.GET), 'add': True, 'change': False, 'has_delete_permission': False, diff --git a/django/contrib/auth/tests/test_views.py b/django/contrib/auth/tests/test_views.py index ce69048bfbf..d9a848bdd23 100644 --- a/django/contrib/auth/tests/test_views.py +++ b/django/contrib/auth/tests/test_views.py @@ -526,7 +526,6 @@ class LoginTest(AuthViewsTestCase): req.COOKIES[settings.CSRF_COOKIE_NAME] = token1 req.method = "POST" req.POST = {'username': 'testclient', 'password': password, 'csrfmiddlewaretoken': token1} - req.REQUEST = req.POST # Use POST request to log in SessionMiddleware().process_request(req) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index d20e0615388..891beaa6424 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -28,7 +28,8 @@ def login(request, template_name='registration/login.html', """ Displays the login form and handles the login action. """ - redirect_to = request.REQUEST.get(redirect_field_name, '') + redirect_to = request.POST.get(redirect_field_name, + request.GET.get(redirect_field_name, '')) if request.method == "POST": form = authentication_form(request, data=request.POST) @@ -71,8 +72,10 @@ def logout(request, next_page=None, if next_page is not None: next_page = resolve_url(next_page) - if redirect_field_name in request.REQUEST: - next_page = request.REQUEST[redirect_field_name] + if (redirect_field_name in request.POST or + redirect_field_name in request.GET): + next_page = request.POST.get(redirect_field_name, + request.GET.get(redirect_field_name)) # Security check -- don't allow redirection to a different host. if not is_safe_url(url=next_page, host=request.get_host()): next_page = request.path diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index 7c7415d3308..c310f1678a0 100644 --- a/django/core/handlers/wsgi.py +++ b/django/core/handlers/wsgi.py @@ -5,6 +5,7 @@ import logging import sys from io import BytesIO from threading import Lock +import warnings from django import http from django.conf import settings @@ -129,6 +130,8 @@ class WSGIRequest(http.HttpRequest): return content_type, content_params def _get_request(self): + warnings.warn('`request.REQUEST` is deprecated, use `request.GET` or ' + '`request.POST` instead.', PendingDeprecationWarning, 2) if not hasattr(self, '_request'): self._request = datastructures.MergeDict(self.POST, self.GET) return self._request diff --git a/django/utils/datastructures.py b/django/utils/datastructures.py index 2f60c29cb02..fe0fc164e99 100644 --- a/django/utils/datastructures.py +++ b/django/utils/datastructures.py @@ -12,6 +12,8 @@ class MergeDict(object): first occurrence will be used. """ def __init__(self, *dicts): + warnings.warn('`MergeDict` is deprecated, use `dict.update()` ' + 'instead.', PendingDeprecationWarning, 2) self.dicts = dicts def __bool__(self): diff --git a/django/views/i18n.py b/django/views/i18n.py index c9139229999..86acb9c032a 100644 --- a/django/views/i18n.py +++ b/django/views/i18n.py @@ -24,7 +24,7 @@ def set_language(request): redirect to the page in the request (the 'next' parameter) without changing any state. """ - next = request.REQUEST.get('next') + next = request.POST.get('next', request.GET.get('next')) if not is_safe_url(url=next, host=request.get_host()): next = request.META.get('HTTP_REFERER') if not is_safe_url(url=next, host=request.get_host()): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index b584a28ba1f..bf26d8d8558 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -469,6 +469,10 @@ these changes. * ``django.forms.get_declared_fields`` will be removed. +* The ``WSGIRequest.REQUEST`` property will be removed. + +* The class ``django.utils.datastructures.MergeDict`` will be removed. + 2.0 --- diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index 57222c8ca78..e5ded0c9e05 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -110,6 +110,9 @@ All attributes should be considered read-only, unless stated otherwise below. .. attribute:: HttpRequest.REQUEST + .. deprecated:: 1.7 + Use the more explicit ``GET`` and ``POST`` instead. + For convenience, a dictionary-like object that searches ``POST`` first, then ``GET``. Inspired by PHP's ``$_REQUEST``. diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index aa613c45389..d5c1f886da9 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -639,3 +639,18 @@ deprecated. Use :djadminopt:`--natural-foreign` instead. Similarly, the ``use_natural_keys`` argument for ``serializers.serialize()`` has been deprecated. Use ``use_natural_foreign_keys`` instead. + +Merging of ``POST`` and ``GET`` arguments into ``WSGIRequest.REQUEST`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +It was already strongly suggested that you use ``GET`` and ``POST`` instead of +``REQUEST``, because the former are more explicit. The property ``REQUEST`` is +deprecated and will be removed in Django 1.9. + +``django.utils.datastructures.MergeDict`` class +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``MergeDict`` exists primarily to support merging ``POST`` and ``GET`` +arguments into a ``REQUEST`` property on ``WSGIRequest``. To merge +dictionaries, use ``dict.update()`` instead. The class ``MergeDict`` is +deprecated and will be removed in Django 1.9. diff --git a/tests/deprecation/tests.py b/tests/deprecation/tests.py index 719bd635dbe..5353a16e428 100644 --- a/tests/deprecation/tests.py +++ b/tests/deprecation/tests.py @@ -1,8 +1,9 @@ from __future__ import unicode_literals import warnings -from django.test import SimpleTestCase +from django.test import SimpleTestCase, RequestFactory from django.utils import six +from django.utils.datastructures import MergeDict from django.utils.deprecation import RenameMethodsBase @@ -156,3 +157,22 @@ class RenameMethodsTests(SimpleTestCase): '`DeprecatedMixin.old` is deprecated, use `new` instead.', '`RenamedMixin.old` is deprecated, use `new` instead.', ]) + + +class DeprecatingRequestMergeDictTest(SimpleTestCase): + def test_deprecated_request(self): + """ + Ensure the correct warning is raised when WSGIRequest.REQUEST is + accessed. + """ + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter('always') + request = RequestFactory().get('/') + _ = request.REQUEST + + msgs = [str(warning.message) for warning in recorded] + self.assertEqual(msgs, [ + '`request.REQUEST` is deprecated, use `request.GET` or ' + '`request.POST` instead.', + '`MergeDict` is deprecated, use `dict.update()` instead.', + ]) diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 33cf24f7c60..e9718609090 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import copy import datetime +import warnings from django.core.files.uploadedfile import SimpleUploadedFile from django.core.validators import RegexValidator @@ -560,9 +561,12 @@ class FormsTestCase(TestCase): f = SongForm(data) self.assertEqual(f.errors, {}) - data = MergeDict(MultiValueDict(dict(name=['Yesterday'], composers=['J', 'P']))) - f = SongForm(data) - self.assertEqual(f.errors, {}) + # MergeDict is deprecated, but is supported until removed. + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + data = MergeDict(MultiValueDict(dict(name=['Yesterday'], composers=['J', 'P']))) + f = SongForm(data) + self.assertEqual(f.errors, {}) def test_multiple_hidden(self): class SongForm(Form): diff --git a/tests/test_client_regress/views.py b/tests/test_client_regress/views.py index d7c54cf2f72..7bd439a88ef 100644 --- a/tests/test_client_regress/views.py +++ b/tests/test_client_regress/views.py @@ -1,4 +1,5 @@ import json +import warnings from django.conf import settings from django.contrib.auth.decorators import login_required @@ -32,13 +33,20 @@ get_view = login_required(get_view) def request_data(request, template='base.html', data='sausage'): "A simple view that returns the request data in the context" + + # request.REQUEST is deprecated, but needs testing until removed. + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + request_foo = request.REQUEST.get('foo') + request_bar = request.REQUEST.get('bar') + return render_to_response(template, { - 'get-foo':request.GET.get('foo',None), - 'get-bar':request.GET.get('bar',None), - 'post-foo':request.POST.get('foo',None), - 'post-bar':request.POST.get('bar',None), - 'request-foo':request.REQUEST.get('foo',None), - 'request-bar':request.REQUEST.get('bar',None), + 'get-foo': request.GET.get('foo'), + 'get-bar': request.GET.get('bar'), + 'post-foo': request.POST.get('foo'), + 'post-bar': request.POST.get('bar'), + 'request-foo': request_foo, + 'request-bar': request_bar, 'data': data, }) diff --git a/tests/utils_tests/test_datastructures.py b/tests/utils_tests/test_datastructures.py index 5887cad085c..4908957a192 100644 --- a/tests/utils_tests/test_datastructures.py +++ b/tests/utils_tests/test_datastructures.py @@ -136,7 +136,7 @@ class SortedDictTests(IgnorePendingDeprecationWarningsMixin, SimpleTestCase): self.assertEqual(list(reversed(self.d2)), [7, 0, 9, 1]) -class MergeDictTests(SimpleTestCase): +class MergeDictTests(IgnorePendingDeprecationWarningsMixin, SimpleTestCase): def test_simple_mergedict(self): d1 = {'chris':'cool', 'camri':'cute', 'cotton':'adorable',