Opened 16 years ago

Closed 14 years ago

Last modified 13 years ago

#8593 closed Bug (fixed)

Image upload in Windows changes filename to lowercase

Reported by: Rob van der Linde Owned by: Chris Beaven
Component: File uploads/storage Version: 1.0
Severity: Normal Keywords: upload image lowercase
Cc: Armin Ronacher Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using an ImageField and uploading an image via admin, say the filename was Test.jpg, uploading it in Linux will save the file as Test.jpg on the disk, and the reference in the database is also Test.jpg, things work normally as expected here.

However, when uploading the same file in Windows, the filename is saved on the disk as test.jpg (all lower case), but in the database is still referenced as Test.jpg. Since the filesystem in Windows is not case sensitive, this is not really much of an issue immediately. If you however, then copy your Django based site back to a Linux based machine, you will get a missing image, as it is expected to find the file "Test.jpg" in the filesystem, however the file is "test.jpg". In this case, this can become a bit of a problem.

I am not sure if the problem is related to Django or Python on Windows.

Attachments (5)

8593-regression-tests.diff (4.4 KB ) - added by Ramiro Morales 16 years ago.
File with new tests added to the django test suite that fail
8593-r9832.diff (6.7 KB ) - added by Ramiro Morales 16 years ago.
Patch that solves the issue, also includes the tests already uploaded in 8593-regression-tests.diff
8593-dont-lowercase-uploaded-filenames-r9961.diff (7.1 KB ) - added by Ramiro Morales 16 years ago.
Same patch but with two comments edited to be more accurate
8593-dont-lowercase-uploaded-filenames-r10727.diff (7.3 KB ) - added by Ramiro Morales 16 years ago.
Updated patch so it applies after file_storage tests switch from doctests to unit tests in r10707
8593.diff (8.0 KB ) - added by Chris Beaven 14 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 by anonymous, 16 years ago

As a note, Windows is case sensitive on file creation, but not after that. Django is (correctly) doing to-lower to test for file existence, but I agree with the issue that either:

  • the name in the database should be lowercased to match
  • the file name when actually written should honor the original casing

comment:2 by Chris Beaven, 16 years ago

Writing with the original casing will be more difficult (this is done by django.utils._os.safe_join) so just normalizing the case seems the easiest course of action.

Easiest solution seems to be using os.path.normcase in the get_directory_name and get_filename methods of FileField.

comment:3 by Soviut, 16 years ago

Version: SVN1.0

I can confirm that this issue exists in 1.0 since its the release I'm suffering from it.

comment:4 by Chris Beaven, 16 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Karen Tracey, 16 years ago

#9442 was a dup, it also has a suggested fix.

in reply to:  4 comment:6 by dawidjoubert, 16 years ago

Replying to SmileyChris:
Where does this put us with a fix?

comment:7 by Karen Tracey, 16 years ago

If you want to move the process along, the next step would be posting a patch that fixes the issue, passes all existing tests, and contains a test that fails on the current code (it's OK if it fails only on Windows -- but it can't rely on a particular configuration like a networked drive) and succeeds with the fix.

by Ramiro Morales, 16 years ago

Attachment: 8593-regression-tests.diff added

File with new tests added to the django test suite that fail

comment:8 by Ramiro Morales, 16 years ago

I think that, contrarily to what SmileyChris suggests, it is worth going for the real and correct solution to this problem as the reporter of #9442 suggests rather than adding a workaround.

By correct solution I mean making sure Django uses the same path (case-wise) both for:

  • the file written to disk
  • and the path stored in the database

when storing a FileField/ImageField in all supported platforms.

I've attached a patch (8593-regression-tests.diff) that add tests to the test suite that show how currently, when running on Windows (whose file systems are case-insensitive but case-preserving), Django is lowercasing the path used to create the file in the file system.

These new tests shouldn't fail on other platfoms (i've tested also on Linux).

by Ramiro Morales, 16 years ago

Attachment: 8593-r9832.diff added

Patch that solves the issue, also includes the tests already uploaded in 8593-regression-tests.diff

comment:9 by Ramiro Morales, 16 years ago

Has patch: set

8593-r9832.diff implements a fix for this issue by modifying Django's django.utils._os.safe_join to not call os.path.normcase() under Windows, the modification has been implemented in a way that doesn't break the security safeguards that are the raison d'être of such function.

As safe_join() is also used by the template loaders, the case-insensitive-filesystems-specific template regression tests have been also modified (AFAICS these tests as implemented aren't really useful because they aren't using a template file path as written to the file system when comparing paths but rather two Django maintained in-memory representations of such path.)

Net effect of the path is that now Django doesn't use a path in the file system that differs from the path stored in the DB for uploaded files, irrespective of platform. This should help when moving projects and deployments form platform to platform.

by Ramiro Morales, 16 years ago

Same patch but with two comments edited to be more accurate

comment:10 by Ramiro Morales, 16 years ago

Owner: changed from nobody to Ramiro Morales

by Ramiro Morales, 16 years ago

Updated patch so it applies after file_storage tests switch from doctests to unit tests in r10707

comment:11 by Armin Ronacher, 16 years ago

Cc: Armin Ronacher added

The case is more complex. On OS X the IO system performs unicode normalization for the filename after the file was stored. This is fine until you mount the volume on a linux machine or access it over the network. I'm not sure what the fix is, OS X normalizes in the IO system somehow, it also affects IO access on NFS mounts and similar volumes. You can't really find out how the normalization works from inside Python.

comment:12 by Adam Nelson, 14 years ago

Patch needs improvement: set

Reflecting @mitsuhiko's comment by marking that the patch needs improvement.

comment:13 by Luke Plant, 14 years ago

Severity: Normal
Type: Bug

comment:14 by Chris Beaven, 14 years ago

Easy pickings: unset

For what it's worth, I agree with ramiro that this should be fixed to preserve the case.

I've got a branch with the a new patch here: https://github.com/SmileyChris/django/compare/master...8593-safejoin-lowercase-on-windows

I'm ignoring mitsuhiko's comment, as it has nothing to do with image uploads on windows.

by Chris Beaven, 14 years ago

Attachment: 8593.diff added

comment:15 by Chris Beaven, 14 years ago

milestone: 1.4
Patch needs improvement: unset

comment:16 by srj55, 14 years ago

milestone: 1.4
Patch needs improvement: set

Successfully tested the patch from @SmileyChris on Win7

comment:17 by srj55, 14 years ago

milestone: 1.4
Patch needs improvement: unset

Successfully tested the patch from @SmileyChris on Win7

comment:18 by Ramiro Morales, 14 years ago

Owner: Ramiro Morales removed

comment:19 by Jannis Leidel, 14 years ago

Triage Stage: AcceptedReady for checkin

comment:20 by Chris Beaven, 14 years ago

Owner: set to Chris Beaven
Resolution: fixed
Status: newclosed

In [16267]:

Fixes #8593 -- better handling of safe_join case sensitivity on windows. Thanks for the initial patch, ramiro.

comment:21 by Jacob, 13 years ago

milestone: 1.4

Milestone 1.4 deleted

Note: See TracTickets for help on using tickets.
Back to Top