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
1=== modified file 'wlmaps/forms.py'
2--- wlmaps/forms.py 2016-02-15 14:06:09 +0000
3+++ wlmaps/forms.py 2016-12-01 20:39:47 +0000
4@@ -7,7 +7,6 @@
5 from django import forms
6 from django.forms import ModelForm
7 from django.core.files.storage import default_storage
8-from django.core.files.base import ContentFile
9
10 from settings import MEDIA_ROOT
11 from wlmaps.models import Map
12@@ -22,26 +21,26 @@
13 def clean(self):
14 cleaned_data = super(UploadMapForm, self).clean()
15
16- file = cleaned_data.get('file')
17- if not file:
18+ mem_file_obj = cleaned_data.get('file')
19+ if not mem_file_obj:
20 # no clean file => abort
21 return cleaned_data
22
23- name = MEDIA_ROOT + 'wlmaps/maps/' + file.name
24- default_storage.save(name, ContentFile(file.read()))
25+ file_path = MEDIA_ROOT + 'wlmaps/maps/' + default_storage.get_valid_name(mem_file_obj.name)
26+ saved_file = default_storage.save(file_path, mem_file_obj)
27 try:
28 # call map info tool to generate minimap and json info file
29- check_call(['wl_map_info', name])
30+ check_call(['wl_map_info', saved_file])
31 # TODO(shevonar): delete file because it will be saved again when
32 # the model is saved. File should not be saved twice
33- default_storage.delete(name)
34+ default_storage.delete(saved_file)
35 except CalledProcessError:
36 self._errors['file'] = self.error_class(
37 ['The map file could not be processed.'])
38 del cleaned_data['file']
39 return cleaned_data
40
41- mapinfo = json.load(open(name + '.json'))
42+ mapinfo = json.load(open(saved_file + '.json'))
43
44 if Map.objects.filter(name=mapinfo['name']):
45 self._errors['file'] = self.error_class(
46@@ -60,10 +59,10 @@
47
48 self.instance.world_name = mapinfo['world_name']
49 # mapinfo["minimap"] is an absolute path and cannot be used.
50- self.instance.minimap = '/wlmaps/maps/' + file.name + '.png'
51+ self.instance.minimap = '/wlmaps/maps/' + default_storage.get_valid_name(mem_file_obj.name) + '.png'
52
53 # the json file is no longer needed
54- default_storage.delete(name + '.json')
55+ default_storage.delete(saved_file + '.json')
56
57 return cleaned_data
58

Subscribers

People subscribed via source and target branches