From b3d5dc6932bf896a909e9871d508654494b34563 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Sun, 3 May 2015 23:08:28 +0100 Subject: [PATCH] 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. --- AUTHORS | 1 + django/contrib/sites/models.py | 18 +++++++++++++---- docs/ref/contrib/sites.txt | 21 ++++++++++++++++---- docs/releases/1.9.txt | 6 +++++- tests/sites_tests/tests.py | 35 ++++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index b25ac1d2f7..aca7fc81eb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -521,6 +521,7 @@ answer newbie questions, and generally made Django that much better: Niall Kelly Nick Efford Nick Lane + Nick Pope Nick Presta Nick Sandford Niclas Olofsson diff --git a/django/contrib/sites/models.py b/django/contrib/sites/models.py index 6471dca136..e22fd4021d 100644 --- a/django/contrib/sites/models.py +++ b/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): """ diff --git a/docs/ref/contrib/sites.txt b/docs/ref/contrib/sites.txt index 4928c92119..da2bd879db 100644 --- a/docs/ref/contrib/sites.txt +++ b/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() ` if the + :setting:`SITE_ID` setting is not defined. + + Both a domain and a port may be returned by :meth:`request.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() ` 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. diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 4b7ef5184a..73ed556d31 100644 --- a/docs/releases/1.9.txt +++ b/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` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/sites_tests/tests.py b/tests/sites_tests/tests.py index 563ee6a681..7d5dcf10bb 100644 --- a/tests/sites_tests/tests.py +++ b/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