widelands-dev team mailing list archive

[kaputtnik <somal@xxxxxxxx>]

kaputtnik has proposed merging lp:~widelands-dev/widelands-website/safe_map_upload into lp:widelands-website.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1533789 in Widelands Website: "Upload of Map with German Umlaut fails"
  https://bugs.launchpad.net/widelands-website/+bug/1533789

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands-website/safe_map_upload/+merge/312866

This branch is tested on the alpha site and works nicely.

Prerequisite is to have an additional export in the /etc/init/wlwebsite.conf:

export LANG="de_DE.utf8"

On the alpha site i put it direct under the existing one for DISPLAY.

Using 'env' instead of 'export' doesn't work.

Tested are also uploading of images with with uppercase chars.

The additional view http://alpha.widelands.org/locale/ is just added to verify the settings. It may has to be removed before committing.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands-website/safe_map_upload into lp:widelands-website.

=== modified file 'mainpage/views.py'
--- mainpage/views.py	2016-11-14 21:05:57 +0000
+++ mainpage/views.py	2016-12-08 22:25:35 +0000
@@ -6,12 +6,13 @@
 from django.core.mail import send_mail
 from mainpage.forms import ContactForm
 from django.shortcuts import render
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, HttpResponse
 from django.core.urlresolvers import reverse
 import sys
 import json
 import os
 import os.path
+import locale
 
 
 def mainpage(request):
@@ -153,3 +154,11 @@
 def custom_http_500(request):
     """A custom http 500 error page to not lose css styling."""
     return render(request, '500.html', status=500)
+
+
+def view_locale(request):
+    loc_info = "getlocale: " + str(locale.getlocale()) + \
+        "<br/>getdefaultlocale(): " + str(locale.getdefaultlocale()) + \
+        "<br/>fs_encoding: " + str(sys.getfilesystemencoding()) + \
+        "<br/>sys default encoding: " + str(sys.getdefaultencoding())
+    return HttpResponse(loc_info)

=== modified file 'urls.py'
--- urls.py	2016-11-18 15:30:25 +0000
+++ urls.py	2016-12-08 22:25:35 +0000
@@ -47,6 +47,7 @@
 
     # WL specific:
     url(r'^$', mainpage, name="mainpage"),
+    url(r'^locale/$', 'mainpage.views.view_locale'),
     url(r'^changelog/$', "mainpage.views.changelog", name="changelog"),
     url(r'^developers/$', "mainpage.views.developers", name="developers"),
     url(r'^legal_notice/$', "mainpage.views.legal_notice", name="legal_notice"),

=== modified file 'wlmaps/forms.py'
--- wlmaps/forms.py	2016-12-01 22:22:30 +0000
+++ wlmaps/forms.py	2016-12-08 22:25:35 +0000
@@ -7,13 +7,28 @@
 from django import forms
 from django.forms import ModelForm
 from django.core.files.storage import default_storage
-from django.core.files.base import ContentFile
 
 from settings import MEDIA_ROOT
 from wlmaps.models import Map
 
 
 class UploadMapForm(ModelForm):
+    """
+    We have to handle here three different kind of files:
+    1. The map which is uploaded
+    2. The files created by 'wl_map_info'
+        a. The json file containing infos of the map
+        b. The image of the minimap (png)
+
+    The filename of uploaded maps may contain bad characters which must be handled.
+
+    If a map get deleted in the database, the underlying files (.wmf, .png) still exists. Uploading
+    a map with the same name does not overwrite the existing file(s), instead they get a
+    name in form of 'filename_<random_chars>.wmf(.png)'. This is importand for linking the correct
+    minimap.
+    The map file and the minimap (png) have different random characters in such a case, because the map
+    get saved twice: One time for the call to wl_map_info, and when the model is saved.
+    """
 
     class Meta:
         model = Map
@@ -22,26 +37,36 @@
     def clean(self):
         cleaned_data = super(UploadMapForm, self).clean()
 
-        file = cleaned_data.get('file')
-        if not file:
+        mem_file_obj = cleaned_data.get('file')
+        if not mem_file_obj:
             # no clean file => abort
             return cleaned_data
 
-        name = MEDIA_ROOT + 'wlmaps/maps/' + file.name
-        default_storage.save(name, ContentFile(file.read()))
+        try:
+            # Try to make a safe filename
+            safe_name = default_storage.get_valid_name(mem_file_obj.name)
+            file_path = MEDIA_ROOT + 'wlmaps/maps/' + safe_name
+            saved_file = default_storage.save(file_path, mem_file_obj)
+        except UnicodeEncodeError:
+            self._errors['file'] = self.error_class(
+                ['The filename contain characters which cannot be handled. Please rename and upload again.'])
+            del cleaned_data['file']
+            return cleaned_data
+
         try:
             # call map info tool to generate minimap and json info file
-            check_call(['wl_map_info', name])
+            check_call(['wl_map_info', saved_file])
+
             # TODO(shevonar): delete file because it will be saved again when
             # the model is saved. File should not be saved twice
-            default_storage.delete(name)
+            default_storage.delete(saved_file)
         except CalledProcessError:
             self._errors['file'] = self.error_class(
                 ['The map file could not be processed.'])
             del cleaned_data['file']
             return cleaned_data
 
-        mapinfo = json.load(open(name + '.json'))
+        mapinfo = json.load(open(saved_file + '.json'))
 
         if Map.objects.filter(name=mapinfo['name']):
             self._errors['file'] = self.error_class(
@@ -57,14 +82,16 @@
         self.instance.nr_players = mapinfo['nr_players']
         self.instance.descr = mapinfo['description']
         self.instance.hint = mapinfo['hint']
-
         self.instance.world_name = mapinfo['world_name']
-        # mapinfo["minimap"] is an absolute path and cannot be used.
-        self.instance.minimap = '/wlmaps/maps/' + file.name + '.png'
+
+        # mapinfo["minimap"] is an absolute path.
+        # We partition it to get the correct file path
+        minimap_path = mapinfo['minimap'].partition(MEDIA_ROOT)[2]
+        self.instance.minimap = '/' + minimap_path
 
         # the json file is no longer needed
-        default_storage.delete(name + '.json')
-        
+        default_storage.delete(saved_file + '.json')
+
         return cleaned_data
 
     def save(self, *args, **kwargs):