Merge lp:~widelands-dev/widelands-website/fix_map_upload into lp:widelands-website

Proposed by kaputtnik
Status: Merged
Merged at revision: 437
Proposed branch: lp:~widelands-dev/widelands-website/fix_map_upload
Merge into: lp:widelands-website
Diff against target: 57 lines (+9/-10)
1 file modified
wlmaps/forms.py (+9/-10)
To merge this branch: bzr merge lp:~widelands-dev/widelands-website/fix_map_upload
Reviewer Review Type Date Requested Status
GunChleoc Approve
Review via email: mp+312300@code.launchpad.net

Description of the change

This should fix bug 1533789 (Upload with german umlauts fails)

This is just a try, because the server error couldn't be reproduced here locally.

This fix uses Djangos get_valid_name() of the storage class to get safe filenames.

I have also renamed some vars.

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

LGTM - ship it so we can test.

review: Approve
Revision history for this message
kaputtnik (franku) wrote :

This fix worked with the map 'Schrägland.wmf' but not with the map 'Hier ein bößes Beispiel.wmf'.

I have reverted the changes.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Character encoding is so not fun :(

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'wlmaps/forms.py'
--- wlmaps/forms.py 2016-02-15 14:06:09 +0000
+++ wlmaps/forms.py 2016-12-01 20:39:47 +0000
@@ -7,7 +7,6 @@
7from django import forms7from django import forms
8from django.forms import ModelForm8from django.forms import ModelForm
9from django.core.files.storage import default_storage9from django.core.files.storage import default_storage
10from django.core.files.base import ContentFile
1110
12from settings import MEDIA_ROOT11from settings import MEDIA_ROOT
13from wlmaps.models import Map12from wlmaps.models import Map
@@ -22,26 +21,26 @@
22 def clean(self):21 def clean(self):
23 cleaned_data = super(UploadMapForm, self).clean()22 cleaned_data = super(UploadMapForm, self).clean()
2423
25 file = cleaned_data.get('file')24 mem_file_obj = cleaned_data.get('file')
26 if not file:25 if not mem_file_obj:
27 # no clean file => abort26 # no clean file => abort
28 return cleaned_data27 return cleaned_data
2928
30 name = MEDIA_ROOT + 'wlmaps/maps/' + file.name29 file_path = MEDIA_ROOT + 'wlmaps/maps/' + default_storage.get_valid_name(mem_file_obj.name)
31 default_storage.save(name, ContentFile(file.read()))30 saved_file = default_storage.save(file_path, mem_file_obj)
32 try:31 try:
33 # call map info tool to generate minimap and json info file32 # call map info tool to generate minimap and json info file
34 check_call(['wl_map_info', name])33 check_call(['wl_map_info', saved_file])
35 # TODO(shevonar): delete file because it will be saved again when34 # TODO(shevonar): delete file because it will be saved again when
36 # the model is saved. File should not be saved twice35 # the model is saved. File should not be saved twice
37 default_storage.delete(name)36 default_storage.delete(saved_file)
38 except CalledProcessError:37 except CalledProcessError:
39 self._errors['file'] = self.error_class(38 self._errors['file'] = self.error_class(
40 ['The map file could not be processed.'])39 ['The map file could not be processed.'])
41 del cleaned_data['file']40 del cleaned_data['file']
42 return cleaned_data41 return cleaned_data
4342
44 mapinfo = json.load(open(name + '.json'))43 mapinfo = json.load(open(saved_file + '.json'))
4544
46 if Map.objects.filter(name=mapinfo['name']):45 if Map.objects.filter(name=mapinfo['name']):
47 self._errors['file'] = self.error_class(46 self._errors['file'] = self.error_class(
@@ -60,10 +59,10 @@
6059
61 self.instance.world_name = mapinfo['world_name']60 self.instance.world_name = mapinfo['world_name']
62 # mapinfo["minimap"] is an absolute path and cannot be used.61 # mapinfo["minimap"] is an absolute path and cannot be used.
63 self.instance.minimap = '/wlmaps/maps/' + file.name + '.png'62 self.instance.minimap = '/wlmaps/maps/' + default_storage.get_valid_name(mem_file_obj.name) + '.png'
6463
65 # the json file is no longer needed64 # the json file is no longer needed
66 default_storage.delete(name + '.json')65 default_storage.delete(saved_file + '.json')
67 66
68 return cleaned_data67 return cleaned_data
6968

Subscribers

People subscribed via source and target branches