Refs #26956 -- Allowed is_safe_url() to validate against multiple hosts

6 files changed, 48 insertions, 18 deletions

diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py
index cf256d1639..8912c87aee 100644
--- a/django/contrib/auth/views.py
+++ b/django/contrib/auth/views.py
@@ -86,7 +86,7 @@ class LoginView(FormView):
         )
         url_is_safe = is_safe_url(
             url=redirect_to,
-            host=self.request.get_host(),
+            allowed_hosts={self.request.get_host()},
             require_https=self.request.is_secure(),
         )
         if not url_is_safe:
@@ -157,7 +157,7 @@ class LogoutView(TemplateView):
             )
             url_is_safe = is_safe_url(
                 url=next_page,
-                host=self.request.get_host(),
+                allowed_hosts={self.request.get_host()},
                 require_https=self.request.is_secure(),
             )
             # Security check -- don't allow redirection to a different host.
diff --git a/django/utils/http.py b/django/utils/http.py
index f662e395b4..c6285e6f40 100644
--- a/django/utils/http.py
+++ b/django/utils/http.py
@@ -6,12 +6,14 @@ import datetime
 import re
 import sys
 import unicodedata
+import warnings
 from binascii import Error as BinasciiError
 from email.utils import formatdate
 
 from django.core.exceptions import TooManyFieldsSent
 from django.utils import six
 from django.utils.datastructures import MultiValueDict
+from django.utils.deprecation import RemovedInDjango21Warning
 from django.utils.encoding import force_bytes, force_str, force_text
 from django.utils.functional import keep_lazy_text
 from django.utils.six.moves.urllib.parse import (
@@ -277,7 +279,7 @@ def is_same_domain(host, pattern):
     )
 
 
-def is_safe_url(url, host=None, require_https=False):
+def is_safe_url(url, host=None, allowed_hosts=None, require_https=False):
     """
     Return ``True`` if the url is a safe redirection (i.e. it doesn't point to
     a different host and uses a safe scheme).
@@ -296,13 +298,23 @@ def is_safe_url(url, host=None, require_https=False):
             url = force_text(url)
         except UnicodeDecodeError:
             return False
+    if allowed_hosts is None:
+        allowed_hosts = set()
+    if host:
+        warnings.warn(
+            "The host argument is deprecated, use allowed_hosts instead.",
+            RemovedInDjango21Warning,
+            stacklevel=2,
+        )
+        # Avoid mutating the passed in allowed_hosts.
+        allowed_hosts = allowed_hosts | {host}
     # Chrome treats \ completely as / in paths but it could be part of some
     # basic auth credentials so we need to check both URLs.
-    return (_is_safe_url(url, host, require_https=require_https) and
-            _is_safe_url(url.replace('\\', '/'), host, require_https=require_https))
+    return (_is_safe_url(url, allowed_hosts, require_https=require_https) and
+            _is_safe_url(url.replace('\\', '/'), allowed_hosts, require_https=require_https))
 
 
-def _is_safe_url(url, host, require_https=False):
+def _is_safe_url(url, allowed_hosts, require_https=False):
     # Chrome considers any URL with more than two slashes to be absolute, but
     # urlparse is not so flexible. Treat any url with three slashes as unsafe.
     if url.startswith('///'):
@@ -324,7 +336,7 @@ def _is_safe_url(url, host, require_https=False):
     if not url_info.scheme and url_info.netloc:
         scheme = 'http'
     valid_schemes = ['https'] if require_https else ['http', 'https']
-    return ((not url_info.netloc or url_info.netloc == host) and
+    return ((not url_info.netloc or url_info.netloc in allowed_hosts) and
             (not scheme or scheme in valid_schemes))
 
 
diff --git a/django/views/i18n.py b/django/views/i18n.py
index 5cc6a3ebe1..eda8422f69 100644
--- a/django/views/i18n.py
+++ b/django/views/i18n.py
@@ -38,11 +38,11 @@ def set_language(request):
     """
     next = request.POST.get('next', request.GET.get('next'))
     if ((next or not request.is_ajax()) and
-            not is_safe_url(url=next, host=request.get_host(), require_https=request.is_secure())):
+            not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure())):
         next = request.META.get('HTTP_REFERER')
         if next:
             next = urlunquote(next)  # HTTP_REFERER may be encoded.
-        if not is_safe_url(url=next, host=request.get_host(), require_https=request.is_secure()):
+        if not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure()):
             next = '/'
     response = http.HttpResponseRedirect(next) if next else http.HttpResponse(status=204)
     if request.method == 'POST':
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index b983ae1ace..5d93915dc6 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -30,6 +30,9 @@ details on these changes.
 * ``django.core.cache.backends.memcached.PyLibMCCache`` will no longer support
   passing ``pylibmc`` behavior settings as top-level attributes of ``OPTIONS``.
 
+* The ``host`` parameter of ``django.utils.http.is_safe_url()`` will be
+  removed.
+
 .. _deprecation-removed-in-2.0:
 
 2.0
diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt
index 46bc7f1529..a202232cb6 100644
--- a/docs/releases/1.11.txt
+++ b/docs/releases/1.11.txt
@@ -520,3 +520,6 @@ Miscellaneous
 * For the ``PyLibMCCache`` cache backend, passing ``pylibmc`` behavior settings
   as top-level attributes of ``OPTIONS`` is deprecated. Set them under a
   ``behaviors`` key within ``OPTIONS`` instead.
+
+* The ``host`` parameter of ``django.utils.http.is_safe_url()`` is deprecated
+  in favor of the new ``allowed_hosts`` parameter.
diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py
index b690055f30..e8b4e51eb3 100644
--- a/tests/utils_tests/test_http.py
+++ b/tests/utils_tests/test_http.py
@@ -5,8 +5,10 @@ import sys
 import unittest
 from datetime import datetime
 
+from django.test import ignore_warnings
 from django.utils import http, six
 from django.utils.datastructures import MultiValueDict
+from django.utils.deprecation import RemovedInDjango21Warning
 
 
 class TestUtilsHttp(unittest.TestCase):
@@ -107,7 +109,12 @@ class TestUtilsHttp(unittest.TestCase):
             '\n',
         )
         for bad_url in bad_urls:
-            self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
+            with ignore_warnings(category=RemovedInDjango21Warning):
+                self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
+            self.assertFalse(
+                http.is_safe_url(bad_url, allowed_hosts={'testserver', 'testserver2'}),
+                "%s should be blocked" % bad_url,
+            )
 
         good_urls = (
             '/view/?param=http://example.com',
@@ -121,20 +128,25 @@ class TestUtilsHttp(unittest.TestCase):
             '/url%20with%20spaces/',
         )
         for good_url in good_urls:
-            self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
+            with ignore_warnings(category=RemovedInDjango21Warning):
+                self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
+            self.assertTrue(
+                http.is_safe_url(good_url, allowed_hosts={'otherserver', 'testserver'}),
+                "%s should be allowed" % good_url,
+            )
 
         if six.PY2:
             # Check binary URLs, regression tests for #26308
             self.assertTrue(
-                http.is_safe_url(b'https://testserver/', host='testserver'),
+                http.is_safe_url(b'https://testserver/', allowed_hosts={'testserver'}),
                 "binary URLs should be allowed on Python 2"
             )
-            self.assertFalse(http.is_safe_url(b'\x08//example.com', host='testserver'))
-            self.assertTrue(http.is_safe_url('àview/'.encode('utf-8'), host='testserver'))
-            self.assertFalse(http.is_safe_url('àview'.encode('latin-1'), host='testserver'))
+            self.assertFalse(http.is_safe_url(b'\x08//example.com', allowed_hosts={'testserver'}))
+            self.assertTrue(http.is_safe_url('àview/'.encode('utf-8'), allowed_hosts={'testserver'}))
+            self.assertFalse(http.is_safe_url('àview'.encode('latin-1'), allowed_hosts={'testserver'}))
 
         # Valid basic auth credentials are allowed.
-        self.assertTrue(http.is_safe_url(r'http://user:pass@testserver/', host='user:pass@testserver'))
+        self.assertTrue(http.is_safe_url(r'http://user:pass@testserver/', allowed_hosts={'user:pass@testserver'}))
         # A path without host is allowed.
         self.assertTrue(http.is_safe_url('/confirm/me@example.com'))
         # Basic auth without host is not allowed.
@@ -147,7 +159,7 @@ class TestUtilsHttp(unittest.TestCase):
             '/view/?param=http://example.com',
         )
         for url in secure_urls:
-            self.assertTrue(http.is_safe_url(url, 'example.com', require_https=True))
+            self.assertTrue(http.is_safe_url(url, allowed_hosts={'example.com'}, require_https=True))
 
     def test_is_safe_url_secure_param_non_https_urls(self):
         not_secure_urls = (
@@ -156,7 +168,7 @@ class TestUtilsHttp(unittest.TestCase):
             '//example.com/p',
         )
         for url in not_secure_urls:
-            self.assertFalse(http.is_safe_url(url, 'example.com', require_https=True))
+            self.assertFalse(http.is_safe_url(url, allowed_hosts={'example.com'}, require_https=True))
 
     def test_urlsafe_base64_roundtrip(self):
         bytestring = b'foo'