From 244169d6d52806266481a034670cb89c2b081508 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Fri, 8 Jan 2021 14:50:23 +0100 Subject: [PATCH] make sure to generate valid filenames (#2238) --- UPGRADING.md | 1 + src/Export/Base/PDFRenderer.php | 9 +++-- src/Invoice/InvoiceFilename.php | 13 ++++--- src/Utils/FileHelper.php | 17 +++++++++ tests/Command/InvoiceCreateCommandTest.php | 2 +- tests/Invoice/InvoiceFilenameTest.php | 10 +++--- tests/Utils/FileHelperTest.php | 42 ++++++++++++++++++++++ 7 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 tests/Utils/FileHelperTest.php diff --git a/UPGRADING.md b/UPGRADING.md index c007f473..9122fbbb 100644 --- a/UPGRADING.md +++ b/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) diff --git a/src/Export/Base/PDFRenderer.php b/src/Export/Base/PDFRenderer.php index 21661c4a..c9aa1bd8 100644 --- a/src/Export/Base/PDFRenderer.php +++ b/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); diff --git a/src/Invoice/InvoiceFilename.php b/src/Invoice/InvoiceFilename.php index 40179d6b..00ae37f0 100644 --- a/src/Invoice/InvoiceFilename.php +++ b/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; diff --git a/src/Utils/FileHelper.php b/src/Utils/FileHelper.php index e4e8c312..d4451307 100644 --- a/src/Utils/FileHelper.php +++ b/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; + } } diff --git a/tests/Command/InvoiceCreateCommandTest.php b/tests/Command/InvoiceCreateCommandTest.php index 632c599d..9e4cf865 100644 --- a/tests/Command/InvoiceCreateCommandTest.php +++ b/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) diff --git a/tests/Invoice/InvoiceFilenameTest.php b/tests/Invoice/InvoiceFilenameTest.php index 2706e0ec..4584757b 100644 --- a/tests/Invoice/InvoiceFilenameTest.php +++ b/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()); } } diff --git a/tests/Utils/FileHelperTest.php b/tests/Utils/FileHelperTest.php new file mode 100644 index 00000000..dcbfbc89 --- /dev/null +++ b/tests/Utils/FileHelperTest.php @@ -0,0 +1,42 @@ +@' . "\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)); + } +}