Permalink
Browse files

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.
  • Loading branch information...
gnprice authored and timabbott committed Oct 10, 2017
1 parent 8828e96 commit 55426894cd0d430b90c72f2bf0880154c19838bc
Showing with 33 additions and 1 deletion.
  1. +16 −1 zerver/tests/test_urls.py
  2. +17 −0 zproject/urls.py
View
@@ -5,7 +5,7 @@
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 @@ def test_non_api_url_resolution(self):
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)
View
@@ -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 @@
# 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')

0 comments on commit 5542689

Please sign in to comment.