Fixed #22315 -- str/bytes mismatch in staticfiles #2463

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@timgraham timgraham commented on the diff Mar 24, 2014
django/contrib/staticfiles/storage.py
@@ -292,7 +292,7 @@ def __init__(self, *args, **kwargs):
def read_manifest(self):
try:
with self.open(self.manifest_name) as manifest:
- return manifest.read()
+ return manifest.read().decode('utf-8')
@timgraham
timgraham Mar 24, 2014

should this use settings.FILE_CHARSET instead?

@evansd
evansd Mar 24, 2014

I don't think so: JSON is mandated to be UTF-8

@timgraham
timgraham Mar 25, 2014

I'm not sure, but I found this: "JSON text SHALL be encoded in Unicode. The default encoding is UTF-8."
http://www.ietf.org/rfc/rfc4627.txt

@evansd
evansd Mar 25, 2014

Ah, I see -- thanks. I still think that given that Django controls both ends of the process it should use UTF-8 anyway. But on reflection, we should explicitly encode as UTF-8 when saving as well. I'll update the patch.

@timgraham
Django member

We could probably backport this to 1.6.x -- would you mind adding a line to the 1.6.3 release notes?

Please squash your commits and follow our commit message guidelines if possible. Thank-you.

@evansd

The fix doesn't apply to 1.6 as ManifestFilesMixin is new in 1.7.

I'll squash and rewrite the commit message.

@evansd evansd changed the title from Fix str/bytes mistmatch in ManifestFilesMixin to Fixed #22315 -- str/bytes mismatch in staticfiles Mar 24, 2014
@evansd evansd Fixed #22315 -- str/bytes mismatch in staticfiles
Previously, `ManifestFilesMixin.read_manifest` failed in Python 3
because `json.loads` accepts `str` not `bytes`.

Also added test for `read_manifest`.
0b4df2f
@evansd

Patch updated following discussion on JSON character encoding.

@evansd evansd commented on the diff Mar 25, 2014
django/contrib/staticfiles/storage.py
@@ -319,7 +319,8 @@ def post_process(self, *args, **kwargs):
payload = {'paths': self.hashed_files, 'version': self.manifest_version}
if self.exists(self.manifest_name):
self.delete(self.manifest_name)
- self._save(self.manifest_name, ContentFile(json.dumps(payload)))
+ contents = json.dumps(payload).encode('utf-8')
@evansd
evansd Mar 25, 2014

This is actually redundant as ContentFile will run this through force_bytes which would encode it as UTF-8 anyway, but perhaps it's better to be explicit.

@timgraham
Django member

merged in 86dcac4, thanks!

@timgraham timgraham closed this Mar 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment