diff --git a/django/conf/__init__.py b/django/conf/__init__.py index e9611aa3fe..4fec2ee646 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -74,17 +74,7 @@ class LazySettings(LazyObject): return self._wrapped is not empty -class BaseSettings(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): +class Settings(object): def __init__(self, settings_module): # update this dict from global settings (but only for ALL_CAPS 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. """ diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py index 8187a4e561..e950cb5637 100644 --- a/django/core/checks/urls.py +++ b/django/core/checks/urls.py @@ -90,3 +90,20 @@ def get_warning_for_invalid_pattern(pattern): hint=hint, 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', + ) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index ef6bbcee0e..f8e5522a2b 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -675,3 +675,5 @@ The following checks are performed on your URL configuration: ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances. * **urls.W005**: URL namespace ```` isn't unique. You may not be able to reverse all URLs in this namespace. +* **urls.E006**: The :setting:`MEDIA_URL`/ :setting:`STATIC_URL` setting must + end with a slash. diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py index 656736be82..af3ada5bcc 100644 --- a/tests/admin_scripts/tests.py +++ b/tests/admin_scripts/tests.py @@ -1144,7 +1144,11 @@ class ManageSettingsWithSettingsErrors(AdminScriptTestCase): Test listing available commands output note when only core commands are 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'] out, err = self.run_manage(args) self.assertOutput(out, 'only Django core commands are listed') diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py index 3a52f21d59..b60578cbdb 100644 --- a/tests/check_framework/test_urls.py +++ b/tests/check_framework/test_urls.py @@ -1,14 +1,14 @@ from django.conf import settings from django.core.checks.messages import Warning 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, ) from django.test import SimpleTestCase from django.test.utils import override_settings -class CheckUrlsTest(SimpleTestCase): +class CheckUrlConfigTests(SimpleTestCase): @override_settings(ROOT_URLCONF='check_framework.urls.no_warnings') def test_no_warnings(self): result = check_url_config(None) @@ -116,3 +116,28 @@ class CheckUrlsTest(SimpleTestCase): def test_check_unique_namespaces(self): result = check_url_namespaces_unique(None) 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')]) diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py index 8f50a854e4..bf015affc2 100644 --- a/tests/settings_tests/tests.py +++ b/tests/settings_tests/tests.py @@ -310,84 +310,6 @@ class TestComplexSettingOverride(SimpleTestCase): 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): @override_settings(SECURE_PROXY_SSL_HEADER=None)