Merge pull request #624 from taigaio/fixing-attachments-with-long-names

Fixing attachments with long names

taiga/base/utils/files.py

@@ -0,0 +1,42 @@
+# Copyright (C) 2014-2016 Andrey Antukh <niwi@niwi.be>
+# Copyright (C) 2014-2016 Jesús Espino <jespinog@gmail.com>
+# Copyright (C) 2014-2016 David Barragán <bameda@dbarragan.com>
+# Copyright (C) 2014-2016 Alejandro Alonso <alejandro.alonso@kaleidos.net>
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import hashlib
+
+from os import path, urandom
+from unidecode import unidecode
+
+from django.template.defaultfilters import slugify
+from django.utils import timezone
+from django.utils.encoding import force_bytes
+
+from taiga.base.utils.iterators import split_by_n
+
+def get_file_path(instance, filename, base_path):
+    basename = path.basename(filename).lower()
+    base, ext = path.splitext(basename)
+    base = slugify(unidecode(base))[0:100]
+    basename = "".join([base, ext])
+
+    hs = hashlib.sha256()
+    hs.update(force_bytes(timezone.now().isoformat()))
+    hs.update(urandom(1024))
+
+    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
+    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
+
+    return path.join(base_path, hash_part, basename)

taiga/projects/attachments/models.py

@@ -16,35 +16,20 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import hashlib
-import os
-import os.path as path
-
-from unidecode import unidecode
 
 from django.db import models
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.utils import timezone
-from django.utils.encoding import force_bytes
 from django.utils.translation import ugettext_lazy as _
 from django.utils.text import get_valid_filename
 
-from taiga.base.utils.iterators import split_by_n
+from taiga.base.utils.files import get_file_path
 
 
 def get_attachment_file_path(instance, filename):
-    basename = path.basename(filename)
-    basename = get_valid_filename(basename)
-
-    hs = hashlib.sha256()
-    hs.update(force_bytes(timezone.now().isoformat()))
-    hs.update(os.urandom(1024))
-
-    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
-    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
-
-    return path.join("attachments", hash_part, basename)
+    return get_file_path(instance, filename, "attachments")
 
 
 class Attachment(models.Model):

taiga/projects/migrations/0031_project_logo.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='project',
             name='logo',
-            field=models.FileField(null=True, blank=True, upload_to=taiga.projects.models.get_user_file_path, verbose_name='logo', max_length=500),
+            field=models.FileField(null=True, blank=True, upload_to=taiga.projects.models.get_project_logo_file_path, verbose_name='logo', max_length=500),
         ),
     ]

taiga/projects/models.py

@@ -15,14 +15,9 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import hashlib
-import os
-import os.path as path
 import itertools
 import uuid
 
-from unidecode import unidecode
-
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import models
@@ -32,8 +27,6 @@ from django.conf import settings
 from django.dispatch import receiver
 from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext_lazy as _
-from django.template.defaultfilters import slugify
-from django.utils.encoding import force_bytes
 from django.utils import timezone
 
 from django_pgjson.fields import JsonField
@@ -41,7 +34,7 @@ from djorm_pgarray.fields import TextArrayField
 
 from taiga.base.tags import TaggedMixin
 from taiga.base.utils.dicts import dict_sum
-from taiga.base.utils.iterators import split_by_n
+from taiga.base.utils.files import get_file_path
 from taiga.base.utils.sequence import arithmetic_progression
 from taiga.base.utils.slug import slugify_uniquely
 from taiga.base.utils.slug import slugify_uniquely_for_queryset
@@ -62,20 +55,8 @@ from . import choices
 from dateutil.relativedelta import relativedelta
 
 
-def get_user_file_path(instance, filename):
-    basename = path.basename(filename).lower()
-    base, ext = path.splitext(basename)
-    base = slugify(unidecode(base))
-    basename = "".join([base, ext])
-
-    hs = hashlib.sha256()
-    hs.update(force_bytes(timezone.now().isoformat()))
-    hs.update(os.urandom(1024))
-
-    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
-    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
-
-    return path.join("project", hash_part, basename)
+def get_project_logo_file_path(instance, filename):
+    return get_file_path(instance, filename, "project")
 
 
 class Membership(models.Model):
@@ -167,7 +148,7 @@ class Project(ProjectDefaults, TaggedMixin, models.Model):
     description = models.TextField(null=False, blank=False,
                                    verbose_name=_("description"))
 
-    logo = models.FileField(upload_to=get_user_file_path,
+    logo = models.FileField(upload_to=get_project_logo_file_path,
                              max_length=500, null=True, blank=True,
                              verbose_name=_("logo"))
 

taiga/users/models.py

@@ -15,15 +15,10 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import hashlib
-import os
-import os.path as path
 import random
 import re
 import uuid
 
-from unidecode import unidecode
-
 from django.apps import apps
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
@@ -33,15 +28,13 @@ from django.utils.translation import ugettext_lazy as _
 from django.contrib.auth.models import UserManager, AbstractBaseUser
 from django.core import validators
 from django.utils import timezone
-from django.utils.encoding import force_bytes
-from django.template.defaultfilters import slugify
 
 from django_pgjson.fields import JsonField
 from djorm_pgarray.fields import TextArrayField
 
 from taiga.auth.tokens import get_token_for_user
 from taiga.base.utils.slug import slugify_uniquely
-from taiga.base.utils.iterators import split_by_n
+from taiga.base.utils.files import get_file_path
 from taiga.permissions.permissions import MEMBERS_PERMISSIONS
 from taiga.projects.choices import BLOCKED_BY_OWNER_LEAVING
 from taiga.projects.notifications.choices import NotifyLevel
@@ -54,19 +47,7 @@ def generate_random_hex_color():
 
 
 def get_user_file_path(instance, filename):
-    basename = path.basename(filename).lower()
-    base, ext = path.splitext(basename)
-    base = slugify(unidecode(base))[0:100]
-    basename = "".join([base, ext])
-
-    hs = hashlib.sha256()
-    hs.update(force_bytes(timezone.now().isoformat()))
-    hs.update(os.urandom(1024))
-
-    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
-    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
-
-    return path.join("user", hash_part, basename)
+    return get_file_path(instance, filename, "user")
 
 
 class PermissionsMixin(models.Model):

tests/integration/test_attachments.py

@@ -45,3 +45,19 @@ def test_create_attachment_on_wrong_project(client):
     client.login(issue1.owner)
     response = client.post(url, data)
     assert response.status_code == 400
+
+
+def test_create_attachment_with_long_file_name(client):
+    issue1 = f.create_issue()
+    f.MembershipFactory(project=issue1.project, user=issue1.owner, is_owner=True)
+
+    url = reverse("issue-attachments-list")
+
+    data = {"description": "test",
+            "object_id": issue1.pk,
+            "project": issue1.project.id,
+            "attached_file": SimpleUploadedFile(500*"x"+".txt", b"test")}
+
+    client.login(issue1.owner)
+    response = client.post(url, data)
+    assert response.data["attached_file"].endswith("/"+100*"x"+".txt")

tests/integration/test_projects.py

@@ -629,6 +629,24 @@ def test_update_project_logo(client):
         assert not any(list(map(os.path.exists, original_photo_paths)))
 
 
+@pytest.mark.django_db(transaction=True)
+def test_update_project_logo_with_long_file_name(client):
+    user = f.UserFactory.create(is_superuser=True)
+    project = f.create_project()
+    url = reverse("projects-change-logo", args=(project.id,))
+
+    with NamedTemporaryFile(delete=False) as logo:
+        logo.name=500*"x"+".bmp"
+        logo.write(DUMMY_BMP_DATA)
+        logo.seek(0)
+
+        client.login(user)
+        post_data = {'logo': logo}
+        response = client.post(url, post_data)
+
+        assert response.status_code == 200
+
+
 @pytest.mark.django_db(transaction=True)
 def test_remove_project_logo(client):
     user = f.UserFactory.create(is_superuser=True)