From 70123cf084e3af7dfc61bb7bd2090ff802c3cda4 Mon Sep 17 00:00:00 2001 From: Preston Timmons Date: Mon, 2 Mar 2015 13:31:14 -0600 Subject: [PATCH] Fixed #24399 -- Made filesystem loaders use more specific exceptions. --- django/template/loaders/filesystem.py | 6 +++-- django/views/debug.py | 4 --- docs/releases/1.9.txt | 13 +++++++++ tests/template_tests/test_loaders.py | 39 ++++++++++++++++++++++----- tests/view_tests/tests/test_debug.py | 31 --------------------- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/django/template/loaders/filesystem.py b/django/template/loaders/filesystem.py index cb61d576ed8e6..74801efe56c18 100644 --- a/django/template/loaders/filesystem.py +++ b/django/template/loaders/filesystem.py @@ -2,6 +2,7 @@ Wrapper for loading templates from the filesystem. """ +import errno import io from django.core.exceptions import SuspiciousFileOperation @@ -37,6 +38,7 @@ def load_template_source(self, template_name, template_dirs=None): try: with io.open(filepath, encoding=self.engine.file_charset) as fp: return fp.read(), filepath - except IOError: - pass + except IOError as e: + if e.errno != errno.ENOENT: + raise raise TemplateDoesNotExist(template_name) diff --git a/django/views/debug.py b/django/views/debug.py index 80b0e38eb1c80..15da1624934ae 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -269,10 +269,6 @@ def __init__(self, request, exc_type, exc_value, tb, is_email=False): def format_path_status(self, path): if not os.path.exists(path): return "File does not exist" - if not os.path.isfile(path): - return "Not a file" - if not os.access(path, os.R_OK): - return "File is not readable" return "File exists" def get_traceback_data(self): diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 51bbc917d8f6f..cc2273bd56099 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -224,6 +224,19 @@ the keyword argument ``clear=True``. assignment for many-to-many relations and as a consequence now use the new behavior. +Filesystem-based template loaders catch more specific exceptions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When using the :class:`filesystem.Loader ` +or :class:`app_directories.Loader ` +template loaders, earlier versions of Django raised a +:exc:`~django.template.TemplateDoesNotExist` error if a template source existed +but was unreadable. This could happen under many circumstances, such as if +Django didn't have permissions to open the file, or if the template source was +a directory. Now, Django only silences the exception if the template source +does not exist. All other situations result in the original ``IOError`` being +raised. + Miscellaneous ~~~~~~~~~~~~~ diff --git a/tests/template_tests/test_loaders.py b/tests/template_tests/test_loaders.py index 059e37ed3c6f3..e679624ed3dab 100644 --- a/tests/template_tests/test_loaders.py +++ b/tests/template_tests/test_loaders.py @@ -3,6 +3,7 @@ import os.path import sys +import tempfile import types import unittest from contextlib import contextmanager @@ -176,7 +177,16 @@ def test_not_installed(self): class FileSystemLoaderTests(SimpleTestCase): def setUp(self): - self.engine = Engine() + self.engine = Engine(dirs=[TEMPLATE_DIR]) + + @contextmanager + def set_dirs(self, dirs): + original_dirs = self.engine.dirs + self.engine.dirs = dirs + try: + yield + finally: + self.engine.dirs = original_dirs @contextmanager def source_checker(self, dirs): @@ -189,12 +199,8 @@ def check_sources(path, expected_sources): expected_sources, ) - original_dirs = self.engine.dirs - self.engine.dirs = dirs - try: + with self.set_dirs(dirs): yield check_sources - finally: - self.engine.dirs = original_dirs def test_directory_security(self): with self.source_checker(['/dir1', '/dir2']) as check_sources: @@ -238,6 +244,27 @@ def test_case_sensitivity(self): check_sources('index.html', ['/dir1/index.html', '/DIR2/index.html']) check_sources('/DIR1/index.HTML', ['/DIR1/index.HTML']) + def test_file_does_not_exist(self): + with self.assertRaises(TemplateDoesNotExist): + self.engine.get_template('doesnotexist.html') + + @unittest.skipIf( + sys.platform == 'win32', + "Python on Windows doesn't have working os.chmod().", + ) + def test_permissions_error(self): + with tempfile.NamedTemporaryFile() as tmpfile: + tmpdir = os.path.dirname(tmpfile.name) + tmppath = os.path.join(tmpdir, tmpfile.name) + os.chmod(tmppath, 0o0222) + with self.set_dirs([tmpdir]): + with self.assertRaisesMessage(IOError, 'Permission denied'): + self.engine.get_template(tmpfile.name) + + def test_notafile_error(self): + with self.assertRaisesMessage(IOError, 'Is a directory'): + self.engine.get_template('first') + class AppDirectoriesLoaderTest(SimpleTestCase): diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 0d4fb9aa9a0f8..30dd3a90ad10c 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -7,7 +7,6 @@ import inspect import os import re -import shutil import sys import tempfile from unittest import skipIf @@ -152,36 +151,6 @@ def test_template_loader_postmortem(self): response = self.client.get(reverse('raises_template_does_not_exist', kwargs={"path": template_name})) self.assertContains(response, "%s (File does not exist)" % template_path, status_code=500, count=1) - @skipIf(sys.platform == "win32", "Python on Windows doesn't have working os.chmod() and os.access().") - def test_template_loader_postmortem_notreadable(self): - """Tests for not readable file""" - with tempfile.NamedTemporaryFile() as tmpfile: - template_name = tmpfile.name - tempdir = os.path.dirname(tmpfile.name) - template_path = os.path.join(tempdir, template_name) - os.chmod(template_path, 0o0222) - with override_settings(TEMPLATES=[{ - 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'DIRS': [tempdir], - }]): - response = self.client.get(reverse('raises_template_does_not_exist', kwargs={"path": template_name})) - self.assertContains(response, "%s (File is not readable)" % template_path, status_code=500, count=1) - - def test_template_loader_postmortem_notafile(self): - """Tests for not being a file""" - try: - template_path = tempfile.mkdtemp() - template_name = os.path.basename(template_path) - tempdir = os.path.dirname(template_path) - with override_settings(TEMPLATES=[{ - 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'DIRS': [tempdir], - }]): - response = self.client.get(reverse('raises_template_does_not_exist', kwargs={"path": template_name})) - self.assertContains(response, "%s (Not a file)" % template_path, status_code=500, count=1) - finally: - shutil.rmtree(template_path) - def test_no_template_source_loaders(self): """ Make sure if you don't specify a template, the debug view doesn't blow up.