make sure to generate valid filenames (#2238)

UPGRADING.md

@@ -11,6 +11,7 @@ Perform EACH version specific task between your version and the new one, otherwi
 ## [1.13](https://github.com/kevinpapst/kimai2/releases/tag/1.13)
 
 - Deprecated `now` variable in export templates: create it yourself with `{% set now = create_date('now', app.user) %}`
+- Changed invoice filename generation (check if you use cronjob for invoices)
 
 ## [1.12](https://github.com/kevinpapst/kimai2/releases/tag/1.12)
 

src/Export/Base/PDFRenderer.php

@@ -13,6 +13,7 @@ use App\Export\ExportContext;
 use App\Export\ExportItemInterface;
 use App\Repository\ProjectRepository;
 use App\Repository\Query\TimesheetQuery;
+use App\Utils\FileHelper;
 use App\Utils\HtmlToPdfConverter;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\ResponseHeaderBag;
@@ -94,7 +95,7 @@ class PDFRenderer
     public function render(array $timesheets, TimesheetQuery $query): Response
     {
         $context = new ExportContext();
-        $context->setOption('filename', 'kimai-export.pdf');
+        $context->setOption('filename', 'kimai-export');
 
         $summary = $this->calculateSummary($timesheets);
         $content = $this->twig->render($this->getTemplate(), array_merge([
@@ -116,10 +117,12 @@ class PDFRenderer
 
         $filename = $context->getOption('filename');
         if (empty($filename)) {
-            $filename = 'kimai-export.pdf';
+            $filename = 'kimai-export';
         }
 
-        $disposition = $response->headers->makeDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT, $filename);
+        $filename = FileHelper::convertToAsciiFilename($filename);
+
+        $disposition = $response->headers->makeDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT, $filename . '.pdf');
 
         $response->headers->set('Content-Type', 'application/pdf');
         $response->headers->set('Content-Disposition', $disposition);

src/Invoice/InvoiceFilename.php

@@ -10,7 +10,7 @@
 namespace App\Invoice;
 
 use App\Entity\Project;
-use Symfony\Component\String\UnicodeString;
+use App\Utils\FileHelper;
 
 final class InvoiceFilename
 {
@@ -31,8 +31,7 @@ final class InvoiceFilename
         }
 
         if (!empty($company)) {
-            $uCompany = new UnicodeString($company);
-            $filename .= '-' . $uCompany->ascii()->snake();
+            $filename .= '-' . $this->convert($company);
         }
 
         if (null !== $model->getQuery()) {
@@ -40,8 +39,7 @@ final class InvoiceFilename
             if (\count($projects) === 1) {
                 $pName = $projects[0];
                 if ($pName instanceof Project) {
-                    $uProject = new UnicodeString($pName->getName());
-                    $filename .= '-' . $uProject->ascii()->snake();
+                    $filename .= '-' . $this->convert($pName->getName());
                 }
             }
         }
@@ -49,6 +47,11 @@ final class InvoiceFilename
         $this->filename = $filename;
     }
 
+    private function convert(string $filename): string
+    {
+        return FileHelper::convertToAsciiFilename($filename);
+    }
+
     public function getFilename()
     {
         return $this->filename;

src/Utils/FileHelper.php

@@ -10,6 +10,7 @@
 namespace App\Utils;
 
 use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\String\UnicodeString;
 
 final class FileHelper
 {
@@ -68,4 +69,20 @@ final class FileHelper
     {
         $this->filesystem->remove($filename);
     }
+
+    public static function convertToAsciiFilename(string $filename): string
+    {
+        $filename = new UnicodeString($filename);
+        $filename = (string) $filename->collapseWhitespace()->trim()->replace(PHP_EOL, '')->replace(' ', '_');
+
+        $dangerousCharacters = ['"', "'", '&', '/', '\\', '?', '#', '%'];
+        $filename = str_replace($dangerousCharacters, ' ', $filename);
+
+        $filename = new UnicodeString($filename);
+        $filename = (string) $filename->collapseWhitespace()->replace(' ', '_')->ascii()->trim();
+        $filename = preg_replace('/[^a-zA-Z0-9\x7f-\xff\-]++/', ' ', $filename);
+        $filename = str_replace(' ', '_', trim($filename));
+
+        return $filename;
+    }
 }

tests/Command/InvoiceCreateCommandTest.php

@@ -186,7 +186,7 @@ class InvoiceCreateCommandTest extends KernelTestCase
         $this->assertStringContainsString('| ID | Customer | Total | Filename                                                                |', $output);
         $this->assertStringContainsString('+----+----------+-------+-------------------------------------------------------------------------+', $output);
         $this->assertStringContainsString('| 1  | Test     | 0 EUR | /', $output);
-        $this->assertStringContainsString('/tests/_data/invoices/2020-001-test.html |', $output);
+        $this->assertStringContainsString('/tests/_data/invoices/' . ((new \DateTime())->format('Y')) . '-001-Test.html |', $output);
     }
 
     protected function prepareFixtures(\DateTime $start)

tests/Invoice/InvoiceFilenameTest.php

@@ -46,11 +46,11 @@ class InvoiceFilenameTest extends TestCase
         self::assertEquals($datePrefix . '-foo', $sut->getFilename());
         self::assertEquals($datePrefix . '-foo', (string) $sut);
 
-        $customer->setCompany('barß / laölala # ldksjf 123');
+        $customer->setCompany('barß / laölala #   ldksjf 123 MyAwesome GmbH');
         $sut = new InvoiceFilename($model);
 
-        self::assertEquals($datePrefix . '-barss_laolala_ldksjf123', $sut->getFilename());
-        self::assertEquals($datePrefix . '-barss_laolala_ldksjf123', (string) $sut);
+        self::assertEquals($datePrefix . '-barss_laolala_ldksjf_123_MyAwesome_GmbH', $sut->getFilename());
+        self::assertEquals($datePrefix . '-barss_laolala_ldksjf_123_MyAwesome_GmbH', (string) $sut);
 
         $customer->setCompany('까깨꺄꺠꺼께껴꼐꼬꽈sssss');
         $sut = new InvoiceFilename($model);
@@ -58,7 +58,7 @@ class InvoiceFilenameTest extends TestCase
 
         $customer->setCompany('\"#+ß.!$%&/()=?\\n=/*-+´_<>@' . "\n");
         $sut = new InvoiceFilename($model);
-        self::assertEquals($datePrefix . '-ss_n', $sut->getFilename());
+        self::assertEquals($datePrefix . '-ss_n_-', $sut->getFilename());
 
         $project = new Project();
         $project->setName('Demo ProjecT1');
@@ -69,6 +69,6 @@ class InvoiceFilenameTest extends TestCase
 
         $customer->setCompany('\"#+ß.!$%&/()=?\\n=/*-+´_<>@' . "\n");
         $sut = new InvoiceFilename($model);
-        self::assertEquals($datePrefix . '-ss_n-demo_projec_t1', $sut->getFilename());
+        self::assertEquals($datePrefix . '-ss_n_--Demo_ProjecT1', $sut->getFilename());
     }
 }

tests/Utils/FileHelperTest.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Utils;
+
+use App\Utils\FileHelper;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \App\Utils\FileHelper
+ */
+class FileHelperTest extends TestCase
+{
+    public function getFileTestData()
+    {
+        return [
+            ['Barss_laolala_ld_ksjf_123_MyAwesome_GmbH', 'Barß / laölala #   ld_ksjf 123 MyAwesome GmbH'],
+            ['namaste', 'नमस्ते'],
+            ['sa_yonara', 'さ!よなら'],
+            ['sp_asibo_spa_sibo_spas_--_ibo', ' сп.асибо/спа   сибо#/!спас -- ибо!!'],
+            ['kkakkaekkyakkyaekkeokke_kkyeokkyekkokkwasssss', '까깨꺄꺠꺼께_껴꼐꼬꽈sssss'],
+            ['ss_n_-', '\"#+ß.!$%&/()=?\\n=/*-+´_<>@' . "\n"],
+            ['Demo_ProjecT1', 'Demo ProjecT1'],
+            ['kimai-export', 'kimai-export'],
+            ['D_e_m_o_Pr_oj_e_c_T1', 'D"e&m%o# Pr\'oj\\e/c?T1'],
+        ];
+    }
+
+    /**
+     * @dataProvider getFileTestData
+     */
+    public function testEnsureMaxLength(string $expected, string $original)
+    {
+        self::assertEquals($expected, FileHelper::convertToAsciiFilename($original));
+    }
+}