summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAymeric Augustin <aymeric.augustin@m4x.org>2014-11-11 18:59:49 +0100
committerAymeric Augustin <aymeric.augustin@m4x.org>2014-11-11 19:05:14 +0100
commitb8ba73cd0cb6a3dbdaeb3df65936970956829de3 (patch)
tree6a21c861d3461a6af30358c7b0437c662c33cc5c
parent40ba6f21bb5a728987da9a7055c74fa3c65da4db (diff)
Raised SuspiciousFileOperation in safe_join.
Added a test for the condition safe_join is designed to prevent. Previously, a generic ValueError was raised. It was impossible to tell an intentional exception raised to implement safe_join's contract from an unintentional exception caused by incorrect inputs or unexpected conditions. That resulted in bizarre exception catching patterns, which this patch removes. Since safe_join is a private API and since the change is unlikely to create security issues for users who use it anyway -- at worst, an uncaught SuspiciousFileOperation exception will bubble up -- it isn't documented.
-rw-r--r--django/core/files/storage.py6
-rw-r--r--django/template/loaders/app_directories.py9
-rw-r--r--django/template/loaders/filesystem.py11
-rw-r--r--django/utils/_os.py6
-rw-r--r--tests/utils_tests/test_os_utils.py5
5 files changed, 18 insertions, 19 deletions
diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index cff2d06f1a..7a9bc06665 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -277,11 +277,7 @@ class FileSystemStorage(Storage):
return directories, files
def path(self, name):
- try:
- path = safe_join(self.location, name)
- except ValueError:
- raise SuspiciousFileOperation("Attempted access to '%s' denied." % name)
- return os.path.normpath(path)
+ return safe_join(self.location, name)
def size(self, name):
return os.path.getsize(self.path(name))
diff --git a/django/template/loaders/app_directories.py b/django/template/loaders/app_directories.py
index b66adaed08..cab1edf8cc 100644
--- a/django/template/loaders/app_directories.py
+++ b/django/template/loaders/app_directories.py
@@ -8,6 +8,7 @@ import sys
from django.apps import apps
from django.conf import settings
+from django.core.exceptions import SuspiciousFileOperation
from django.template.base import TemplateDoesNotExist
from django.template.loader import BaseLoader
from django.utils._os import safe_join
@@ -47,11 +48,9 @@ class Loader(BaseLoader):
for template_dir in template_dirs:
try:
yield safe_join(template_dir, template_name)
- except UnicodeDecodeError:
- # The template dir name was a bytestring that wasn't valid UTF-8.
- raise
- except ValueError:
- # The joined path was located outside of template_dir.
+ except SuspiciousFileOperation:
+ # The joined path was located outside of this template_dir
+ # (it might be inside another one, so this isn't fatal).
pass
def load_template_source(self, template_name, template_dirs=None):
diff --git a/django/template/loaders/filesystem.py b/django/template/loaders/filesystem.py
index 52e41ef0b9..dc7de84abf 100644
--- a/django/template/loaders/filesystem.py
+++ b/django/template/loaders/filesystem.py
@@ -3,6 +3,7 @@ Wrapper for loading templates from the filesystem.
"""
from django.conf import settings
+from django.core.exceptions import SuspiciousFileOperation
from django.template.base import TemplateDoesNotExist
from django.template.loader import BaseLoader
from django.utils._os import safe_join
@@ -22,13 +23,9 @@ class Loader(BaseLoader):
for template_dir in template_dirs:
try:
yield safe_join(template_dir, template_name)
- except UnicodeDecodeError:
- # The template dir name was a bytestring that wasn't valid UTF-8.
- raise
- except ValueError:
- # The joined path was located outside of this particular
- # template_dir (it might be inside another one, so this isn't
- # fatal).
+ except SuspiciousFileOperation:
+ # The joined path was located outside of this template_dir
+ # (it might be inside another one, so this isn't fatal).
pass
def load_template_source(self, template_name, template_dirs=None):
diff --git a/django/utils/_os.py b/django/utils/_os.py
index 1d7ddf619e..bcfe3de636 100644
--- a/django/utils/_os.py
+++ b/django/utils/_os.py
@@ -4,6 +4,7 @@ import sys
import tempfile
from os.path import join, normcase, normpath, abspath, isabs, sep, dirname
+from django.core.exceptions import SuspiciousFileOperation
from django.utils.encoding import force_text
from django.utils import six
@@ -77,8 +78,9 @@ def safe_join(base, *paths):
if (not normcase(final_path).startswith(normcase(base_path + sep)) and
normcase(final_path) != normcase(base_path) and
dirname(normcase(base_path)) != normcase(base_path)):
- raise ValueError('The joined path (%s) is located outside of the base '
- 'path component (%s)' % (final_path, base_path))
+ raise SuspiciousFileOperation(
+ 'The joined path ({}) is located outside of the base path '
+ 'component ({})'.format(final_path, base_path))
return final_path
diff --git a/tests/utils_tests/test_os_utils.py b/tests/utils_tests/test_os_utils.py
index da54e93ac0..7e6c74b3da 100644
--- a/tests/utils_tests/test_os_utils.py
+++ b/tests/utils_tests/test_os_utils.py
@@ -1,6 +1,7 @@
import os
import unittest
+from django.core.exceptions import SuspiciousFileOperation
from django.utils._os import safe_join
@@ -24,3 +25,7 @@ class SafeJoinTests(unittest.TestCase):
path,
os.path.sep,
)
+
+ def test_parent_path(self):
+ with self.assertRaises(SuspiciousFileOperation):
+ safe_join("/abc/", "../def")