Opened 3 years ago

Closed 2 years ago

#18404 closed Bug (fixed)

SuspiciousOperation exception is thrown if application static path contains non-ascii characters

Reported by: andkit@… Owned by: fhahn
Component: contrib.staticfiles Version: master
Severity: Normal Keywords: sprint2013
Cc: aaron@…, m.r.sopacua@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

AppStaticStorage sets its location based on __file__ of the specific application,
but when FileSystemStore.path checks the location the bytestring will be converted to unicode (by decoding from utf-8) leading to a SuspiciousOperation exception (at least on a windows-machine with a german locale, but I believe this will happen on all systems with non-utf8 filesystem encoding)

Attached patch uses the same approach as the template loaders and will decode the path using the filesystem encoding before further processing happens

Attachments (2)

encoding.patch (629 bytes) - added by andkit@… 3 years ago.
18404-sample.zip (6.4 KB) - added by andkit@… 3 years ago.
Small sample project to reproduce the problem

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by andkit@…

comment:1 Changed 3 years ago by darkpixel

  • Cc aaron@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 follow-up: Changed 3 years ago by jezdez

I can't reproduce the problem, can you provide a test case demonstrating it?

Changed 3 years ago by andkit@…

Small sample project to reproduce the problem

comment:3 in reply to: ↑ 2 Changed 3 years ago by andkit@…

Replying to jezdez:

I can't reproduce the problem, can you provide a test case demonstrating it?

I have attached a sample project. The zip file should contains a directory with a german u-umlaut in latin1 encoding. Set your encoding (export LC_CTYPE=de_DE.ISO-8859-1 should do), unpack anywhere run manage.py runserver and access http://127.0.0.1:8000/ in your browser. (in case unziping doesnt preserve the encoding, you can create a suitable directory with os.mkdir(u"kap\xfct".encode("latin1")))

comment:4 follow-up: Changed 3 years ago by aaugustin

I tried the sample project but runserver didn't even start:

(django-dev)myk@mYk bug18404 % locale                                                                                ~/Downloads/kap%FCt/bug18404
LANG="fr_FR.ISO-8859-1"
LC_COLLATE="fr_FR"
LC_CTYPE="fr_FR"
LC_MESSAGES="fr_FR"
LC_MONETARY="fr_FR"
LC_NUMERIC="fr_FR"
LC_TIME="fr_FR"
LC_ALL="fr_FR"
(django-dev)myk@mYk bug18404 % python manage.py runserver --traceback                                                ~/Downloads/kap%FCt/bug18404
Traceback (most recent call last):
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 222, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 247, in execute
    translation.activate('en-us')
  File "/Users/myk/Documents/dev/django-trunk/django/utils/translation/__init__.py", line 89, in activate
    return _trans.activate(language)
  File "/Users/myk/Documents/dev/django-trunk/django/utils/translation/trans_real.py", line 179, in activate
    _active.value = translation(language)
  File "/Users/myk/Documents/dev/django-trunk/django/utils/translation/trans_real.py", line 168, in translation
    default_translation = _fetch(settings.LANGUAGE_CODE)
  File "/Users/myk/Documents/dev/django-trunk/django/utils/translation/trans_real.py", line 151, in _fetch
    apppath = os.path.join(os.path.dirname(app.__file__), 'locale')
  File "/Users/myk/.virtualenvs/django-dev/bin/../lib/python2.7/posixpath.py", line 71, in join
    path += '/' + b
UnicodeDecodeError: 'ascii' codec can't decode byte 0xfc in position 24: ordinal not in range(128)

Apparently my terminal (under OS X) doesn't support export LC_ALL=fr_FR.ISO-8859-1; that activates the C locale instead.

comment:5 in reply to: ↑ 4 Changed 3 years ago by msopacua

  • Cc m.r.sopacua@… added

Replying to aaugustin:

Apparently my terminal (under OS X) doesn't support export LC_ALL=fr_FR.ISO-8859-1; that activates the C locale instead.

That's cause on FreeBSD derived systems the locale would be fr_FR.ISO8859-1.

The root cause is that u umlaut in latin-1 conflicts with UTF-8 encoding, where the value is used as part of the variable size encoding scheme:

>>> u = unicode('\xfc', 'latin1')
>>> u.encode('utf-8')
'\xc3\xbc'

comment:6 Changed 3 years ago by lrekucki

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

Unfortunately, the abspathu used inside FileSystemStorage.__init doesn't return unicode if not given unicode:

>>> import sys
>>> sys.path
['', 'C:\\Temp\\g\xb9ska', ...snip... ]
>>> import foo
>>> foo.__file__
'C:\\Temp\\g\xb9ska\\foo\\__init__.py'
>>> from django.core.files.storage import *
>>> st = FileSystemStorage(os.path.join(os.path.dirname(foo.__file__), "static"), base_url=u"/foo")
>>> st.path("bar")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "django\core\files\storage.py", line 259, in path
    raise SuspiciousOperation("Attempted access to '%s' denied." % name)
django.core.exceptions.SuspiciousOperation: Attempted access to 'bar' denied.

So this is definitly a bug. Most likely FileSystemStorage should accept only unicode locations and AppStaticStorage should pass a unicode path as done in the patch. It would also be good if path didn't confuse UnicodeDecodeError with ValueError raised by safe_join. Maybe safe_join could raise a more meaningful subclass of ValueError instead?

Note that on master (1.5) this will fail earlier (i.e. on every os.path.join), because of unicode_literals switch.

Last edited 3 years ago by lrekucki (previous) (diff)

comment:7 Changed 3 years ago by lrekucki

  • Patch needs improvement set

comment:8 Changed 2 years ago by fhahn

  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.4 to master

I've added a test for the patch and created a pull request: https://github.com/django/django/pull/814

I put to decoding in FileSystemStorage.init, because AppStaticStorage is only a subclass of FileSystemStorage.

comment:9 Changed 2 years ago by fhahn

  • Keywords sprint2013 added
  • Owner changed from nobody to fhahn
  • Status changed from new to assigned

I had another look and reworked my patch. I've added a test for AppStaticStorage with non ascii characters in the module path.

I think this bug was fixed by the changes introduced by #19357.

upath is used to convert the file name to an unicode string: https://github.com/fhahn/django/blob/ticket_18404/django/contrib/staticfiles/storage.py#L300

comment:10 Changed 2 years ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ca39c0a6becf497880181b3a5ef6db84130f9723:

Fixed #18404 -- Added test for AppStaticStorage with non ascii path

(bug was already fixed in #19357)

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