From 41e3db81be6425a3aa5790fae53b548a129b02ae Mon Sep 17 00:00:00 2001 From: rht Date: Fri, 2 Feb 2018 09:43:18 +0500 Subject: [PATCH] dependencies: Upgrade to Django 2.2.10. Django 2.2.x is the next LTS release after Django 1.11.x; I expect we'll be on it for a while, as Django 3.x won't have an LTS release series out for a while. Because of upstream API changes in Django, this commit includes several changes beyond requirements and: * urls: django.urls.resolvers.RegexURLPattern has been replaced by django.urls.resolvers.URLPattern; affects OpenAPI code and related features which re-parse Django's internals. https://code.djangoproject.com/ticket/28593 * test_runner: Change number to suffix. Django changed the name in this ticket: https://code.djangoproject.com/ticket/28578 * Delete now-unnecessary SameSite cookie code (it's now the default). * forms: urlsafe_base64_encode returns string in Django 2.2. https://docs.djangoproject.com/en/2.2/ref/utils/#django.utils.http.urlsafe_base64_encode * upload: Django's File.size property replaces _get_size(). https://docs.djangoproject.com/en/2.2/_modules/django/core/files/base/ * process_queue: Migrate to new autoreload API. * test_messages: Add an extra query caused by .refresh_from_db() losing the .select_related() on the Realm object. * session: Sync SessionHostDomainMiddleware with Django 2.2. There's a lot more we can do to take advantage of the new release; this is tracked in #11341. Many changes by Tim Abbott, Umair Waheed, and Mateusz Mandera squashed are squashed into this commit. Fixes #10835. --- corporate/tests/test_stripe.py | 2 +- docs/subsystems/django-upgrades.md | 8 +++----- requirements/common.in | 2 +- requirements/dev.txt | 10 +++++++--- requirements/prod.txt | 10 +++++++--- version.py | 2 +- zerver/forms.py | 2 +- zerver/lib/integrations.py | 4 ++-- zerver/lib/test_helpers.py | 8 ++++---- zerver/lib/test_runner.py | 10 +++++----- zerver/management/commands/process_queue.py | 4 ++-- zerver/middleware.py | 5 +++-- zerver/models.py | 2 +- zerver/tests/test_messages.py | 2 +- zerver/tests/test_openapi.py | 4 ++-- zerver/tests/test_urls.py | 2 +- zerver/views/upload.py | 2 +- zproject/settings.py | 6 ++---- zproject/urls.py | 20 -------------------- 19 files changed, 45 insertions(+), 60 deletions(-) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 24d13f9eaf..e2d6c2f72e 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -11,7 +11,7 @@ import ujson import json from django.core import signing -from django.core.urlresolvers import get_resolver +from django.urls.resolvers import get_resolver from django.http import HttpResponse from django.utils.timezone import utc as timezone_utc diff --git a/docs/subsystems/django-upgrades.md b/docs/subsystems/django-upgrades.md index ba91596cb8..b565a31144 100644 --- a/docs/subsystems/django-upgrades.md +++ b/docs/subsystems/django-upgrades.md @@ -28,8 +28,6 @@ new major versions of Django. Here are the steps: * `PasswordResetForm` and any other forms we import from `django.contrib.auth.forms` in `zerver/forms.py` (which has all of our Django forms). - * `AsyncDjangoHandlerBase` in `zerver/tornado/handlers.py` is a very - small patch on `base.BaseHandler`; see the comments with `BY - ZULIP` for our changes (unfortunately now more different due to - style differences and mypy annotations). This configurability - should also be contributed to Django upstream. + * Our AsyncDjangoHandler class has some code copied from the core + Django handlers code; look at whether that code was changed in + Django upstream. diff --git a/requirements/common.in b/requirements/common.in index a89b8cb6db..2a67abd63c 100644 --- a/requirements/common.in +++ b/requirements/common.in @@ -3,7 +3,7 @@ # and requirements/prod.txt. # See requirements/README.md for more detail. # Django itself -Django==1.11.* # https://github.com/zulip/zulip/issues/10835 +Django==2.2.* # needed for Deque (in Python < 3.5.4) and TypedDict typing-extensions diff --git a/requirements/dev.txt b/requirements/dev.txt index 22cc8c3f7d..27363b6bde 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -269,9 +269,9 @@ django-two-factor-auth[phonenumberslite]==1.10.0 \ django-webpack4-loader==0.0.5 \ --hash=sha256:baa043c4601ed763d161490e2888cf6aa93a2fd9b60681e6b19e35dc7fcb155d \ --hash=sha256:be90257041170f39c025ff674c9064569c9303b464536488b6a6cedd8e3d28be -django==1.11.28 \ - --hash=sha256:a3b01cdff845a43830d7ccacff55e0b8ff08305a4cbf894517a686e53ba3ad2d \ - --hash=sha256:b33ce35f47f745fea6b5aa3cf3f4241069803a3712d423ac748bd673a39741eb +django==2.2.10 \ + --hash=sha256:1226168be1b1c7efd0e66ee79b0e0b58b2caa7ed87717909cd8a57bb13a7079a \ + --hash=sha256:9a4635813e2d498a3c01b10c701fe4a515d76dd290aaa792ccb65ca4ccb6b038 docker==4.1.0 \ --hash=sha256:6e06c5e70ba4fad73e35f00c55a895a448398f3ada7faae072e2bb01348bafc1 \ --hash=sha256:8f93775b8bdae3a2df6bc9a5312cce564cade58d6555f2c2570165a1270cd8a7 \ @@ -804,6 +804,10 @@ sphinxcontrib-serializinghtml==1.1.3 \ # via sphinx sqlalchemy==1.3.13 \ --hash=sha256:64a7b71846db6423807e96820993fa12a03b89127d278290ca25c0b11ed7b4fb +sqlparse==0.3.0 \ + --hash=sha256:40afe6b8d4b1117e7dff5504d7a8ce07d9a1b15aeeade8a2d10f130a834f8177 \ + --hash=sha256:7c3dca29c022744e95b547e867cee89f4fce4373f3549ccd8797d8eb52cdb873 \ + # via django sshpubkeys==3.1.0 \ --hash=sha256:9f73d51c2ef1e68cd7bde0825df29b3c6ec89f4ce24ebca3bf9eaa4a23a284db \ --hash=sha256:b388399caeeccdc145f06fd0d2665eeecc545385c60b55c282a15a022215af80 \ diff --git a/requirements/prod.txt b/requirements/prod.txt index 02c1d9dd55..238f63c081 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -190,9 +190,9 @@ django-two-factor-auth[phonenumberslite]==1.10.0 \ django-webpack4-loader==0.0.5 \ --hash=sha256:baa043c4601ed763d161490e2888cf6aa93a2fd9b60681e6b19e35dc7fcb155d \ --hash=sha256:be90257041170f39c025ff674c9064569c9303b464536488b6a6cedd8e3d28be -django==1.11.28 \ - --hash=sha256:a3b01cdff845a43830d7ccacff55e0b8ff08305a4cbf894517a686e53ba3ad2d \ - --hash=sha256:b33ce35f47f745fea6b5aa3cf3f4241069803a3712d423ac748bd673a39741eb +django==2.2.10 \ + --hash=sha256:1226168be1b1c7efd0e66ee79b0e0b58b2caa7ed87717909cd8a57bb13a7079a \ + --hash=sha256:9a4635813e2d498a3c01b10c701fe4a515d76dd290aaa792ccb65ca4ccb6b038 future==0.18.2 \ --hash=sha256:b1bead90b70cf6ec3f0710ae53a525360fa360d306a86583adc6bf83a4db537d \ # via python-twitter @@ -529,6 +529,10 @@ sourcemap==0.2.1 \ --hash=sha256:c448a8c48f9482e522e4582106b0c641a83b5dbc7f13927b178848e3ea20967b sqlalchemy==1.3.13 \ --hash=sha256:64a7b71846db6423807e96820993fa12a03b89127d278290ca25c0b11ed7b4fb +sqlparse==0.3.0 \ + --hash=sha256:40afe6b8d4b1117e7dff5504d7a8ce07d9a1b15aeeade8a2d10f130a834f8177 \ + --hash=sha256:7c3dca29c022744e95b547e867cee89f4fce4373f3549ccd8797d8eb52cdb873 \ + # via django statsd==3.3.0 \ --hash=sha256:c610fb80347fca0ef62666d241bce64184bd7cc1efe582f9690e045c25535eaa \ --hash=sha256:e3e6db4c246f7c59003e51c9720a51a7f39a396541cb9b147ff4b14d15b5dd1f \ diff --git a/version.py b/version.py index dcff2aaa43..a6c68914e1 100644 --- a/version.py +++ b/version.py @@ -26,4 +26,4 @@ LATEST_RELEASE_ANNOUNCEMENT = "https://blog.zulip.org/2019/12/13/zulip-2-1-relea # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = '72.0' +PROVISION_VERSION = '73.0' diff --git a/zerver/forms.py b/zerver/forms.py index cbc8f95244..302574c254 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -211,7 +211,7 @@ class LoggingSetPasswordForm(SetPasswordForm): def generate_password_reset_url(user_profile: UserProfile, token_generator: PasswordResetTokenGenerator) -> str: token = token_generator.make_token(user_profile) - uid = urlsafe_base64_encode(force_bytes(user_profile.id)).decode('ascii') + uid = urlsafe_base64_encode(force_bytes(user_profile.id)) endpoint = reverse('django.contrib.auth.views.password_reset_confirm', kwargs=dict(uidb64=uid, token=token)) return "{}{}".format(user_profile.realm.uri, endpoint) diff --git a/zerver/lib/integrations.py b/zerver/lib/integrations.py index 6c7bab4973..0fad61b905 100644 --- a/zerver/lib/integrations.py +++ b/zerver/lib/integrations.py @@ -3,7 +3,7 @@ import os from typing import Dict, List, Optional, Any, Tuple from django.conf.urls import url from django.contrib.staticfiles.storage import staticfiles_storage -from django.urls.resolvers import LocaleRegexProvider +from django.urls.resolvers import RegexPattern from django.utils.module_loading import import_string from django.utils.translation import ugettext as _ from zerver.lib.storage import static_path @@ -178,7 +178,7 @@ class WebhookIntegration(Integration): self.doc = doc @property - def url_object(self) -> LocaleRegexProvider: + def url_object(self) -> RegexPattern: return url(self.url, self.function) class HubotIntegration(Integration): diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 104b9bd297..1db4b7f714 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -4,7 +4,7 @@ from typing import ( Optional, Tuple, Union, IO, TypeVar, TYPE_CHECKING ) -from django.urls.resolvers import LocaleRegexURLResolver +from django.urls import URLResolver from django.conf import settings from django.test import override_settings from django.http import HttpResponse, HttpResponseRedirect @@ -369,13 +369,13 @@ def write_instrumentation_reports(full_suite: bool, include_webhooks: bool) -> N def find_pattern(pattern: Any, prefixes: List[str]) -> None: - if isinstance(pattern, type(LocaleRegexURLResolver)): + if isinstance(pattern, type(URLResolver)): return # nocoverage -- shouldn't actually happen if hasattr(pattern, 'url_patterns'): return - canon_pattern = prefixes[0] + re_strip(pattern.regex.pattern) + canon_pattern = prefixes[0] + re_strip(pattern.pattern.regex.pattern) cnt = 0 for call in calls: if 'pattern' in call: @@ -386,7 +386,7 @@ def write_instrumentation_reports(full_suite: bool, include_webhooks: bool) -> N for prefix in prefixes: if url.startswith(prefix): match_url = url[len(prefix):] - if pattern.regex.match(match_url): + if pattern.resolve(match_url): if call['status_code'] in [200, 204, 301, 302]: cnt += 1 call['pattern'] = canon_pattern diff --git a/zerver/lib/test_runner.py b/zerver/lib/test_runner.py index f8e13c9001..5b73e01a88 100644 --- a/zerver/lib/test_runner.py +++ b/zerver/lib/test_runner.py @@ -8,7 +8,7 @@ from unittest.result import TestResult from django.conf import settings from django.db import connections, ProgrammingError -from django.urls.resolvers import RegexURLPattern +from django.urls.resolvers import URLPattern from django.test import TestCase from django.test import runner as django_runner from django.test.runner import DiscoverRunner @@ -253,7 +253,7 @@ def destroy_test_databases(worker_id: Optional[int]=None) -> None: if worker_id is not None: """Modified from the Django original to """ database_id = random_id_range_start + worker_id - connection.creation.destroy_test_db(number=database_id) + connection.creation.destroy_test_db(suffix=str(database_id)) else: connection.creation.destroy_test_db() except ProgrammingError: @@ -265,7 +265,7 @@ def create_test_databases(worker_id: int) -> None: for alias in connections: connection = connections[alias] connection.creation.clone_test_db( - number=database_id, + suffix=str(database_id), keepdb=True, ) @@ -304,8 +304,8 @@ def init_worker(counter: Synchronized) -> None: create_test_databases(_worker_id) initialize_worker_path(_worker_id) - def is_upload_avatar_url(url: RegexURLPattern) -> bool: - if url.regex.pattern == r'^user_avatars/(?P.*)$': + def is_upload_avatar_url(url: URLPattern) -> bool: + if url.pattern.regex.pattern == r'^user_avatars/(?P.*)$': return True return False diff --git a/zerver/management/commands/process_queue.py b/zerver/management/commands/process_queue.py index ad92ffd5bc..8343b92e5a 100644 --- a/zerver/management/commands/process_queue.py +++ b/zerver/management/commands/process_queue.py @@ -61,11 +61,11 @@ class Command(BaseCommand): if options['all']: signal.signal(signal.SIGUSR1, exit_with_three) - autoreload.main(run_threaded_workers, (get_active_worker_queues(), logger)) + autoreload.run_with_reloader(run_threaded_workers, get_active_worker_queues(), logger) elif options['multi_threaded']: signal.signal(signal.SIGUSR1, exit_with_three) queues = options['multi_threaded'] - autoreload.main(run_threaded_workers, (queues, logger)) + autoreload.run_with_reloader(run_threaded_workers, queues, logger) else: queue_name = options['queue_name'] worker_num = options['worker_num'] diff --git a/zerver/middleware.py b/zerver/middleware.py index d3b8640d40..2a6d1c3f2b 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -14,7 +14,7 @@ from django.http import HttpRequest, HttpResponse, StreamingHttpResponse from django.shortcuts import render from django.utils.cache import patch_vary_headers from django.utils.deprecation import MiddlewareMixin -from django.utils.http import cookie_date +from django.utils.http import http_date from django.utils.translation import ugettext as _ from django.views.csrf import csrf_failure as html_csrf_failure @@ -447,7 +447,7 @@ class SessionHostDomainMiddleware(SessionMiddleware): else: max_age = request.session.get_expiry_age() expires_time = time.time() + max_age - expires = cookie_date(expires_time) + expires = http_date(expires_time) # Save the session data and refresh the client cookie. # Skip session save for 500 responses, refs #3881. if response.status_code != 500: @@ -474,6 +474,7 @@ class SessionHostDomainMiddleware(SessionMiddleware): path=settings.SESSION_COOKIE_PATH, secure=settings.SESSION_COOKIE_SECURE or None, httponly=settings.SESSION_COOKIE_HTTPONLY or None, + samesite=settings.SESSION_COOKIE_SAMESITE, ) return response diff --git a/zerver/models.py b/zerver/models.py index 3a4cf67310..b38f05e640 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2332,7 +2332,7 @@ class UserPresence(models.Model): ] user_profile = models.ForeignKey(UserProfile, on_delete=CASCADE) # type: UserProfile - realm = models.ForeignKey(Realm) # type: Realm + realm = models.ForeignKey(Realm, on_delete=CASCADE) # type: Realm client = models.ForeignKey(Client, on_delete=CASCADE) # type: Client # The time we heard this update from the client. diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index f6a2a18d17..4317eeb84e 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -4203,7 +4203,7 @@ class SoftDeactivationMessageTest(ZulipTestCase): self.assertNotEqual(idle_user_msg_list[-1], sent_message) with queries_captured() as queries: add_missing_messages(long_term_idle_user) - self.assert_length(queries, 6) + self.assert_length(queries, 7) idle_user_msg_list = get_user_messages(long_term_idle_user) self.assertEqual(len(idle_user_msg_list), idle_user_msg_count + 1) self.assertEqual(idle_user_msg_list[-1], sent_message) diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index de5085b3cb..6588d2c5d6 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -547,7 +547,7 @@ do not match the types declared in the implementation of {}.\n""".format(functio # accepted by every view function in arguments_map. accepted_arguments = set(arguments_map[function_name]) - regex_pattern = p.regex.pattern + regex_pattern = p.pattern.regex.pattern url_pattern = self.convert_regex_to_url_pattern(regex_pattern) if "intentionally_undocumented" in tags: @@ -579,7 +579,7 @@ so maybe we shouldn't include it in pending_endpoints. # # * method is the HTTP method, e.g. GET, POST, or PATCH # - # * p.regex.pattern is the URL pattern; might require + # * p.pattern.regex.pattern is the URL pattern; might require # some processing to match with OpenAPI rules # # * accepted_arguments is the full set of arguments diff --git a/zerver/tests/test_urls.py b/zerver/tests/test_urls.py index ec01b347bd..6d10ecb174 100644 --- a/zerver/tests/test_urls.py +++ b/zerver/tests/test_urls.py @@ -108,7 +108,7 @@ class PublicURLTest(ZulipTestCase): self.assertEqual('ABCD', data['google_client_id']) class URLResolutionTest(TestCase): - def get_callback_string(self, pattern: django.urls.resolvers.RegexURLPattern) -> Optional[str]: + def get_callback_string(self, pattern: django.urls.resolvers.URLPattern) -> Optional[str]: callback_str = hasattr(pattern, 'lookup_str') and 'lookup_str' callback_str = callback_str or '_callback_str' return getattr(pattern, callback_str, None) diff --git a/zerver/views/upload.py b/zerver/views/upload.py index 42a7f4a5a5..9770a4ce48 100644 --- a/zerver/views/upload.py +++ b/zerver/views/upload.py @@ -67,7 +67,7 @@ def upload_file_backend(request: HttpRequest, user_profile: UserProfile) -> Http return json_error(_("You may only upload one file at a time")) user_file = list(request.FILES.values())[0] - file_size = user_file._get_size() + file_size = user_file.size if settings.MAX_FILE_UPLOAD_SIZE * 1024 * 1024 < file_size: return json_error(_("Uploaded file is larger than the allowed limit of %s MB") % ( settings.MAX_FILE_UPLOAD_SIZE)) diff --git a/zproject/settings.py b/zproject/settings.py index 5316305ba0..ea177197a8 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -161,7 +161,6 @@ MIDDLEWARE = ( 'zerver.middleware.JsonErrorHandler', 'zerver.middleware.RateLimitMiddleware', 'zerver.middleware.FlushDisplayRecipientCache', - 'django_cookies_samesite.middleware.CookiesSameSite', 'django.middleware.common.CommonMiddleware', 'zerver.middleware.SessionHostDomainMiddleware', 'django.middleware.locale.LocaleMiddleware', @@ -379,6 +378,8 @@ REDIS_PASSWORD = get_secret('redis_password') # SECURITY SETTINGS ######################################################################## +SESSION_COOKIE_SAMESITE = 'Lax' + # Tell the browser to never send our cookies without encryption, e.g. # when executing the initial http -> https redirect. # @@ -392,9 +393,6 @@ if PRODUCTION: if domain is not None: CSRF_COOKIE_DOMAIN = '.' + domain -# Enable SameSite cookies (default in Django 2.1) -SESSION_COOKIE_SAMESITE = 'Lax' - # Prevent Javascript from reading the CSRF token from cookies. Our code gets # the token from the DOM, which means malicious code could too. But hiding the # cookie will slow down some attackers. diff --git a/zproject/urls.py b/zproject/urls.py index ec3332d771..2343342920 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -1,7 +1,6 @@ from django.conf import settings from django.conf.urls import url, include from django.conf.urls.i18n import i18n_patterns -from django.http import HttpResponseBadRequest, HttpRequest, HttpResponse from django.views.generic import TemplateView, RedirectView from django.utils.module_loading import import_string import os @@ -752,22 +751,3 @@ if settings.DEVELOPMENT: # reverse url mapping points to i18n urls which causes the frontend # tests to fail urlpatterns = i18n_patterns(*i18n_urls) + urls + legacy_urls - -def handler400(request: HttpRequest, exception: Exception) -> HttpResponse: - # (This workaround should become obsolete with Django 2.1; the - # issue was fixed upstream in commit 7ec0fdf62 on 2018-02-14.) - # - # This behaves exactly like the default Django implementation in - # the case where you haven't made a template "400.html", which we - # haven't -- except that it doesn't call `@requires_csrf_token` to - # attempt to set a `csrf_token` variable that the template could - # use if there were a template. We skip @requires_csrf_token - # because that codepath can raise an error on a bad request, which - # is exactly the case we're trying to handle when we get here. - # Bug filed upstream: https://code.djangoproject.com/ticket/28693 - # - # This function is used just because it has this special name in - # the root urls.py file; for more details, see: - # https://docs.djangoproject.com/en/1.11/topics/http/views/#customizing-error-views - return HttpResponseBadRequest( - '

Bad Request (400)

', content_type='text/html')