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

[kaputtnik]

Allow attachments in the forum

Made attachments python3 compatible: https://bazaar.launchpad.net/~widelands-dev/widelands-website/pybb_attachments/revision/545

Set allowed file size to 5 Mb (as we have for nginx)

Delete attachment when deleting a post: https://bazaar.launchpad.net/~widelands-dev/widelands-website/pybb_attachments/revision/548

Added template for showing attachments.

Style tweaks, example: https://bugs.launchpad.net/widelands-website/+bug/964452/+attachment/5277903/+files/attachments_1.png

Autoreload css for the users if this got merged, so the users don't have to hit CTRL+F5

I'll do some explanations in the wiki and announce this change. Probably with the option to remove it again if we get too many problematic attachments.

[lp:~widelands-dev/widelands-website/pybb_attachments]

552. By kaputtnik on 2019-07-19

removed wrong div tag

[GunChleoc (gunchleoc)]

Code LGTM. Do we have a restriction on file types? We won't want people to upload malicious JavaScript ans stuff lie that, so we should have a list of legal file extensions at least.

[kaputtnik (franku)]

Currently there is no restriction on file types. The problem is that a file may not contain what the extension says it should be. Everyone can rename 'image.js' into 'image.png'...

A list of allowed extension do not suffer. I'll try to implement some checks for validating a files type and probably an 'allowed extension list'.

What about restricting uploads to users who have written x posts prior? From my side this can be implemented.

[GunChleoc (gunchleoc)]

> Everyone can rename 'image.js' into 'image.png'...

That's true. It would still be an advantage though if the browser then doesn't execute it.

Doing a full validation too is definitely a better idea.

> What about restricting uploads to users who have written x posts prior?

Also a good idea

[lp:~widelands-dev/widelands-website/pybb_attachments]

553. By kaputtnik on 2019-08-03

make attachments only available to users who had written MAX_HIDDEN_POST

554. By kaputtnik on 2019-08-06

added first file validation logic; render the post_form manually; give textarea more height

555. By kaputtnik on 2019-08-06

some reafctoring

556. By kaputtnik on 2019-08-06

added check for .wgf (widelands savegames); added more comments

557. By kaputtnik on 2019-08-07

merged trunk

558. By kaputtnik on 2019-08-08

use Image verify()

[kaputtnik (franku)]

I am a bit lost here...

Some checks are made now, but i am unsure if we need them all. What is implemented:

1. Link of Attachment:
If a user clicks on a link of an attachment, he will be ever prompted for a download (or to open the file with an associated program), even if it is a file which the browser may can display, e.g. an image (png, jpg) or text file. This is not optimal, but i think it is more secure.

2. Restricting upload permission
A user has only permission to upload files, if he has had written 5 posts prior. The number of 5 derives from MAX_HIDDEN_POST at the moment, but i think it's better to have an own value for this. What number do we want?

3. Allowed extensions:
Added a list:
ALLOWED_EXTENSIONS = ['wgf', 'jpg', 'jpeg', 'gif', 'png', 'ogg', 'lua', 'ods', 'zip', 'json', 'txt', 'csv']
I am unsure if this list will be ever complete. E.g. some may want to upload some other types of files (e.g. py, odg, doc, ...)

4. Extension handling:
- Files without extension aren't allowed (can be problematic e.g. with the campvis file)
- For 'wgf' files: Check if some base entries can be found (e.g. '/binary/' or '/minimap.png')
- For image files: Check if this is really an image using the Pillow library. This finds corrupted .png but not corrupted .jpg
Do we need a check for each allowed extension?

5. Comparing Mime-Types:
When uploading a file, a Mime-Type (content-type) is also submitted. The submitted mime-type is mostly derived from the extension of the file. E.g. if you rename a file from 'image.png' to 'image.txt' the OS (or browser) sends the Mime-type 'text/plain', which is obviously wrong. To prevent such mime-type mismatch, a check is made which compares the sent mime-type with the mime-type derived from python-magic. e.g. python magic will show for 'image.txt' the mime-type 'image/png'.

As said, i am a bit lost, not really knowing if we need all this checks, or if they are enough at all :-S Any opinions?

[GunChleoc (gunchleoc)]

2. I have no opinion on this at this time. Maybe 10 or 20? We can easily change the value if it becomes a problem.

> Files without extension aren't allowed (can be problematic e.g. with the campvis file)

campvis will be replaced by campaigns.conf for the next version, so don't worry about it - we can allow .conf

Other extensions that we're using for Widelands & that might be interesting:

.wai - AI
.wmf - Map
.wrpl - Replay - useless with the accompanying .wgf though.

> Do we need a check for each allowed extension?

Since attachments are marked for download anyway, I'd say that checking for those that are easily checked will do for now.

[kaputtnik (franku)]

> Other extensions that we're using for Widelands & that might be interesting:

> .wai - AI
Ok

> .wmf - Map

I forgot to mention: If the extension is wmf, an errormessage is shown: "This seems to be a widelands map file. Please upload it at our maps section."

> .wrpl - Replay - useless with the accompanying .wgf though.

I am planning to make a wikipage describing, beside other things, the usage of replays, e.g. they have to be zipped together before uploading and link that wikipage in the upload form. But i have to check for .wrpl.wgf.

[kaputtnik (franku)]

I have to stop working on this, other wise i fear to change many other things :-D

Anyway i think this works. wai files need some special treatment though.

Please test on alpha: https://alpha.widelands.org/

review: Needs Resubmitting

[lp:~widelands-dev/widelands-website/pybb_attachments]

559. By kaputtnik on 2019-08-12

merged with latest changes

560. By kaputtnik on 2019-08-13

added a Reset File button

561. By kaputtnik on 2019-08-14

added clamav check; fixed a failure for zip files

[kaputtnik (franku)]

stonerl, can you install and configure clamav on the server, please?

The current branch implemented a check with clamdscan, which needs a clamav daemon. We can also stick with clamscan, which can be run on demand, but this much slow: A file check last 55 sec. on my machine, whereas clamdscan usually needs only 0,1 sec.

The current branch is not running on alpha, yet. Since i am afk for some some days, i'll shut down alpha now.

[Toni Förster (stonerl)]

Already installed. ;)

[lp:~widelands-dev/widelands-website/pybb_attachments]

562. By kaputtnik on 2019-08-14

make virus scan a stand alone validor

563. By kaputtnik on 2019-08-15

cleanups

[GunChleoc (gunchleoc)]

1 tiny nit.

I also wonder whether we should disallow map files - it might come in handy if somebody wants some feedback before publishing to the maps section.

[kaputtnik (franku)]

> I also wonder whether we should disallow map files - it might come in handy if somebody wants some feedback before publishing to the maps section.

I just don't want to have map files in the forum. I 'fear' most (new) users will upload a map in the forum instead in the 'right' place. A Feedback can also be done in the map comments, or by linking a map in the forum and ask there for a feedback. Once there is a fix for bug 357914 'Allow submitters of maps to upload new versions' get implement, it should be fine to not allow maps in forum.

But of course we can remove that part of code and if we get in trouble we can add it again.

[lp:~widelands-dev/widelands-website/pybb_attachments]

564. By kaputtnik on 2019-08-19

MiB instead of Mb

[kaputtnik (franku)]

@Toni: Can we have the package 'clamdscan' installed? For some reason it is a recommended package for clamd and we do not install recommended packages by default.

Or is clamd configured to have on-access-scanning enabled? Didn't saw that it is configured like that.

[Toni Förster (stonerl)]

How do you intend to access clamav? The current mode is via socket:

LocalSocket /var/run/clamav/clamd.ctl

If you'd like, I can add a local TCP daemon on port 3310.

[Toni Förster (stonerl)]

Stupid me. :-) I installed clamdscan.

[lp:~widelands-dev/widelands-website/pybb_attachments]

565. By kaputtnik on 2019-08-21

added attachment to admin site of a post

[kaputtnik (franku)]

Thanks :-)

But the check for clamdscan doesn't work on the server... i know why, but i don't know the underlying cause. Will talk on IRC about that issue when i have some more time.

Just found that map uploading is broken with the changes in this branch. So the code for map uploading needs adapting too...

review: Needs Fixing

[lp:~widelands-dev/widelands-website/pybb_attachments]

566. By kaputtnik on 2019-08-25

refactored uploading of maps to fit with the FileUploadHandler

567. By kaputtnik on 2019-08-25

use a setting to enable virus check

568. By kaputtnik on 2019-08-25

use an absolute path for clamdscan. This is needed on the server

[kaputtnik (franku)]

Finally i got it (hopefully)

Please check again on alpha... https://alpha.widelands.org/

[kaputtnik (franku)]

review: Needs Resubmitting

[GunChleoc (gunchleoc)]

I have tested map upload and attaching to forum.

review: Approve

[kaputtnik (franku)]

Manx thanks :)

I wait only for janus to approve the changes regarding python3. He will look at it next weekend.

[lp:~widelands-dev/widelands-website/pybb_attachments]

569. By kaputtnik on 2019-09-02

simplified attachment hash

[kaputtnik (franku)]

Finally this is productive now :-)

=== modified file 'mainpage/settings.py'
--- mainpage/settings.py	2019-04-11 16:11:21 +0000
+++ mainpage/settings.py	2019-09-02 20:01:34 +0000
@@ -1,4 +1,5 @@
-# Django settings for widelands project.
+# Django default settings for widelands project.
+# Overwrite these settings in local_settings.py!
 
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import os
@@ -162,7 +163,7 @@
 DEFAULT_FROM_EMAIL = '[e-mail]'
 ACCOUNT_ACTIVATION_DAYS = 2  # Days an activation token keeps active
 
-# Franku: SHA1 Needed as compatibility for old passwords
+# SHA1 Needed as compatibility for old passwords
 # https://docs.djangoproject.com/en/1.11/releases/1.10/#removed-weak-password-hashers-from-the-default-password-hashers-setting
 PASSWORD_HASHERS = [
     'django.contrib.auth.hashers.PBKDF2PasswordHasher',
@@ -190,10 +191,50 @@
 ######################
 # Pybb Configuration #
 ######################
-# See also pybb defaults in pybb.settings.py
-PYBB_ATTACHMENT_ENABLE = False  # disable gzip middleware when enabling attachments
+
+PYBB_ATTACHMENT_ENABLE = True
+
+# To prevent sending errors from the webserver, keep
+# this below the webserver settings 
+PYBB_ATTACHMENT_SIZE_LIMIT = 1024*1024*4
 PYBB_DEFAULT_MARKUP = 'markdown'
-INTERNAL_PERM='pybb.can_access_internal' # The permission string derived from pybb.models.category
+INTERNAL_PERM = 'pybb.can_access_internal' # The permission string derived from pybb.models.category
+
+##################################
+# Uploading files and validation #
+##################################
+
+# Use only this handler to get real a file in /tmp
+# Some validation checks needs a real file
+FILE_UPLOAD_HANDLERS = [
+    'django.core.files.uploadhandler.TemporaryFileUploadHandler',
+    ]
+
+ALLOWED_EXTENSIONS = [
+    'wgf', 'jpg', 'jpeg', 'gif', 'png', 'ogg', 'lua',
+    'ods', 'zip', 'json', 'txt', 'csv', 'wai',
+    ]
+
+# Widelands Savegame should contain at least these entries
+WGF_CONTENT_CHECK = ['/binary/', '/map/', '/minimap.png', '/preload',]
+
+# Do not check mime type for these extensions
+SKIP_MIME_EXTENSIONS = ['wai',]
+
+ALLOWED_WAI_SECTIONS = [
+    'magic_numbers', 'neuron_values',
+    'neuron_functions', 'fneurons',
+    ]
+
+# Allow attachments only after this amount of posts
+ALLOW_ATTACHMENTS_AFTER = 5
+
+# Page describing uploads
+ATTACHMENT_DESCR_PAGE = 'Attachments'
+
+# If clamav including clamdscan is installed and running
+# set this to True
+VIRUS_CHECK = False
 
 ##############################################
 # Link classification and other Markup stuff #

=== added file 'mainpage/validators.py'
--- mainpage/validators.py	1970-01-01 00:00:00 +0000
+++ mainpage/validators.py	2019-09-02 20:01:34 +0000
@@ -0,0 +1,30 @@
+from django.core.exceptions import ValidationError
+from django.conf import settings
+
+import shutil
+import subprocess
+
+
+def virus_scan(uploaded_file):
+    '''Scan uploaded_file for viruses with clamav.'''
+
+    if settings.VIRUS_CHECK:
+        try:
+            tmp_file_path = uploaded_file.temporary_file_path()
+            process_compl = subprocess.run(['/usr/bin/clamdscan',
+                                            '--multiscan',
+                                            '--fdpass',
+                                            tmp_file_path])
+            if process_compl.returncode == 1:
+                raise ValidationError(
+                    'This file seems to contain malicious code.'
+                    )
+            if process_compl.returncode == 2:
+                raise ValidationError(
+                    'Some error occured during virus scannning...'
+                )
+        except FileNotFoundError:
+            raise ValidationError(
+                'Please check the installation of clamav and make \
+                sure clamdscan is working.'
+            )

=== modified file 'pip_requirements.txt'
--- pip_requirements.txt	2019-07-05 06:57:46 +0000
+++ pip_requirements.txt	2019-09-02 20:01:34 +0000
@@ -19,6 +19,7 @@
 numpy==1.16.4
 Pillow==6.1.0
 pydot==1.4.1
+python-magic==0.4.15
 Sphinx==2.1.2
 Whoosh==2.7.4
 

=== modified file 'pybb/admin.py'
--- pybb/admin.py	2019-02-27 09:14:05 +0000
+++ pybb/admin.py	2019-09-02 20:01:34 +0000
@@ -1,7 +1,7 @@
 # -*- coding: utf-8
 from django.utils.translation import ugettext_lazy as _
 from django.contrib import admin
-from pybb.models import Category, Forum, Topic, Post, Read
+from pybb.models import Category, Forum, Topic, Post, Read, Attachment
 
 
 def delete_selected(modeladmin, request, queryset):
@@ -75,6 +75,12 @@
     )
 
 
+class AttachmentInline(admin.StackedInline):
+    model = Attachment
+    extra = 0
+    exclude = ('hash',)
+
+
 class PostAdmin(admin.ModelAdmin):
     list_display = ['summary', 'topic', 'user', 'created', 'hidden']
     list_per_page = 20
@@ -82,12 +88,13 @@
     date_hierarchy = 'created'
     search_fields = ['body', 'topic__name']
     actions = [delete_selected, unhide_post]
+    inlines = [AttachmentInline,]
     fieldsets = (
         (None, {
             'fields': ('topic', 'user', 'markup', 'hidden')
         }
         ),
-        (_('Additional options'), {
+        (_('Date and Time'), {
             'classes': ('collapse',),
             'fields': (('created', 'updated'),)
         }

=== modified file 'pybb/forms.py'
--- pybb/forms.py	2019-03-20 21:32:32 +0000
+++ pybb/forms.py	2019-09-02 20:01:34 +0000
@@ -1,6 +1,6 @@
 import re
 from datetime import datetime
-import os.path
+import os
 
 from django import forms
 from django.conf import settings
@@ -10,16 +10,24 @@
 from pybb.models import Topic, Post, Attachment
 from pybb import settings as pybb_settings
 from django.conf import settings
+from .util import validate_file
+from mainpage.validators import virus_scan
 
 
 class AddPostForm(forms.ModelForm):
     name = forms.CharField(label=_('Subject'))
-    attachment = forms.FileField(label=_('Attachment'), required=False)
+    attachment = forms.FileField(
+        label=_('Attachment'),
+        required=False,
+        validators=[virus_scan, validate_file, ])
 
     class Meta:
         model = Post
-        # Listing fields again to get the the right order; See also the TODO
+        # Listing fields again to get the the right order
         fields = ['name', 'body', 'markup', 'attachment', ]
+        widgets = {
+            'body': forms.Textarea(attrs={'cols': 80, 'rows': 15}),
+        }
 
     def __init__(self, *args, **kwargs):
         self.user = kwargs.pop('user', None)
@@ -31,17 +39,10 @@
             self.fields['name'].widget = forms.HiddenInput()
             self.fields['name'].required = False
 
-        if not pybb_settings.ATTACHMENT_ENABLE:
+        if not pybb_settings.ATTACHMENT_ENABLE or self.user.wlprofile.post_count() < settings.ALLOW_ATTACHMENTS_AFTER:
             self.fields['attachment'].widget = forms.HiddenInput()
             self.fields['attachment'].required = False
 
-    def clean_attachment(self):
-        if self.cleaned_data['attachment']:
-            memfile = self.cleaned_data['attachment']
-            if memfile.size > pybb_settings.ATTACHMENT_SIZE_LIMIT:
-                raise forms.ValidationError(_('Attachment is too big'))
-        return self.cleaned_data['attachment']
-
     def save(self, *args, **kwargs):
         if self.forum:
             topic_is_new = True
@@ -69,9 +70,15 @@
                              name=memfile.name, post=post)
             dir = os.path.join(settings.MEDIA_ROOT,
                                pybb_settings.ATTACHMENT_UPLOAD_TO)
-            fname = '%d.0' % post.id
+            if not os.path.exists(dir):
+                os.makedirs(dir)
+
+            fname = '{}.0'.format(post.id)
             path = os.path.join(dir, fname)
-            file(path, 'w').write(memfile.read())
+
+            with open(path, 'wb') as f:
+                f.write(memfile.read())
+
             obj.path = fname
             obj.save()
 

=== modified file 'pybb/models.py'
--- pybb/models.py	2019-04-11 15:20:33 +0000
+++ pybb/models.py	2019-09-02 20:01:34 +0000
@@ -18,7 +18,7 @@
 from django.conf import settings
 from notification.models import send
 from check_input.models import SuspiciousInput
-
+import magic
 
 try:
     from notification import models as notification
@@ -345,6 +345,11 @@
     def delete(self, *args, **kwargs):
         self_id = self.id
         head_post_id = self.topic.posts.order_by('created')[0].id
+
+        if self.attachments.all():
+            for attach in self.attachments.all():
+                attach.delete()
+
         super(Post, self).delete(*args, **kwargs)
 
         self.topic.save()
@@ -398,7 +403,7 @@
         super(Attachment, self).save(*args, **kwargs)
         if not self.hash:
             self.hash = hashlib.sha1(
-                str(self.id) + settings.SECRET_KEY).hexdigest()
+                bytes(self.id) + settings.SECRET_KEY.encode('utf-8')).hexdigest()
         super(Attachment, self).save(*args, **kwargs)
 
     def __str__(self):
@@ -407,19 +412,17 @@
     def get_absolute_url(self):
         return reverse('pybb_attachment', args=[self.hash])
 
-    def size_display(self):
-        size = self.size
-        if size < 1024:
-            return '%b' % size
-        elif size < 1024 * 1024:
-            return '%dKb' % int(size / 1024)
-        else:
-            return '%.2fMb' % (size / float(1024 * 1024))
-
     def get_absolute_path(self):
         return os.path.join(settings.MEDIA_ROOT, pybb_settings.ATTACHMENT_UPLOAD_TO,
                             self.path)
 
+    def delete(self, *args, **kwargs):
+        try:
+            os.remove(self.get_absolute_path())
+        except FileNotFoundError:
+            pass
+        super(Attachment, self).delete(*args, **kwargs)
+
 
 if notification is not None:
     signals.post_save.connect(notification.handle_observations, sender=Post)

=== modified file 'pybb/settings.py'
--- pybb/settings.py	2019-03-15 08:58:09 +0000
+++ pybb/settings.py	2019-09-02 20:01:34 +0000
@@ -16,6 +16,7 @@
 DEFAULT_MARKUP = get('PYBB_DEFAULT_MARKUP', 'bbcode')
 ATTACHMENT_UPLOAD_TO = get('PYBB_ATTACHMENT_UPLOAD_TO', 'pybb/attachments')
 ATTACHMENT_SIZE_LIMIT = get('PYBB_ATTACHMENT_SIZE_LIMIT', 1024 * 1024)
+PYBB_ATTACHMENT_ENABLE = get('PYBB_ATTACHMENT_ENABLE', False)
 ATTACHMENT_ENABLE = get('PYBB_ATTACHMENT_ENABLE', True)
 INTERNAL_PERM =  get('INTERNAL_PERM', 'pybb.can_access_internal')
 LAST_POSTS_DAYS = get('LAST_POSTS_DAYS', 30)

=== modified file 'pybb/static/css/forum.css'
--- pybb/static/css/forum.css	2019-02-28 16:09:16 +0000
+++ pybb/static/css/forum.css	2019-09-02 20:01:34 +0000
@@ -132,6 +132,10 @@
 	border: 1px solid #474444;
 }
 
+.attachment {
+	display: inline-block
+}
+
 /*****************/
 /* Edit/add Post */
 /*   related     */
@@ -145,7 +149,6 @@
 }
 
 .post-form #id_body {
-   height: 150px;
    width: 100%;
 }
 

=== modified file 'pybb/templates/pybb/base.html'
--- pybb/templates/pybb/base.html	2019-01-24 18:03:54 +0000
+++ pybb/templates/pybb/base.html	2019-09-02 20:01:34 +0000
@@ -7,7 +7,7 @@
 {% endblock %}
 
 {% block extra_head %}
-<link rel="stylesheet" type="text/css" media="all" href="{% static 'css/forum.css' %}" />
+<link rel="stylesheet" type="text/css" media="all" href="{% static 'css/forum.css' %}?v1" />
 
 <link rel="alternate" type="application/atom+xml" title="Latest Posts on all forums" href="{% url 'pybb_feed_posts' %}" />
 <link rel="alternate" type="application/atom+xml" title="Latest Topics on all forums" href="{% url 'pybb_feed_topics' %}" />

=== modified file 'pybb/templates/pybb/delete_post.html'
--- pybb/templates/pybb/delete_post.html	2018-10-14 13:24:15 +0000
+++ pybb/templates/pybb/delete_post.html	2019-09-02 20:01:34 +0000
@@ -15,13 +15,18 @@
 	<div class="preview-box">
 		<div class="content post">
 		{{ post.body_html|safe }}
+
+		{% if post.attachments.all %}
+			{% include 'pybb/inlines/attachment.html' %}
+		{% endif %}
 		</div>
+		<br />
 	</div>
 	<form method="post">
-		<div class="info_line">
+		<p class="info_line">
 			<input type="submit" value="{% trans "Yes, I am sure." %}" />
 			<input type="button" value="{% trans "Cancel" %}" onclick="history.back();" />
-		</div>
+		</p>
 		{% csrf_token %}
 	</form>
 </div>

=== added file 'pybb/templates/pybb/inlines/attachment.html'
--- pybb/templates/pybb/inlines/attachment.html	1970-01-01 00:00:00 +0000
+++ pybb/templates/pybb/inlines/attachment.html	2019-09-02 20:01:34 +0000
@@ -0,0 +1,11 @@
+{% load i18n %}
+
+{% for attach in post.attachments.all %}
+	<div class="attachment">
+		<hr />
+		{% trans "Attachment" %}:
+		<a href="{{ attach.get_absolute_url }}" download="{{attach.name }}" type="{{attach.content_type}}">{{ attach.name }}</a>
+		({{ attach.size|filesizeformat }})
+	</div>
+{% endfor %}
+

=== modified file 'pybb/templates/pybb/inlines/post.html'
--- pybb/templates/pybb/inlines/post.html	2018-12-21 12:50:32 +0000
+++ pybb/templates/pybb/inlines/post.html	2019-09-02 20:01:34 +0000
@@ -29,19 +29,16 @@
 	<a id="post-{{ post.id }}" href="{{post.get_absolute_url}}" title="{% trans "Permalink" %}" class="posRight small permalink">&nbsp;</a>
 	<span class="small">Posted at: {{ post.created|custom_date:user}}</span>
 	<hr />
-	<div class="post">
 		{{ post.body_html|safe }}
-	</div>
+
+	{% if post.updated %}
+		<div class="small">{% trans "Edited" %}: {{ post.updated|custom_date:user|title}}</div>
+	{% endif %}
 
 	{% if post.attachment_cache %}
-		{% for attach in post.attachment_cache %}
-			{% trans "Attachment" %}: <a href="{{ attach.get_absolute_url }}">{{ attach.name }}</a> ({{ attach.size_display }})
-		{% endfor %}
+		{% include 'pybb/inlines/attachment.html' %}
 	{% endif %}
 
-	{% if post.updated %}
-		<span class="small">{% trans "Edited" %}: {{ post.updated|custom_date:user|title}}</span>
-	{% endif %}
 	<hr />
 	{% if user.is_authenticated %}
 		{% ifequal user.wlprofile.show_signatures 1 %}
@@ -54,7 +51,6 @@
 			{{ post.user.wlprofile.signature|urlize|linebreaks }}
 		{% endif %}
 	{% endif %}
-
 	<a class="button posRight" href="#top">
 		<img src="{% static 'forum/img/top.png' %}" alt ="" class="middle" />
 		<span class="middle">{% trans "Top" %}</span>

=== modified file 'pybb/templates/pybb/post_form.html'
--- pybb/templates/pybb/post_form.html	2018-11-22 11:08:51 +0000
+++ pybb/templates/pybb/post_form.html	2019-09-02 20:01:34 +0000
@@ -33,6 +33,11 @@
 			
 		});
 	});
+	$(document).ready(function() {
+		$('#btn-file-reset').on('click', function() {
+			$('#id_attachment').val('');
+	});
+});
 </script>
 {% endblock %}
 
@@ -52,7 +57,40 @@
 	</div>
 
 	<form class="post-form" action="{{ form_url }}" method="post" enctype="multipart/form-data">
-	{{ form.as_p }}
+		{% if not form.name.is_hidden %}
+		<p>
+			<p class="errormessage">{{ form.name.errors.as_text }}</p>
+			{{ form.name.label }}:
+			{{ form.name }}
+		</p>
+		{% endif %}
+		<p>
+			<p class="errormessage">{{ form.body.errors.as_text }}</p>
+			{{ form.body.label }}:
+			{{ form.body }}
+		</p>
+		<p>
+			<p class="errormessage">{{ form.markup.errors.as_text }}</p>
+			{{ form.markup.label }}:
+			{{ form.markup }}
+		</p>
+		{% if not form.attachment.is_hidden %}
+		<p>
+			<p>
+				Please refer to the wiki page
+				<a href="{{ site }}/wiki/{{ wikipage }}">{{ wikipage }}</a>
+				for pointers.
+			</p>
+			<p class="errormessage">{{ form.attachment.errors.as_text }}</p>
+			{{ form.attachment.label }}
+			{{ form.attachment }}
+			<button id="btn-file-reset" type="button">
+				<img src="{% static 'img/delete.png' %}" alt ="" class="middle" />
+				<span class="middle">{% trans "Reset" %}</span>
+			</button>
+		</p>
+		{% endif %}
+
 	{% csrf_token %}
 	<button type="submit">
 		<img src="{% static 'forum/img/send.png' %}" alt ="" class="middle" />

=== modified file 'pybb/util.py'
--- pybb/util.py	2019-04-11 15:20:33 +0000
+++ pybb/util.py	2019-09-02 20:01:34 +0000
@@ -3,6 +3,7 @@
 import traceback
 import json
 import re
+import subprocess
 
 from bs4 import BeautifulSoup, NavigableString
 from datetime import datetime
@@ -14,7 +15,14 @@
 from django import forms
 from django.core.paginator import Paginator, EmptyPage, InvalidPage
 from django.conf import settings
+from django.core.exceptions import ValidationError
 from pybb import settings as pybb_settings
+import magic
+import zipfile
+import configparser
+from PIL import Image
+
+
 
 
 def render_to(template_path):
@@ -31,7 +39,6 @@
             output = func(request, *args, **kwargs)
             if not isinstance(output, dict):
                 return output
-            
 
             # TODO(Franku): 'MIME_TYPE' is never in output as i can see for now.
             # But if, this should maybe 'content_type' instead
@@ -173,3 +180,144 @@
     text = text.replace('&quot;', '"')
     text = text.replace('&#39;', '\'')
     return text
+
+
+def validate_file(attachment):
+
+    tmp_file_path = attachment.temporary_file_path()
+
+    # Helper functions
+    def _split_mime(mime_type):
+        main, sub = mime_type.split('/', maxsplit=1)
+        return {'maintype': main, 'subtype': sub}
+
+    def _is_image():
+        # Use PIL to determine if it is a valid image file
+        # works not for corrupted jpg
+        try:
+            with Image.open(tmp_file_path) as im:
+                im.verify()
+        except:
+            return False
+        return True
+
+    def _is_zip():
+        try:
+            zip_obj = zipfile.ZipFile(tmp_file_path)
+        except zipfile.BadZipfile:
+            return None
+        return zip_obj
+
+    def _zip_contains(zip_parts):
+        # Check if each entry in zip_parts is inside the attachment
+        zip_obj = _is_zip()
+        if zip_obj:
+            try:
+                for obj in zip_parts:
+                    zip_obj.getinfo(obj)
+            except KeyError:
+                return False
+        return True
+
+
+    # Main part of file checks
+    # File size
+    if attachment.size > pybb_settings.ATTACHMENT_SIZE_LIMIT:
+        raise ValidationError(
+            'Attachment is too big. We allow max %(size)s MiB',
+            params = {
+                'size': pybb_settings.ATTACHMENT_SIZE_LIMIT/1024/1024,
+            }
+            )
+
+    # Checks by file extension
+    splitted_fn = attachment.name.rsplit('.', maxsplit=2)
+    if len(splitted_fn) == 1:
+        raise ValidationError(
+            'We do not allow uploading files without an extension.'
+            )
+
+    ext = splitted_fn[-1]
+    if not ext in settings.ALLOWED_EXTENSIONS:
+        raise ValidationError(
+            'This type of file is not allowed.'
+            )
+
+    # Widelands map file
+    if ext == 'wmf':
+        raise ValidationError(
+            'This seems to be a widelands map file. Please upload \
+            it at our maps section.'
+        )
+
+    # Widelands savegame (*.wgf) and widelands replay (*.wrpl.wgf)
+    # are not the same.
+    if ext == 'wgf' and not splitted_fn[-2] == 'wrpl':
+        if not _zip_contains(settings.WGF_CONTENT_CHECK):
+            raise ValidationError(
+                'This is not a valid widelands savegame.'
+            )
+
+    # Widelands replay
+    if ext == 'wrpl':
+        raise ValidationError(
+            'This file is part of a replay. Please zip it together with \
+            the corresponding .wrpl.wgf file and upload again.'
+        )
+
+    if ext == 'zip':
+        if _is_zip() == None:
+            raise ValidationError(
+                'This is not a valid zip file.'
+            )
+
+    # Widelands AI configuration
+    if ext == 'wai':
+        wai = configparser.ConfigParser()
+        try:
+            wai.read(tmp_file_path)
+            wai_sections = wai.sections()
+            if len(settings.ALLOWED_WAI_SECTIONS) == len(wai_sections):
+                for section in settings.ALLOWED_WAI_SECTIONS:
+                    if section not in wai_sections:
+                        raise
+            else:
+                raise
+        except:
+            raise ValidationError(
+                'This not a valid wai file.'
+                )
+
+    # Checks by MimeType
+    # Get MIME-Type from python-magic
+    magic_mime = magic.from_file(tmp_file_path, mime=True)
+    magic_mime = _split_mime(magic_mime)
+    send_mime = _split_mime(attachment.content_type)
+
+    # Check for valid image file. Use te mime-type provided by python-magic,
+    # because for a renamed image the wrong mime-type is send by the browser.
+    if magic_mime['maintype'] == 'image':
+        if not _is_image():
+            raise ValidationError(
+                'This is not a valid image: %(file)s',
+                params={'file': attachment.name}
+            )
+
+    # Compare Mime type send by browser and Mime type from python-magic.
+    # We only compare the main type (the first part) because the second
+    # part may not be recoginzed correctly. E.g. for .lua the submitted
+    # type is 'text/x-lua' but 'x-lua' is not official at all. See:
+    # https://www.iana.org/assignments/media-types/media-types.xhtml
+    # Unrecoginzed extension are always send with mime type
+    # 'application/octet-stream'. Skip if we know them. 
+    if not ext in settings.SKIP_MIME_EXTENSIONS:
+        if not magic_mime['maintype'] == send_mime['maintype']:
+            raise ValidationError(
+                'The file %(file)s looks like %(send_mime)s, \
+                but we think it is %(magic_mime)s',
+                params={
+                    'file': attachment.name,
+                    'send_mime': send_mime['maintype'],
+                    'magic_mime': magic_mime['maintype'],
+                    },
+            )

=== modified file 'pybb/views.py'
--- pybb/views.py	2019-05-27 18:18:14 +0000
+++ pybb/views.py	2019-09-02 20:01:34 +0000
@@ -11,6 +11,7 @@
 from django.shortcuts import redirect
 from django.db.models import Q
 from django.http import Http404
+from django.conf import settings
 
 from pybb.util import render_to, build_form, quote_text, ajax, urlize
 from pybb.models import Category, Forum, Topic, Post, Attachment,\
@@ -77,8 +78,12 @@
 
 
 def show_topic_ctx(request, topic_id):
+    """View of topic posts including a form to add a Post."""
+
+    context = {}
     try:
         topic = Topic.objects.select_related().get(pk=topic_id)
+        context.update({'topic': topic})
     except Topic.DoesNotExist:
         raise Http404()
 
@@ -92,25 +97,37 @@
         topic.update_read(request.user)
 
     last_post = topic.posts.order_by('-created')[0]
+    context.update({'last_post': last_post})
 
     initial = {}
+    user_is_mod = False
     if request.user.is_authenticated:
         initial = {'markup': 'markdown'}
-    form = AddPostForm(topic=topic, initial=initial)
-
-    user_is_mod = pybb_moderated_by(topic, request.user)
-    subscribed = (request.user.is_authenticated and
-                  request.user in topic.subscribers.all())
-
-    is_spam = False
-    if topic.is_hidden:
-            is_spam = topic.posts.first().is_spam()
+
+        form = AddPostForm(topic=topic,
+                           initial=initial,
+                           user=request.user,
+                           )
+        context.update({'form': form})
+
+        user_is_mod = pybb_moderated_by(topic, request.user)
+        context.update({'user_is_mod': user_is_mod})
+        
+        subscribed = (request.user.is_authenticated and
+                      request.user in topic.subscribers.all())
+        context.update({'subscribed': subscribed})
+    
+        is_spam = False
+        if topic.is_hidden:
+                is_spam = topic.posts.first().is_spam()
+        context.update({'is_spam': is_spam})
 
     if user_is_mod:
         posts = topic.posts.select_related()
     else:
         posts = topic.posts.exclude(hidden=True).select_related()
- 
+    context.update({'posts': posts})
+
     # TODO: fetch profiles
     # profiles = Profile.objects.filter(user__pk__in=
     #     set(x.user.id for x in page.object_list))
@@ -119,23 +136,24 @@
     # for post in page.object_list:
     #     post.user.pybb_profile = profiles[post.user.id]
 
-    load_related(posts, Attachment.objects.all(), 'post')
-
-    return {'topic': topic,
-            'last_post': last_post,
-            'form': form,
-            'user_is_mod': user_is_mod,
-            'subscribed': subscribed,
-            'posts': posts,
-            'page_size': pybb_settings.TOPIC_PAGE_SIZE,
-            'form_url': reverse('pybb_add_post', args=[topic.id]),
-            'is_spam': is_spam,
-            }
+    if pybb_settings.PYBB_ATTACHMENT_ENABLE:
+        load_related(posts, Attachment.objects.all(), 'post')
+
+    context.update({
+        'page_size': pybb_settings.TOPIC_PAGE_SIZE,
+        'form_url': reverse('pybb_add_post', args=[topic.id]),
+        'wikipage': settings.ATTACHMENT_DESCR_PAGE,
+    })
+
+    return context
+
 show_topic = render_to('pybb/topic.html')(show_topic_ctx)
 
 
 @login_required
 def add_post_ctx(request, forum_id, topic_id):
+    """ Standalone view for adding posts."""
+
     forum = None
     topic = None
 
@@ -224,6 +242,7 @@
             'topic': topic,
             'forum': forum,
             'form_url': form_url,
+            'wikipage': settings.ATTACHMENT_DESCR_PAGE,
             }
 add_post = render_to('pybb/add_post.html')(add_post_ctx)
 
@@ -297,7 +316,7 @@
     if not allowed:
         return HttpResponseRedirect(post.get_absolute_url())
 
-    if 'POST' == request.method:
+    if request.method == 'POST':
         topic = post.topic
         forum = post.topic.forum
         post.delete()
@@ -352,12 +371,12 @@
     return HttpResponseRedirect(reverse('pybb_topic', args=[topic.id]))
 
 
-@login_required
 def show_attachment(request, hash):
     attachment = get_object_or_404(Attachment, hash=hash)
-    file_obj = file(attachment.get_absolute_path())
-    return HttpResponse(file_obj, content_type=attachment.content_type)
-
+    
+    with open(attachment.get_absolute_path(), 'rb') as file_obj:
+        return HttpResponse(file_obj, content_type=attachment.content_type)
+    return HTTP404
 
 @login_required
 @ajax

=== modified file 'wlmaps/forms.py'
--- wlmaps/forms.py	2019-07-12 08:11:24 +0000
+++ wlmaps/forms.py	2019-09-02 20:01:34 +0000
@@ -6,30 +6,32 @@
 
 from django import forms
 from django.forms import ModelForm
+from django.conf import settings
 from django.core.files.storage import default_storage
-from django.conf import settings
 
 from wlmaps.models import Map
 import os
 import shutil
+import tempfile
 
 
 class UploadMapForm(ModelForm):
     """
+    All file operations are done in the temp folder.
+    
     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.
+    1. The map which is uploaded, stored as e.g '/tmp/tmp_xyz.upload'.
+       Because 'wl_map_info' can't handle files with this extension and
+       produces a minimap with the same name like the initial name, the
+       uploade file will be copied to a file with the name of the
+       uploaded file and extension, e.g. '/tmp/original_filename.wmf'
+    2. 'wl_map_info' produces two files which will be stored at the same
+        location as the uploaded file and named like the map file:
+        a. '/tmp/original_filename.wmf.json': Contains infos of the map
+        b. '/tmp/original_filename.wmf.png': The image of the minimap (png).
+
+    Because we can't be sure the original filename is a valid filename, we
+    may modify it to be a valid filename. 
     """
 
     class Meta:
@@ -39,44 +41,47 @@
     def clean(self):
         cleaned_data = super(UploadMapForm, self).clean()
 
-        mem_file_obj = cleaned_data.get('file')
-        if not mem_file_obj:
+        file_obj = cleaned_data.get('file')
+        if not file_obj:
             # no clean file => abort
             return cleaned_data
 
-        try:
-            # Try to make a safe filename
-            safe_name = default_storage.get_valid_name(mem_file_obj.name)
-            file_path = settings.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 contains characters which cannot be handled. Please rename and upload again.'])
-            del cleaned_data['file']
-            return cleaned_data
+        # Make a save filename and copy the uploaded file
+        saved_file = file_obj.temporary_file_path()
+        safe_name = default_storage.get_valid_name(file_obj.name)
+        tmpdir = tempfile.gettempdir()
+        copied_file = shutil.copyfile(saved_file,
+                                      os.path.join(tmpdir, safe_name)
+                                      )
 
         try:
             # call map info tool to generate minimap and json info file
+            # change working directory so that the datadir is found
             old_cwd = os.getcwd()
             os.chdir(settings.WIDELANDS_SVN_DIR)
-            check_call(['wl_map_info', saved_file])
-
-            # Deleting the file because it will be saved again when
-            # the model is saved.
-            default_storage.delete(saved_file)
+            check_call(['wl_map_info', copied_file])
             os.chdir(old_cwd)
         except CalledProcessError:
             self._errors['file'] = self.error_class(
                 ['The map file could not be processed.'])
             del cleaned_data['file']
+            os.remove(copied_file)
             return cleaned_data
 
-        mapinfo = json.load(open(saved_file + '.json'))
+        mapinfo = json.load(open(copied_file + '.json'))
 
         if Map.objects.filter(name=mapinfo['name']):
             self._errors['file'] = self.error_class(
                 ['A map with the same name already exists.'])
             del cleaned_data['file']
+            # Delete the file copy and the generated files
+            try:
+                os.remove(copied_file)
+                os.remove(copied_file + '.json')
+                os.remove(copied_file + '.png')
+            except os.FileNotFoundError:
+                pass
+
             return cleaned_data
 
         # Add information to the map
@@ -90,19 +95,18 @@
         self.instance.world_name = mapinfo['world_name']
         self.instance.wl_version_after = mapinfo['needs_widelands_version_after']
 
-        # mapinfo["minimap"] is the absolute path containing the path where it
-        # is saved, extract the name
+        # mapinfo["minimap"] is the absolute path to the image file
+        # We move the file to the correct destination
         minimap_name = mapinfo['minimap'].rpartition('/')[2]
         minimap_upload_to = self.instance._meta.get_field('minimap').upload_to
-        # Set the destination relative to MEDIA_ROOT
         minimap_path = os.path.join(minimap_upload_to, minimap_name)
         self.instance.minimap = minimap_path
-        # Move the minimap file (.png) from wlmaps/maps to wlmaps/minimaps
         shutil.move(mapinfo['minimap'], os.path.join(
             settings.MEDIA_ROOT, minimap_path))
 
-        # the json file is no longer needed
-        default_storage.delete(saved_file + '.json')
+        # Final cleanup
+        os.remove(copied_file + '.json')
+        os.remove(copied_file)
 
         return cleaned_data