mirror of
https://github.com/zulip/zulip.git
synced 2025-11-02 04:53:36 +00:00
errors: Force a super-simpler handler for 400 errors.
This works around a bug in Django in handling the error case of a
client sending an inappropriate HTTP `Host:` header. Various
internal Django machinery expects to be able to casually call
`request.get_host()`, which will attempt to parse that header, so an
exception will be raised. The exception-handling machinery attempts
to catch that exception and just turn it into a 400 response... but
in a certain case, that machinery itself ends up trying to call
`request.get_host()`, and we end up with an uncaught exception that
causes a 500 response, a chain of tracebacks in the logs, and an email
to the server admins. See example below.
That `request.get_host` call comes in the midst of some CSRF-related
middleware, which doesn't even serve any function unless you have a
form in your 400 response page that you want CSRF protection for.
We use the default 400 response page, which is a 26-byte static
HTML error message. So, just send that with no further ado.
Example exception from server logs (lightly edited):
2017-10-08 09:51:50.835 ERR [django.security.DisallowedHost] Invalid HTTP_HOST header: 'example.com'. You may need to add 'example.com' to ALLOWED_HOSTS.
2017-10-08 09:51:50.835 ERR [django.request] Internal Server Error: /loginWithSetCookie
Traceback (most recent call last):
File ".../django/core/handlers/exception.py", line 41, in inner
response = get_response(request)
File ".../django/utils/deprecation.py", line 138, in __call__
response = self.process_request(request)
File ".../django/middleware/common.py", line 57, in process_request
host = request.get_host()
File ".../django/http/request.py", line 113, in get_host
raise DisallowedHost(msg)
django.core.exceptions.DisallowedHost: Invalid HTTP_HOST header: 'example.com'. You may need to add 'example.com' to ALLOWED_HOSTS.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File ".../django/core/handlers/exception.py", line 109, in get_exception_response
response = callback(request, **dict(param_dict, exception=exception))
File ".../django/utils/decorators.py", line 145, in _wrapped_view
result = middleware.process_view(request, view_func, args, kwargs)
File ".../django/middleware/csrf.py", line 276, in process_view
good_referer = request.get_host()
File ".../django/http/request.py", line 113, in get_host
raise DisallowedHost(msg)
django.core.exceptions.DisallowedHost: Invalid HTTP_HOST header: 'example.com'. You may need to add 'example.com' to ALLOWED_HOSTS.
This commit is contained in:
@@ -5,7 +5,7 @@ import os
|
||||
import ujson
|
||||
|
||||
import django.core.urlresolvers
|
||||
from django.test import TestCase
|
||||
from django.test import TestCase, Client
|
||||
from typing import List, Optional
|
||||
|
||||
from zerver.lib.test_classes import ZulipTestCase
|
||||
@@ -143,3 +143,18 @@ class URLResolutionTest(TestCase):
|
||||
if callback_str:
|
||||
(module_name, base_view) = callback_str.rsplit(".", 1)
|
||||
self.check_function_exists(module_name, base_view)
|
||||
|
||||
class ErrorPageTest(TestCase):
|
||||
def test_bogus_http_host(self):
|
||||
# type: () -> None
|
||||
# This tests that we've successfully worked around a certain bug in
|
||||
# Django's exception handling. The enforce_csrf_checks=True,
|
||||
# secure=True, and HTTP_REFERER with an `https:` scheme are all
|
||||
# there to get us down just the right path for Django to blow up
|
||||
# when presented with an HTTP_HOST that's not a valid DNS name.
|
||||
client = Client(enforce_csrf_checks=True)
|
||||
result = client.post('/json/users',
|
||||
secure=True,
|
||||
HTTP_REFERER='https://somewhere',
|
||||
HTTP_HOST='$nonsense')
|
||||
self.assertEqual(result.status_code, 400)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
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
|
||||
@@ -511,3 +512,19 @@ 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, exception):
|
||||
# type: (HttpRequest, Exception) -> HttpResponse
|
||||
# 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.
|
||||
#
|
||||
# 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(
|
||||
'<h1>Bad Request (400)</h1>', content_type='text/html')
|
||||
|
||||
Reference in New Issue
Block a user