Skip to content

Commit

Permalink
Fixed #24399 -- Made filesystem loaders use more specific exceptions.
Browse files Browse the repository at this point in the history
  • Loading branch information
prestontimmons authored and aaugustin committed Mar 3, 2015
1 parent 85757d0 commit 70123cf
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 43 deletions.
6 changes: 4 additions & 2 deletions django/template/loaders/filesystem.py
Expand Up @@ -2,6 +2,7 @@
Wrapper for loading templates from the filesystem.
"""

import errno
import io

from django.core.exceptions import SuspiciousFileOperation
Expand Down Expand Up @@ -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)
4 changes: 0 additions & 4 deletions django/views/debug.py
Expand Up @@ -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):
Expand Down
13 changes: 13 additions & 0 deletions docs/releases/1.9.txt
Expand Up @@ -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
~~~~~~~~~~~~~

Expand Down
39 changes: 33 additions & 6 deletions tests/template_tests/test_loaders.py
Expand Up @@ -3,6 +3,7 @@

import os.path
import sys
import tempfile
import types
import unittest
from contextlib import contextmanager
Expand Down Expand Up @@ -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):
Expand All @@ -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:
Expand Down Expand Up @@ -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):

Expand Down
31 changes: 0 additions & 31 deletions tests/view_tests/tests/test_debug.py
Expand Up @@ -7,7 +7,6 @@
import inspect
import os
import re
import shutil
import sys
import tempfile
from unittest import skipIf
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 70123cf

Please sign in to comment.