Fixed #27626 -- Moved MEDIA_URL/STATIC_URL validation to a system check.

This commit is contained in:
Adam Chainz 2016-12-23 15:55:00 +00:00 committed by Tim Graham
parent 8d94d575f8
commit 8669cf0e68
6 changed files with 53 additions and 93 deletions

View File

@ -74,17 +74,7 @@ class LazySettings(LazyObject):
return self._wrapped is not empty return self._wrapped is not empty
class BaseSettings(object): class Settings(object):
"""
Common logic for settings whether set by a module or by the user.
"""
def __setattr__(self, name, value):
if name in ("MEDIA_URL", "STATIC_URL") and value and not value.endswith('/'):
raise ImproperlyConfigured("If set, %s must end with a slash" % name)
object.__setattr__(self, name, value)
class Settings(BaseSettings):
def __init__(self, settings_module): def __init__(self, settings_module):
# update this dict from global settings (but only for ALL_CAPS settings) # update this dict from global settings (but only for ALL_CAPS settings)
for setting in dir(global_settings): for setting in dir(global_settings):
@ -137,7 +127,7 @@ class Settings(BaseSettings):
} }
class UserSettingsHolder(BaseSettings): class UserSettingsHolder(object):
""" """
Holder for user configured settings. Holder for user configured settings.
""" """

View File

@ -90,3 +90,20 @@ def get_warning_for_invalid_pattern(pattern):
hint=hint, hint=hint,
id="urls.E004", id="urls.E004",
)] )]
@register(Tags.urls)
def check_url_settings(app_configs, **kwargs):
errors = []
for name in ('STATIC_URL', 'MEDIA_URL'):
value = getattr(settings, name)
if value and not value.endswith('/'):
errors.append(E006(name))
return errors
def E006(name):
return Error(
'The {} setting must end with a slash.'.format(name),
id='urls.E006',
)

View File

@ -675,3 +675,5 @@ The following checks are performed on your URL configuration:
``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances. ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances.
* **urls.W005**: URL namespace ``<namespace>`` isn't unique. You may not be * **urls.W005**: URL namespace ``<namespace>`` isn't unique. You may not be
able to reverse all URLs in this namespace. able to reverse all URLs in this namespace.
* **urls.E006**: The :setting:`MEDIA_URL`/ :setting:`STATIC_URL` setting must
end with a slash.

View File

@ -1144,7 +1144,11 @@ class ManageSettingsWithSettingsErrors(AdminScriptTestCase):
Test listing available commands output note when only core commands are Test listing available commands output note when only core commands are
available. available.
""" """
self.write_settings('settings.py', sdict={'MEDIA_URL': '"/no_ending_slash"'}) self.write_settings(
'settings.py',
extra='from django.core.exceptions import ImproperlyConfigured\n'
'raise ImproperlyConfigured()',
)
args = ['help'] args = ['help']
out, err = self.run_manage(args) out, err = self.run_manage(args)
self.assertOutput(out, 'only Django core commands are listed') self.assertOutput(out, 'only Django core commands are listed')

View File

@ -1,14 +1,14 @@
from django.conf import settings from django.conf import settings
from django.core.checks.messages import Warning from django.core.checks.messages import Warning
from django.core.checks.urls import ( from django.core.checks.urls import (
check_url_config, check_url_namespaces_unique, E006, check_url_config, check_url_namespaces_unique, check_url_settings,
get_warning_for_invalid_pattern, get_warning_for_invalid_pattern,
) )
from django.test import SimpleTestCase from django.test import SimpleTestCase
from django.test.utils import override_settings from django.test.utils import override_settings
class CheckUrlsTest(SimpleTestCase): class CheckUrlConfigTests(SimpleTestCase):
@override_settings(ROOT_URLCONF='check_framework.urls.no_warnings') @override_settings(ROOT_URLCONF='check_framework.urls.no_warnings')
def test_no_warnings(self): def test_no_warnings(self):
result = check_url_config(None) result = check_url_config(None)
@ -116,3 +116,28 @@ class CheckUrlsTest(SimpleTestCase):
def test_check_unique_namespaces(self): def test_check_unique_namespaces(self):
result = check_url_namespaces_unique(None) result = check_url_namespaces_unique(None)
self.assertEqual(result, []) self.assertEqual(result, [])
class CheckURLSettingsTests(SimpleTestCase):
@override_settings(STATIC_URL='a/', MEDIA_URL='b/')
def test_slash_no_errors(self):
self.assertEqual(check_url_settings(None), [])
@override_settings(STATIC_URL='', MEDIA_URL='')
def test_empty_string_no_errors(self):
self.assertEqual(check_url_settings(None), [])
@override_settings(STATIC_URL='noslash')
def test_static_url_no_slash(self):
self.assertEqual(check_url_settings(None), [E006('STATIC_URL')])
@override_settings(STATIC_URL='slashes//')
def test_static_url_double_slash_allowed(self):
# The check allows for a double slash, presuming the user knows what
# they are doing.
self.assertEqual(check_url_settings(None), [])
@override_settings(MEDIA_URL='noslash')
def test_media_url_no_slash(self):
self.assertEqual(check_url_settings(None), [E006('MEDIA_URL')])

View File

@ -310,84 +310,6 @@ class TestComplexSettingOverride(SimpleTestCase):
self.assertEqual(str(w[0].message), 'Overriding setting TEST_WARN can lead to unexpected behavior.') self.assertEqual(str(w[0].message), 'Overriding setting TEST_WARN can lead to unexpected behavior.')
class TrailingSlashURLTests(SimpleTestCase):
"""
Tests for the MEDIA_URL and STATIC_URL settings.
They must end with a slash to ensure there's a deterministic way to build
paths in templates.
"""
settings_module = settings
def setUp(self):
self._original_media_url = self.settings_module.MEDIA_URL
self._original_static_url = self.settings_module.STATIC_URL
def tearDown(self):
self.settings_module.MEDIA_URL = self._original_media_url
self.settings_module.STATIC_URL = self._original_static_url
def test_blank(self):
"""
The empty string is accepted, even though it doesn't end in a slash.
"""
self.settings_module.MEDIA_URL = ''
self.assertEqual('', self.settings_module.MEDIA_URL)
self.settings_module.STATIC_URL = ''
self.assertEqual('', self.settings_module.STATIC_URL)
def test_end_slash(self):
"""
It works if the value ends in a slash.
"""
self.settings_module.MEDIA_URL = '/foo/'
self.assertEqual('/foo/', self.settings_module.MEDIA_URL)
self.settings_module.MEDIA_URL = 'http://media.foo.com/'
self.assertEqual('http://media.foo.com/', self.settings_module.MEDIA_URL)
self.settings_module.STATIC_URL = '/foo/'
self.assertEqual('/foo/', self.settings_module.STATIC_URL)
self.settings_module.STATIC_URL = 'http://static.foo.com/'
self.assertEqual('http://static.foo.com/', self.settings_module.STATIC_URL)
def test_no_end_slash(self):
"""
An ImproperlyConfigured exception is raised if the value doesn't end
in a slash.
"""
with self.assertRaises(ImproperlyConfigured):
self.settings_module.MEDIA_URL = '/foo'
with self.assertRaises(ImproperlyConfigured):
self.settings_module.MEDIA_URL = 'http://media.foo.com'
with self.assertRaises(ImproperlyConfigured):
self.settings_module.STATIC_URL = '/foo'
with self.assertRaises(ImproperlyConfigured):
self.settings_module.STATIC_URL = 'http://static.foo.com'
def test_double_slash(self):
"""
If the value ends in more than one slash, presume they know what
they're doing.
"""
self.settings_module.MEDIA_URL = '/wrong//'
self.assertEqual('/wrong//', self.settings_module.MEDIA_URL)
self.settings_module.MEDIA_URL = 'http://media.foo.com/wrong//'
self.assertEqual('http://media.foo.com/wrong//', self.settings_module.MEDIA_URL)
self.settings_module.STATIC_URL = '/wrong//'
self.assertEqual('/wrong//', self.settings_module.STATIC_URL)
self.settings_module.STATIC_URL = 'http://static.foo.com/wrong//'
self.assertEqual('http://static.foo.com/wrong//', self.settings_module.STATIC_URL)
class SecureProxySslHeaderTest(SimpleTestCase): class SecureProxySslHeaderTest(SimpleTestCase):
@override_settings(SECURE_PROXY_SSL_HEADER=None) @override_settings(SECURE_PROXY_SSL_HEADER=None)