p31829507
/
django
mirror of https://github.com/django/django.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fixed #30747 -- Renamed is_safe_url() to url_has_allowed_host_and_scheme().

This commit is contained in:
Carlton Gibson 2019-08-14 17:39:21 +02:00 committed by Mariusz Felisiak
parent 13a8884a08
commit 4f61810751
6 changed files with 80 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
django/contrib/auth/views.py
View File

@ -17,7 +17,9 @@ from django.http import HttpResponseRedirect, QueryDict
from django.shortcuts import resolve_url
from django.urls import reverse_lazy
from django.utils.decorators import method_decorator
from django.utils.http import is_safe_url, urlsafe_base64_decode
from django.utils.http import (
url_has_allowed_host_and_scheme, urlsafe_base64_decode,
)
from django.utils.translation import gettext_lazy as _
from django.views.decorators.cache import never_cache
from django.views.decorators.csrf import csrf_protect
@ -70,7 +72,7 @@ class LoginView(SuccessURLAllowedHostsMixin, FormView):
self.redirect_field_name,
self.request.GET.get(self.redirect_field_name, '')
)
url_is_safe = is_safe_url(
url_is_safe = url_has_allowed_host_and_scheme(
url=redirect_to,
allowed_hosts=self.get_success_url_allowed_hosts(),
require_https=self.request.is_secure(),
@ -138,7 +140,7 @@ class LogoutView(SuccessURLAllowedHostsMixin, TemplateView):
self.redirect_field_name,
self.request.GET.get(self.redirect_field_name)
)
url_is_safe = is_safe_url(
url_is_safe = url_has_allowed_host_and_scheme(
url=next_page,
allowed_hosts=self.get_success_url_allowed_hosts(),
require_https=self.request.is_secure(),

26
django/utils/http.py
View File

@ -294,15 +294,18 @@ def is_same_domain(host, pattern):
)
def is_safe_url(url, allowed_hosts, require_https=False):
def url_has_allowed_host_and_scheme(url, allowed_hosts, 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).
Return ``True`` if the url uses an allowed host and a safe scheme.
Always return ``False`` on an empty url.
If ``require_https`` is ``True``, only 'https' will be considered a valid
scheme, as opposed to 'http' and 'https' with the default, ``False``.
Note: "True" doesn't entail that a URL is "safe". It may still be e.g.
quoted incorrectly. Ensure to also use django.utils.encoding.iri_to_uri()
on the path component of untrusted URLs.
"""
if url is not None:
url = url.strip()
@ -314,8 +317,19 @@ def is_safe_url(url, allowed_hosts, require_https=False):
allowed_hosts = {allowed_hosts}
# 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, allowed_hosts, require_https=require_https) and
_is_safe_url(url.replace('\\', '/'), allowed_hosts, require_https=require_https))
return (
_url_has_allowed_host_and_scheme(url, allowed_hosts, require_https=require_https) and
_url_has_allowed_host_and_scheme(url.replace('\\', '/'), allowed_hosts, require_https=require_https)
)
def is_safe_url(url, allowed_hosts, require_https=False):
warnings.warn(
'django.utils.http.is_safe_url() is deprecated in favor of '
'url_has_allowed_host_and_scheme().',
RemovedInDjango40Warning, stacklevel=2,
)
return url_has_allowed_host_and_scheme(url, allowed_hosts, require_https)
# Copied from urllib.parse.urlparse() but uses fixed urlsplit() function.
@ -367,7 +381,7 @@ def _urlsplit(url, scheme='', allow_fragments=True):
return _coerce_result(v)
def _is_safe_url(url, allowed_hosts, require_https=False):
def _url_has_allowed_host_and_scheme(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('///'):

14
django/views/i18n.py
View File

@ -10,7 +10,7 @@ from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
from django.template import Context, Engine
from django.urls import translate_url
from django.utils.formats import get_format
from django.utils.http import is_safe_url
from django.utils.http import url_has_allowed_host_and_scheme
from django.utils.translation import (
LANGUAGE_SESSION_KEY, check_for_language, get_language,
)
@ -32,11 +32,17 @@ def set_language(request):
any state.
"""
next = request.POST.get('next', request.GET.get('next'))
if ((next or not request.is_ajax()) and
not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure())):
if (
(next or not request.is_ajax()) and
not url_has_allowed_host_and_scheme(
url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure(),
)
):
next = request.META.get('HTTP_REFERER')
next = next and unquote(next) # HTTP_REFERER may be encoded.
if not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure()):
if not url_has_allowed_host_and_scheme(
url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure(),
):
next = '/'
response = HttpResponseRedirect(next) if next else HttpResponse(status=204)
if request.method == 'POST':

2
docs/internals/deprecation.txt
View File

@ -32,6 +32,8 @@ details on these changes.
* ``django.utils.text.unescape_entities()`` will be removed.
* ``django.utils.http.is_safe_url()`` will be removed.
.. _deprecation-removed-in-3.1:
3.1

8
docs/releases/3.0.txt
View File

@ -601,6 +601,14 @@ Miscellaneous
:func:`html.unescape`. Note that unlike ``unescape_entities()``,
``html.unescape()`` evaluates lazy strings immediately.
* To avoid possible confusion as to effective scope, the private internal
utility ``is_safe_url()`` is renamed to
``url_has_allowed_host_and_scheme()``. That a URL has an allowed host and
scheme doesn't in general imply that it's "safe". It may still be quoted
incorrectly, for example. Ensure to also use
:func:`~django.utils.encoding.iri_to_uri` on the path component of untrusted
URLs.
.. _removed-features-3.0:
Features removed in 3.0

47
tests/utils_tests/test_http.py
View File

@ -7,8 +7,8 @@ from django.utils.deprecation import RemovedInDjango40Warning
from django.utils.http import (
base36_to_int, escape_leading_slashes, http_date, int_to_base36,
is_safe_url, is_same_domain, parse_etags, parse_http_date, quote_etag,
urlencode, urlquote, urlquote_plus, urlsafe_base64_decode,
urlsafe_base64_encode, urlunquote, urlunquote_plus,
url_has_allowed_host_and_scheme, urlencode, urlquote, urlquote_plus,
urlsafe_base64_decode, urlsafe_base64_encode, urlunquote, urlunquote_plus,
)
@ -128,7 +128,7 @@ class Base36IntTests(SimpleTestCase):
self.assertEqual(base36_to_int(b36), n)
class IsSafeURLTests(unittest.TestCase):
class IsSafeURLTests(SimpleTestCase):
def test_bad_urls(self):
bad_urls = (
'http://example.com',
@ -164,7 +164,10 @@ class IsSafeURLTests(unittest.TestCase):
)
for bad_url in bad_urls:
with self.subTest(url=bad_url):
self.assertIs(is_safe_url(bad_url, allowed_hosts={'testserver', 'testserver2'}), False)
self.assertIs(
url_has_allowed_host_and_scheme(bad_url, allowed_hosts={'testserver', 'testserver2'}),
False,
)
def test_good_urls(self):
good_urls = (
@ -181,21 +184,27 @@ class IsSafeURLTests(unittest.TestCase):
)
for good_url in good_urls:
with self.subTest(url=good_url):
self.assertIs(is_safe_url(good_url, allowed_hosts={'otherserver', 'testserver'}), True)
self.assertIs(
url_has_allowed_host_and_scheme(good_url, allowed_hosts={'otherserver', 'testserver'}),
True,
)
def test_basic_auth(self):
# Valid basic auth credentials are allowed.
self.assertIs(is_safe_url(r'http://user:pass@testserver/', allowed_hosts={'user:pass@testserver'}), True)
self.assertIs(
url_has_allowed_host_and_scheme(r'http://user:pass@testserver/', allowed_hosts={'user:pass@testserver'}),
True,
)
def test_no_allowed_hosts(self):
# A path without host is allowed.
self.assertIs(is_safe_url('/confirm/me@example.com', allowed_hosts=None), True)
self.assertIs(url_has_allowed_host_and_scheme('/confirm/me@example.com', allowed_hosts=None), True)
# Basic auth without host is not allowed.
self.assertIs(is_safe_url(r'http://testserver\@example.com', allowed_hosts=None), False)
self.assertIs(url_has_allowed_host_and_scheme(r'http://testserver\@example.com', allowed_hosts=None), False)
def test_allowed_hosts_str(self):
self.assertIs(is_safe_url('http://good.com/good', allowed_hosts='good.com'), True)
self.assertIs(is_safe_url('http://good.co/evil', allowed_hosts='good.com'), False)
self.assertIs(url_has_allowed_host_and_scheme('http://good.com/good', allowed_hosts='good.com'), True)
self.assertIs(url_has_allowed_host_and_scheme('http://good.co/evil', allowed_hosts='good.com'), False)
def test_secure_param_https_urls(self):
secure_urls = (
@ -205,7 +214,10 @@ class IsSafeURLTests(unittest.TestCase):
)
for url in secure_urls:
with self.subTest(url=url):
self.assertIs(is_safe_url(url, allowed_hosts={'example.com'}, require_https=True), True)
self.assertIs(
url_has_allowed_host_and_scheme(url, allowed_hosts={'example.com'}, require_https=True),
True,
)
def test_secure_param_non_https_urls(self):
insecure_urls = (
@ -215,7 +227,18 @@ class IsSafeURLTests(unittest.TestCase):
)
for url in insecure_urls:
with self.subTest(url=url):
self.assertIs(is_safe_url(url, allowed_hosts={'example.com'}, require_https=True), False)
self.assertIs(
url_has_allowed_host_and_scheme(url, allowed_hosts={'example.com'}, require_https=True),
False,
)
def test_is_safe_url_deprecated(self):
msg = (
'django.utils.http.is_safe_url() is deprecated in favor of '
'url_has_allowed_host_and_scheme().'
)
with self.assertWarnsMessage(RemovedInDjango40Warning, msg):
is_safe_url('https://example.com', allowed_hosts={'example.com'})
class URLSafeBase64Tests(unittest.TestCase):