From fcf7fbc68cadb7efd8bc779c0e189389d6475463 Mon Sep 17 00:00:00 2001 From: Chris Beaven Date: Sun, 22 May 2011 23:56:42 +0000 Subject: [PATCH] Fixes #8593 -- better handling of safe_join case sensitivity on windows. Thanks for the initial patch, ramiro. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16267 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/utils/_os.py | 15 +++++----- tests/regressiontests/file_storage/tests.py | 21 ++++++++++++-- tests/regressiontests/file_uploads/tests.py | 31 +++++++++++++++++++++ tests/regressiontests/file_uploads/urls.py | 1 + tests/regressiontests/file_uploads/views.py | 9 ++++++ tests/regressiontests/templates/tests.py | 8 +++--- 6 files changed, 71 insertions(+), 14 deletions(-) diff --git a/django/utils/_os.py b/django/utils/_os.py index f7b279db24..2c7b88e2c3 100644 --- a/django/utils/_os.py +++ b/django/utils/_os.py @@ -30,17 +30,16 @@ def safe_join(base, *paths): The final path must be located inside of the base path component (otherwise a ValueError is raised). """ - # We need to use normcase to ensure we don't false-negative on case - # insensitive operating systems (like Windows). base = force_unicode(base) paths = [force_unicode(p) for p in paths] - final_path = normcase(abspathu(join(base, *paths))) - base_path = normcase(abspathu(base)) + final_path = abspathu(join(base, *paths)) + base_path = abspathu(base) base_path_len = len(base_path) - # Ensure final_path starts with base_path and that the next character after - # the final path is os.sep (or nothing, in which case final_path must be - # equal to base_path). - if not final_path.startswith(base_path) \ + # Ensure final_path starts with base_path (using normcase to ensure we + # don't false-negative on case insensitive operating systems like Windows) + # and that the next character after the final path is os.sep (or nothing, + # in which case final_path must be equal to base_path). + if not normcase(final_path).startswith(normcase(base_path)) \ or final_path[base_path_len:base_path_len+1] not in ('', sep): raise ValueError('The joined path (%s) is located outside of the base ' 'path component (%s)' % (final_path, base_path)) diff --git a/tests/regressiontests/file_storage/tests.py b/tests/regressiontests/file_storage/tests.py index e9e9d31867..cdec1879ea 100644 --- a/tests/regressiontests/file_storage/tests.py +++ b/tests/regressiontests/file_storage/tests.py @@ -90,13 +90,16 @@ class FileStorageTests(unittest.TestCase): storage_class = FileSystemStorage def setUp(self): - self.temp_dir = tempfile.mktemp() - os.makedirs(self.temp_dir) + self.temp_dir = tempfile.mkdtemp() self.storage = self.storage_class(location=self.temp_dir, base_url='/test_media_url/') + # Set up a second temporary directory which is ensured to have a mixed + # case name. + self.temp_dir2 = tempfile.mkdtemp(suffix='aBc') def tearDown(self): shutil.rmtree(self.temp_dir) + shutil.rmtree(self.temp_dir2) def test_file_access_options(self): """ @@ -265,6 +268,20 @@ class FileStorageTests(unittest.TestCase): self.assertRaises(SuspiciousOperation, self.storage.exists, '..') self.assertRaises(SuspiciousOperation, self.storage.exists, '/etc/passwd') + def test_file_storage_preserves_filename_case(self): + """The storage backend should preserve case of filenames.""" + # Create a storage backend associated with the mixed case name + # directory. + temp_storage = self.storage_class(location=self.temp_dir2) + # Ask that storage backend to store a file with a mixed case filename. + mixed_case = 'CaSe_SeNsItIvE' + file = temp_storage.open(mixed_case, 'w') + file.write('storage contents') + file.close() + self.assertEqual(os.path.join(self.temp_dir2, mixed_case), + temp_storage.path(mixed_case)) + temp_storage.delete(mixed_case) + class CustomStorage(FileSystemStorage): def get_available_name(self, name): """ diff --git a/tests/regressiontests/file_uploads/tests.py b/tests/regressiontests/file_uploads/tests.py index 3c126b7d0f..d42e0279a7 100644 --- a/tests/regressiontests/file_uploads/tests.py +++ b/tests/regressiontests/file_uploads/tests.py @@ -278,6 +278,37 @@ class FileUploadTests(TestCase): # CustomUploadError is the error that should have been raised self.assertEqual(err.__class__, uploadhandler.CustomUploadError) + def test_filename_case_preservation(self): + """ + The storage backend shouldn't mess with the case of the filenames + uploaded. + """ + # Synthesize the contents of a file upload with a mixed case filename + # so we don't have to carry such a file in the Django tests source code + # tree. + vars = {'boundary': 'oUrBoUnDaRyStRiNg'} + post_data = [ + '--%(boundary)s', + 'Content-Disposition: form-data; name="file_field"; ' + 'filename="MiXeD_cAsE.txt"', + 'Content-Type: application/octet-stream', + '', + 'file contents\n' + '', + '--%(boundary)s--\r\n', + ] + response = self.client.post( + '/file_uploads/filename_case/', + '\r\n'.join(post_data) % vars, + 'multipart/form-data; boundary=%(boundary)s' % vars + ) + self.assertEqual(response.status_code, 200) + id = int(response.content) + obj = FileModel.objects.get(pk=id) + # The name of the file uploaded and the file stored in the server-side + # shouldn't differ. + self.assertEqual(os.path.basename(obj.testfile.path), 'MiXeD_cAsE.txt') + class DirectoryCreationTests(unittest.TestCase): """ Tests for error handling during directory creation diff --git a/tests/regressiontests/file_uploads/urls.py b/tests/regressiontests/file_uploads/urls.py index 9f814c4d4c..a5c2702311 100644 --- a/tests/regressiontests/file_uploads/urls.py +++ b/tests/regressiontests/file_uploads/urls.py @@ -11,4 +11,5 @@ urlpatterns = patterns('', (r'^quota/broken/$', views.file_upload_quota_broken), (r'^getlist_count/$', views.file_upload_getlist_count), (r'^upload_errors/$', views.file_upload_errors), + (r'^filename_case/$', views.file_upload_filename_case_view), ) diff --git a/tests/regressiontests/file_uploads/views.py b/tests/regressiontests/file_uploads/views.py index dba7522bae..c88c77548a 100644 --- a/tests/regressiontests/file_uploads/views.py +++ b/tests/regressiontests/file_uploads/views.py @@ -120,3 +120,12 @@ def file_upload_getlist_count(request): def file_upload_errors(request): request.upload_handlers.insert(0, ErroringUploadHandler()) return file_upload_echo(request) + +def file_upload_filename_case_view(request): + """ + Check adding the file to the database will preserve the filename case. + """ + file = request.FILES['file_field'] + obj = FileModel() + obj.testfile.save(file.name, file) + return HttpResponse('%d' % obj.pk) diff --git a/tests/regressiontests/templates/tests.py b/tests/regressiontests/templates/tests.py index abfdb8d735..5237bb6ba0 100644 --- a/tests/regressiontests/templates/tests.py +++ b/tests/regressiontests/templates/tests.py @@ -161,8 +161,8 @@ class Templates(unittest.TestCase): fs_loader = filesystem.Loader() def test_template_sources(path, template_dirs, expected_sources): if isinstance(expected_sources, list): - # Fix expected sources so they are normcased and abspathed - expected_sources = [os.path.normcase(os.path.abspath(s)) for s in expected_sources] + # Fix expected sources so they are abspathed + expected_sources = [os.path.abspath(s) for s in expected_sources] # Test the two loaders (app_directores and filesystem). func1 = lambda p, t: list(ad_loader.get_template_sources(p, t)) func2 = lambda p, t: list(fs_loader.get_template_sources(p, t)) @@ -205,9 +205,9 @@ class Templates(unittest.TestCase): if os.path.normcase('/TEST') == os.path.normpath('/test'): template_dirs = ['/dir1', '/DIR2'] test_template_sources('index.html', template_dirs, - ['/dir1/index.html', '/dir2/index.html']) + ['/dir1/index.html', '/DIR2/index.html']) test_template_sources('/DIR1/index.HTML', template_dirs, - ['/dir1/index.html']) + ['/DIR1/index.HTML']) def test_loader_debug_origin(self): # Turn TEMPLATE_DEBUG on, so that the origin file name will be kept with