From 70123cf084e3af7dfc61bb7bd2090ff802c3cda4 Mon Sep 17 00:00:00 2001
From: Preston Timmons <prestontimmons@gmail.com>
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 cb61d576ed..74801efe56 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 @@ class Loader(BaseLoader):
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 80b0e38eb1..15da162493 100644
--- a/django/views/debug.py
+++ b/django/views/debug.py
@@ -269,10 +269,6 @@ class ExceptionReporter(object):
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 51bbc917d8..cc2273bd56 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 <django.template.loaders.filesystem.Loader>`
+or :class:`app_directories.Loader <django.template.loaders.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 059e37ed3c..e679624ed3 100644
--- a/tests/template_tests/test_loaders.py
+++ b/tests/template_tests/test_loaders.py
@@ -3,6 +3,7 @@ from __future__ import unicode_literals
import os.path
import sys
+import tempfile
import types
import unittest
from contextlib import contextmanager
@@ -176,7 +177,16 @@ class EggLoaderTests(SimpleTestCase):
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 @@ class FileSystemLoaderTests(SimpleTestCase):
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 @@ class FileSystemLoaderTests(SimpleTestCase):
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 0d4fb9aa9a..30dd3a90ad 100644
--- a/tests/view_tests/tests/test_debug.py
+++ b/tests/view_tests/tests/test_debug.py
@@ -7,7 +7,6 @@ import importlib
import inspect
import os
import re
-import shutil
import sys
import tempfile
from unittest import skipIf
@@ -152,36 +151,6 @@ class DebugViewTests(TestCase):
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.