Browse Source

Refs #26666 -- Added ALLOWED_HOSTS validation when running tests.

Also used ALLOWED_HOSTS to check for external hosts in assertRedirects().
Tobias McNulty 8 years ago
parent
commit
17e661641d

+ 13 - 4
django/test/testcases.py

@@ -29,6 +29,7 @@ from django.core.servers.basehttp import WSGIRequestHandler, WSGIServer
 from django.db import DEFAULT_DB_ALIAS, connection, connections, transaction
 from django.forms.fields import CharField
 from django.http import QueryDict
+from django.http.request import split_domain_port, validate_host
 from django.test.client import Client
 from django.test.html import HTMLParseError, parse_html
 from django.test.signals import setting_changed, template_rendered
@@ -306,10 +307,13 @@ class SimpleTestCase(unittest.TestCase):
                 # netloc might be empty, or in cases where Django tests the
                 # HTTP scheme, the convention is for netloc to be 'testserver'.
                 # Trust both as "internal" URLs here.
-                if netloc and netloc != 'testserver':
+                domain, port = split_domain_port(netloc)
+                if domain and not validate_host(domain, settings.ALLOWED_HOSTS):
                     raise ValueError(
-                        "The Django test client is unable to fetch remote URLs (got %s). "
-                        "Use assertRedirects(..., fetch_redirect_response=False) instead." % url
+                        "The test client is unable to fetch remote URLs (got %s). "
+                        "If the host is served by Django, add '%s' to ALLOWED_HOSTS. "
+                        "Otherwise, use assertRedirects(..., fetch_redirect_response=False)."
+                        % (url, domain)
                     )
                 redirect_response = response.client.get(path, QueryDict(query), secure=(scheme == 'https'))
 
@@ -1324,9 +1328,12 @@ class LiveServerTestCase(TransactionTestCase):
                 conn.allow_thread_sharing = True
                 connections_override[conn.alias] = conn
 
-        # Launch the live server's thread
         specified_address = os.environ.get(
             'DJANGO_LIVE_TEST_SERVER_ADDRESS', 'localhost:8081-8179')
+        cls._live_server_modified_settings = modify_settings(
+            ALLOWED_HOSTS={'append': specified_address.split(':')[0]},
+        )
+        cls._live_server_modified_settings.enable()
 
         # The specified ports may be of the form '8000-8010,8080,9200-9300'
         # i.e. a comma-separated list of ports or ranges of ports, so we break
@@ -1348,6 +1355,7 @@ class LiveServerTestCase(TransactionTestCase):
         except Exception:
             msg = 'Invalid address ("%s") for live server.' % specified_address
             six.reraise(ImproperlyConfigured, ImproperlyConfigured(msg), sys.exc_info()[2])
+        # Launch the live server's thread
         cls.server_thread = cls._create_server_thread(host, possible_ports, connections_override)
         cls.server_thread.daemon = True
         cls.server_thread.start()
@@ -1386,6 +1394,7 @@ class LiveServerTestCase(TransactionTestCase):
     @classmethod
     def tearDownClass(cls):
         cls._tearDownClassInternal()
+        cls._live_server_modified_settings.disable()
         super(LiveServerTestCase, cls).tearDownClass()
 
 

+ 2 - 1
django/test/utils.py

@@ -111,7 +111,8 @@ def setup_test_environment():
     settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend'
 
     request._original_allowed_hosts = settings.ALLOWED_HOSTS
-    settings.ALLOWED_HOSTS = ['*']
+    # Add the default host of the test client.
+    settings.ALLOWED_HOSTS = settings.ALLOWED_HOSTS + ['testserver']
 
     mail.outbox = []
 

+ 7 - 3
docs/ref/settings.txt

@@ -90,14 +90,18 @@ If the ``Host`` header (or ``X-Forwarded-Host`` if
 list, the :meth:`django.http.HttpRequest.get_host()` method will raise
 :exc:`~django.core.exceptions.SuspiciousOperation`.
 
-When :setting:`DEBUG` is ``True`` or when running tests, host validation is
-disabled; any host will be accepted. Thus it's usually only necessary to set it
-in production.
+When :setting:`DEBUG` is ``True``, host validation is disabled; any host will
+be accepted. ``ALLOWED_HOSTS`` is :ref:`checked when running tests
+<topics-testing-advanced-multiple-hosts>`.
 
 This validation only applies via :meth:`~django.http.HttpRequest.get_host()`;
 if your code accesses the ``Host`` header directly from ``request.META`` you
 are bypassing this security protection.
 
+.. versionchanged:: 1.11
+
+    In older versions, ``ALLOWED_HOSTS`` wasn't checked when running tests.
+
 .. setting:: APPEND_SLASH
 
 ``APPEND_SLASH``

+ 5 - 0
docs/releases/1.11.txt

@@ -262,6 +262,11 @@ Miscellaneous
 * CSRF failures are logged to the ``django.security.csrf ``` logger instead of
   ``django.request``.
 
+* :setting:`ALLOWED_HOSTS` validation is no longer disabled when running tests.
+  If your application includes tests with custom host names, you must include
+  those host names in :setting:`ALLOWED_HOSTS`. See
+  :ref:`topics-testing-advanced-multiple-hosts`.
+
 * Using a foreign key's id (e.g. ``'field_id'``) in ``ModelAdmin.list_display``
   displays the related object's ID. Remove the ``_id`` suffix if you want the
   old behavior of the string representation of the object.

+ 1 - 0
docs/spelling_wordlist

@@ -498,6 +498,7 @@ multiline
 multilinestring
 multipart
 multipolygon
+multitenancy
 multithreaded
 multithreading
 Mumbai

+ 55 - 0
docs/topics/testing/advanced.txt

@@ -67,6 +67,61 @@ The following is a simple unit test using the request factory::
             response = MyView.as_view()(request)
             self.assertEqual(response.status_code, 200)
 
+.. _topics-testing-advanced-multiple-hosts:
+
+Tests and multiple host names
+=============================
+
+The :setting:`ALLOWED_HOSTS` setting is validated when running tests. This
+allows the test client to differentiate between internal and external URLs.
+
+Projects that support multitenancy or otherwise alter business logic based on
+the request's host and use custom host names in tests must include those hosts
+in :setting:`ALLOWED_HOSTS`.
+
+The first and simplest option to do so is to add the hosts to your settings
+file. For example, the test suite for docs.djangoproject.com includes the
+following::
+
+    from django.test import TestCase
+
+    class SearchFormTestCase(TestCase):
+        def test_empty_get(self):
+            response = self.client.get('/en/dev/search/', HTTP_HOST='docs.djangoproject.dev:8000')
+            self.assertEqual(response.status_code, 200)
+
+and the settings file includes a list of the domains supported by the project::
+
+    ALLOWED_HOSTS = [
+        'www.djangoproject.dev',
+        'docs.djangoproject.dev',
+        ...
+    ]
+
+Another option is to add the required hosts to :setting:`ALLOWED_HOSTS` using
+:meth:`~django.test.override_settings()` or
+:attr:`~django.test.SimpleTestCase.modify_settings()`. This option may be
+preferable in standalone apps that can't package their own settings file or
+for projects where the list of domains is not static (e.g., subdomains for
+multitenancy). For example, you could write a test for the domain
+``http://otherserver/`` as follows::
+
+    from django.test import TestCase, override_settings
+
+    class MultiDomainTestCase(TestCase):
+        @override_settings(ALLOWED_HOSTS=['otherserver'])
+        def test_other_domain(self):
+            response = self.client.get('http://otherserver/foo/bar/')
+
+Disabling :setting:`ALLOWED_HOSTS` checking (``ALLOWED_HOSTS = ['*']``) when
+running tests prevents the test client from raising a helpful error message if
+you follow a redirect to an external URL.
+
+.. versionchanged:: 1.11
+
+    Older versions didn't validate ``ALLOWED_HOSTS`` while testing so these
+    techniques weren't necessary.
+
 .. _topics-testing-advanced-multidb:
 
 Tests and multiple databases

+ 1 - 0
tests/cache/tests.py

@@ -1393,6 +1393,7 @@ class DefaultNonExpiringCacheKeyTests(SimpleTestCase):
         },
     },
     USE_I18N=False,
+    ALLOWED_HOSTS=['.example.com'],
 )
 class CacheUtils(SimpleTestCase):
     """TestCase for django.utils.cache functions."""

+ 4 - 1
tests/sites_tests/tests.py

@@ -142,6 +142,7 @@ class SitesFrameworkTests(TestCase):
         with self.assertRaises(ValidationError):
             site.full_clean()
 
+    @override_settings(ALLOWED_HOSTS=['example.com'])
     def test_clear_site_cache(self):
         request = HttpRequest()
         request.META = {
@@ -162,7 +163,7 @@ class SitesFrameworkTests(TestCase):
         clear_site_cache(Site, instance=self.site, using='default')
         self.assertEqual(models.SITE_CACHE, {})
 
-    @override_settings(SITE_ID='')
+    @override_settings(SITE_ID='', ALLOWED_HOSTS=['example2.com'])
     def test_clear_site_cache_domain(self):
         site = Site.objects.create(name='example2.com', domain='example2.com')
         request = HttpRequest()
@@ -191,6 +192,7 @@ class SitesFrameworkTests(TestCase):
         self.assertEqual(Site.objects.get_by_natural_key(self.site.domain), self.site)
         self.assertEqual(self.site.natural_key(), (self.site.domain,))
 
+    @override_settings(ALLOWED_HOSTS=['example.com'])
     def test_requestsite_save_notimplemented_msg(self):
         # Test response msg for RequestSite.save NotImplementedError
         request = HttpRequest()
@@ -201,6 +203,7 @@ class SitesFrameworkTests(TestCase):
         with self.assertRaisesMessage(NotImplementedError, msg):
             RequestSite(request).save()
 
+    @override_settings(ALLOWED_HOSTS=['example.com'])
     def test_requestsite_delete_notimplemented_msg(self):
         # Test response msg for RequestSite.delete NotImplementedError
         request = HttpRequest()

+ 1 - 1
tests/staticfiles_tests/test_liveserver.py

@@ -35,9 +35,9 @@ class LiveServerBase(StaticLiveServerTestCase):
 
     @classmethod
     def tearDownClass(cls):
+        super(LiveServerBase, cls).tearDownClass()
         # Restore original settings
         cls.settings_override.disable()
-        super(LiveServerBase, cls).tearDownClass()
 
 
 class StaticLiveServerChecks(LiveServerBase):

+ 7 - 1
tests/test_client/tests.py

@@ -601,7 +601,13 @@ class ClientTest(TestCase):
         a relevant ValueError rather than a non-descript AssertionError.
         """
         response = self.client.get('/django_project_redirect/')
-        with self.assertRaisesMessage(ValueError, 'unable to fetch'):
+        msg = (
+            "The test client is unable to fetch remote URLs (got "
+            "https://www.djangoproject.com/). If the host is served by Django, "
+            "add 'www.djangoproject.com' to ALLOWED_HOSTS. "
+            "Otherwise, use assertRedirects(..., fetch_redirect_response=False)."
+        )
+        with self.assertRaisesMessage(ValueError, msg):
             self.assertRedirects(response, 'https://www.djangoproject.com/')
 
     def test_session_modifying_view(self):

+ 9 - 1
tests/test_client_regress/tests.py

@@ -15,7 +15,8 @@ from django.template import (
 )
 from django.template.response import SimpleTemplateResponse
 from django.test import (
-    Client, SimpleTestCase, TestCase, ignore_warnings, override_settings,
+    Client, SimpleTestCase, TestCase, ignore_warnings, modify_settings,
+    override_settings,
 )
 from django.test.client import RedirectCycleError, RequestFactory, encode_file
 from django.test.utils import ContextList, str_prefix
@@ -455,6 +456,7 @@ class AssertRedirectsTests(SimpleTestCase):
         self.assertRedirects(response, '/no_template_view/', 302, 200)
         self.assertEqual(len(response.redirect_chain), 3)
 
+    @modify_settings(ALLOWED_HOSTS={'append': 'otherserver'})
     def test_redirect_to_different_host(self):
         "The test client will preserve scheme, host and port changes"
         response = self.client.get('/redirect_other_host/', follow=True)
@@ -467,6 +469,12 @@ class AssertRedirectsTests(SimpleTestCase):
         self.assertEqual(response.request.get('wsgi.url_scheme'), 'https')
         self.assertEqual(response.request.get('SERVER_NAME'), 'otherserver')
         self.assertEqual(response.request.get('SERVER_PORT'), '8443')
+        # assertRedirects() can follow redirect to 'otherserver' too.
+        response = self.client.get('/redirect_other_host/', follow=False)
+        self.assertRedirects(
+            response, 'https://otherserver:8443/no_template_view/',
+            status_code=302, target_status_code=200
+        )
 
     def test_redirect_chain_on_non_redirect_page(self):
         "An assertion is raised if the original page couldn't be retrieved as expected"