Browse Source

Fixed #24834 -- Fixed get_current_site() when Host header contains port.

When the Host header contains a port, looking up the Site record fails
as the host will never match the domain.
Nick Pope 10 years ago
parent
commit
b3d5dc6932
5 changed files with 72 additions and 9 deletions
  1. 1 0
      AUTHORS
  2. 14 4
      django/contrib/sites/models.py
  3. 17 4
      docs/ref/contrib/sites.txt
  4. 5 1
      docs/releases/1.9.txt
  5. 35 0
      tests/sites_tests/tests.py

+ 1 - 0
AUTHORS

@@ -521,6 +521,7 @@ answer newbie questions, and generally made Django that much better:
     Niall Kelly <duke.sam.vimes@gmail.com>
     Nick Efford <nick@efford.org>
     Nick Lane <nick.lane.au@gmail.com>
+    Nick Pope <nick@nickpope.me.uk>
     Nick Presta <nick@nickpresta.ca>
     Nick Sandford <nick.sandford@gmail.com>
     Niclas Olofsson <n@niclasolofsson.se>

+ 14 - 4
django/contrib/sites/models.py

@@ -5,6 +5,7 @@ import string
 from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.db import models
 from django.db.models.signals import pre_delete, pre_save
+from django.http.request import split_domain_port
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext_lazy as _
 
@@ -37,10 +38,19 @@ class SiteManager(models.Manager):
 
     def _get_site_by_request(self, request):
         host = request.get_host()
-        if host not in SITE_CACHE:
-            site = self.get(domain__iexact=host)
-            SITE_CACHE[host] = site
-        return SITE_CACHE[host]
+        try:
+            # First attempt to look up the site by host with or without port.
+            if host not in SITE_CACHE:
+                SITE_CACHE[host] = self.get(domain__iexact=host)
+            return SITE_CACHE[host]
+        except Site.DoesNotExist:
+            # Fallback to looking up site after stripping port from the host.
+            domain, port = split_domain_port(host)
+            if not port:
+                raise
+            if domain not in SITE_CACHE:
+                SITE_CACHE[domain] = self.get(domain__iexact=domain)
+            return SITE_CACHE[domain]
 
     def get_current(self, request=None):
         """

+ 17 - 4
docs/ref/contrib/sites.txt

@@ -495,10 +495,23 @@ Finally, to avoid repetitive fallback code, the framework provides a
     A function that checks if ``django.contrib.sites`` is installed and
     returns either the current :class:`~django.contrib.sites.models.Site`
     object or a :class:`~django.contrib.sites.requests.RequestSite` object
-    based on the request.
+    based on the request. It looks up the current site based on
+    :meth:`request.get_host() <django.http.HttpRequest.get_host>` if the
+    :setting:`SITE_ID` setting is not defined.
+
+    Both a domain and a port may be returned by :meth:`request.get_host()
+    <django.http.HttpRequest.get_host>` when the Host header has a port
+    explicitly specified, e.g. ``example.com:80``. In such cases, if the
+    lookup fails because the host does not match a record in the database,
+    the port is stripped and the lookup is retried with the domain part
+    only. This does not apply to
+    :class:`~django.contrib.sites.requests.RequestSite` which will always
+    use the unmodified host.
 
     .. versionchanged:: 1.8
 
-        This function will now lookup the current site based on
-        :meth:`request.get_host() <django.http.HttpRequest.get_host>` if the
-        :setting:`SITE_ID` setting is not defined.
+        Looking up the current site based on ``request.get_host()`` was added.
+
+    .. versionchanged:: 1.9
+
+        Retrying the lookup with the port stripped was added.

+ 5 - 1
docs/releases/1.9.txt

@@ -208,7 +208,11 @@ Minor features
 :mod:`django.contrib.sites`
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-* ...
+* :func:`~django.contrib.sites.shortcuts.get_current_site` now handles the case
+  where ``request.get_host()`` returns ``domain:port``, e.g.
+  ``example.com:80``. If the lookup fails because the host does not match a
+  record in the database and the host has a port, the port is stripped and the
+  lookup is retried with the domain part only.
 
 :mod:`django.contrib.staticfiles`
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

+ 35 - 0
tests/sites_tests/tests.py

@@ -86,6 +86,41 @@ class SitesFrameworkTests(TestCase):
         site = get_current_site(request)
         self.assertEqual(site.name, "example.com")
 
+    @override_settings(SITE_ID='', ALLOWED_HOSTS=['example.com', 'example.net'])
+    def test_get_current_site_no_site_id_and_handle_port_fallback(self):
+        request = HttpRequest()
+        s1 = self.site
+        s2 = Site.objects.create(domain='example.com:80', name='example.com:80')
+
+        # Host header without port
+        request.META = {'HTTP_HOST': 'example.com'}
+        site = get_current_site(request)
+        self.assertEqual(site, s1)
+
+        # Host header with port - match, no fallback without port
+        request.META = {'HTTP_HOST': 'example.com:80'}
+        site = get_current_site(request)
+        self.assertEqual(site, s2)
+
+        # Host header with port - no match, fallback without port
+        request.META = {'HTTP_HOST': 'example.com:81'}
+        site = get_current_site(request)
+        self.assertEqual(site, s1)
+
+        # Host header with non-matching domain
+        request.META = {'HTTP_HOST': 'example.net'}
+        self.assertRaises(ObjectDoesNotExist, get_current_site, request)
+
+        # Ensure domain for RequestSite always matches host header
+        with self.modify_settings(INSTALLED_APPS={'remove': 'django.contrib.sites'}):
+            request.META = {'HTTP_HOST': 'example.com'}
+            site = get_current_site(request)
+            self.assertEqual(site.name, 'example.com')
+
+            request.META = {'HTTP_HOST': 'example.com:80'}
+            site = get_current_site(request)
+            self.assertEqual(site.name, 'example.com:80')
+
     def test_domain_name_with_whitespaces(self):
         # Regression for #17320
         # Domain names are not allowed contain whitespace characters