From 1237b8ba47f888bee658f5050a01998e0c5f6f3c Mon Sep 17 00:00:00 2001
From: Raphael Michel <michel@rami.io>
Date: Tue, 23 May 2023 12:17:06 +0200
Subject: [PATCH] Invoice: Improve handling of special characters in file names
 (#3347)

Co-authored-by: Richard Schreiber <schreiber@rami.io>
---
 src/pretix/base/services/mail.py   | 10 +++++++++-
 src/pretix/base/settings.py        | 22 ++++++++++++++++++++++
 src/pretix/control/views/orders.py |  2 +-
 src/pretix/presale/views/order.py  |  2 +-
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/pretix/base/services/mail.py b/src/pretix/base/services/mail.py
index 68b1406d73..908b8b16df 100644
--- a/src/pretix/base/services/mail.py
+++ b/src/pretix/base/services/mail.py
@@ -466,9 +466,17 @@ def mail_send_task(self, *args, to: List[str], subject: str, body: str, html: st
             for inv in invoices:
                 if inv.file:
                     try:
+                        # We try to give the invoice a more human-readable name, e.g. "Invoice_ABC-123.pdf" instead of
+                        # just "ABC-123.pdf", but we only do so if our currently selected language allows to do this
+                        # as ASCII text. For example, we would not want a "فاتورة_" prefix for our filename since this
+                        # has shown to cause deliverability problems of the email and deliverability wins.
+                        filename = pgettext('invoice', 'Invoice {num}').format(num=inv.number).replace(' ', '_') + '.pdf'
+                        if not re.match("^[a-zA-Z0-9-_%./,&:# ]+$", filename):
+                            filename = inv.number.replace(' ', '_') + '.pdf'
+                        filename = re.sub("[^a-zA-Z0-9-_.]+", "_", filename)
                         with language(inv.order.locale):
                             email.attach(
-                                pgettext('invoice', 'Invoice {num}').format(num=inv.number).replace(' ', '_') + '.pdf',
+                                filename,
                                 inv.file.file.read(),
                                 'application/pdf'
                             )
diff --git a/src/pretix/base/settings.py b/src/pretix/base/settings.py
index 29e641c008..b8c1936530 100644
--- a/src/pretix/base/settings.py
+++ b/src/pretix/base/settings.py
@@ -633,6 +633,17 @@ DEFAULTS = {
                         "used at most once over all of your events. This setting only affects future invoices. You can "
                         "use %Y (with century) %y (without century) to insert the year of the invoice, or %m and %d for "
                         "the day of month."),
+            validators=[
+                RegexValidator(
+                    # We actually allow more characters than we name in the error message since some of these characters
+                    # are in active use at the time of the introduction of this validation, so we can't really forbid
+                    # them, but we don't think they belong in an invoice number and don't want to advertise them.
+                    regex="^[a-zA-Z0-9-_%./,&:# ]+$",
+                    message=lazy(lambda *args: _('Please only use the characters {allowed} in this field.').format(
+                        allowed='A-Z, a-z, 0-9, -./:#'
+                    ), str)()
+                )
+            ],
         )
     },
     'invoice_numbers_prefix_cancellations': {
@@ -644,6 +655,17 @@ DEFAULTS = {
             label=_("Invoice number prefix for cancellations"),
             help_text=_("This will be prepended to invoice numbers of cancellations. If you leave this field empty, "
                         "the same numbering scheme will be used that you configured for regular invoices."),
+            validators=[
+                RegexValidator(
+                    # We actually allow more characters than we name in the error message since some of these characters
+                    # are in active use at the time of the introduction of this validation, so we can't really forbid
+                    # them, but we don't think they belong in an invoice number and don't want to advertise them.
+                    regex="^[a-zA-Z0-9-_%./,&:# ]+$",
+                    message=lazy(lambda *args: _('Please only use the characters {allowed} in this field.').format(
+                        allowed='A-Z, a-z, 0-9, -./:#'
+                    ), str)()
+                )
+            ],
         )
     },
     'invoice_renderer_highlight_order_code': {
diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py
index 0ad95570fc..6f37215251 100644
--- a/src/pretix/control/views/orders.py
+++ b/src/pretix/control/views/orders.py
@@ -1512,7 +1512,7 @@ class InvoiceDownload(EventPermissionRequiredMixin, View):
             invoice_pdf_task.apply(args=(self.invoice.pk,))
             return self.get(request, *args, **kwargs)
 
-        resp['Content-Disposition'] = 'inline; filename="{}.pdf"'.format(self.invoice.number)
+        resp['Content-Disposition'] = 'inline; filename="{}.pdf"'.format(re.sub("[^a-zA-Z0-9-_.]+", "_", self.invoice.number))
         resp._csp_ignore = True  # Some browser's PDF readers do not work with CSP
         return resp
 
diff --git a/src/pretix/presale/views/order.py b/src/pretix/presale/views/order.py
index 138ad84a77..e84c5b3454 100644
--- a/src/pretix/presale/views/order.py
+++ b/src/pretix/presale/views/order.py
@@ -1235,7 +1235,7 @@ class InvoiceDownload(EventViewMixin, OrderDetailMixin, View):
         except FileNotFoundError:
             invoice_pdf_task.apply(args=(invoice.pk,))
             return self.get(request, *args, **kwargs)
-        resp['Content-Disposition'] = 'inline; filename="{}.pdf"'.format(invoice.number)
+        resp['Content-Disposition'] = 'inline; filename="{}.pdf"'.format(re.sub("[^a-zA-Z0-9-_.]+", "_", invoice.number))
         resp._csp_ignore = True  # Some browser's PDF readers do not work with CSP
         return resp