Repository: falconry/falcon

Issue:
Response.downloadable_as does not follow RFC 6266 recommendations for UTF-8
At the time of writing, [Response.downloadable_as](https://falcon.readthedocs.io/en/stable/api/request_and_response.html#falcon.Response.downloadable_as) always uses the `attachment; filename="filename.ext"` form regardless of the provided filename. However, [RFC 6266](https://tools.ietf.org/html/rfc6266) specifies that a quoted `filename` shall be interpreted as ISO-8859-1 (although some agents are able to sniff UTF-8), and recommends using `filename*` whenever `filename` cannot be faithfully represented with US-ASCII.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=h1) Report
> Merging [#1750](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/113eea113844058816ed745d8dc9cb197e75cb95&el=desc) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1750/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##            master     #1750   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           55        55           
  Lines         5037      5041    +4     
  Branches       823       824    +1     
=========================================
+ Hits          5037      5041    +4     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/response.py](https://codecov.io/gh/falconry/falcon/pull/1750/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlLnB5) | `100.00% <ø> (ø)` | |
| [falcon/response\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1750/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlX2hlbHBlcnMucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=footer). Last update [113eea1...f7263b3](https://codecov.io/gh/falconry/falcon/pull/1750?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/_newsfragments/1749.bugfix.rst b/docs/_newsfragments/1749.bugfix.rst
new file mode 100644
index 0000000..96777e5
--- /dev/null
+++ b/docs/_newsfragments/1749.bugfix.rst
@@ -0,0 +1,3 @@
+The :attr:`Response.downloadable_as <falcon.Response.downloadable_as>` property
+is now correctly encoding non-ASCII filenames as per
+`RFC 6266 <https://tools.ietf.org/html/rfc6266#appendix-D>`_ recommendations.
diff --git a/falcon/response.py b/falcon/response.py
index dc3850b..f3ec961 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -942,9 +942,14 @@ class Response:
         'Content-Disposition',
         """Set the Content-Disposition header using the given filename.
 
-        The value will be used for the *filename* directive. For example,
+        The value will be used for the ``filename`` directive. For example,
         given ``'report.pdf'``, the Content-Disposition header would be set
         to: ``'attachment; filename="report.pdf"'``.
+
+        As per `RFC 6266 <https://tools.ietf.org/html/rfc6266#appendix-D>`_
+        recommendations, non-ASCII filenames will be encoded using the
+        ``filename*`` directive, whereas ``filename`` will contain the US
+        ASCII fallback.
         """,
         format_content_disposition)
 
diff --git a/falcon/response_helpers.py b/falcon/response_helpers.py
index 1d5fec0..81cecc9 100644
--- a/falcon/response_helpers.py
+++ b/falcon/response_helpers.py
@@ -14,6 +14,9 @@
 
 """Utilities for the Response class."""
 
+from falcon.util import uri
+from falcon.util.misc import isascii, secure_filename
+
 
 def header_property(name, doc, transform=None):
     """Create a header getter/setter.
@@ -81,7 +84,25 @@ def format_range(value):
 def format_content_disposition(value):
     """Formats a Content-Disposition header given a filename."""
 
-    return 'attachment; filename="' + value + '"'
+    # NOTE(vytas): RFC 6266, Appendix D.
+    #   Include a "filename" parameter when US-ASCII ([US-ASCII]) is
+    #   sufficiently expressive.
+    if isascii(value):
+        return 'attachment; filename="' + value + '"'
+
+    # NOTE(vytas): RFC 6266, Appendix D.
+    #   * Include a "filename*" parameter where the desired filename cannot be
+    #     expressed faithfully using the "filename" form.  Note that legacy
+    #     user agents will not process this, and will fall back to using the
+    #     "filename" parameter's content.
+    #   * When a "filename*" parameter is sent, to also generate a "filename"
+    #     parameter as a fallback for user agents that do not support the
+    #     "filename*" form, if possible.
+    #   * When a "filename" parameter is included as a fallback (as per above),
+    #     "filename" should occur first, due to parsing problems in some
+    #     existing implementations.
+    return "attachment; filename=%s; filename*=UTF-8''%s" % (
+        secure_filename(value), uri.encode(value))
 
 
 def format_etag_header(value):


Test patch:
diff --git a/tests/test_headers.py b/tests/test_headers.py
index 41f23f8..ce72a22 100644
--- a/tests/test_headers.py
+++ b/tests/test_headers.py
@@ -207,6 +207,16 @@ class RemoveHeaderResource:
         resp.downloadable_as = None
 
 
+class DownloadableResource:
+    def __init__(self, filename):
+        self.filename = filename
+
+    def on_get(self, req, resp):
+        resp.body = 'Hello, World!\n'
+        resp.content_type = falcon.MEDIA_TEXT
+        resp.downloadable_as = self.filename
+
+
 class ContentLengthHeaderResource:
 
     def __init__(self, content_length, body=None, data=None):
@@ -477,6 +487,28 @@ class TestHeaders:
             hist[name] += 1
             assert 1 == hist[name]
 
+    @pytest.mark.parametrize('filename,expected', [
+        ('report.csv', 'attachment; filename="report.csv"'),
+        ('Hello World.txt', 'attachment; filename="Hello World.txt"'),
+        (
+            'Bold Digit 𝟏.txt',
+            'attachment; filename=Bold_Digit_1.txt; '
+            "filename*=UTF-8''Bold%20Digit%20%F0%9D%9F%8F.txt",
+        ),
+        (
+            'Ångström unit.txt',
+            'attachment; filename=A_ngstro_m_unit.txt; '
+            "filename*=UTF-8''%C3%85ngstr%C3%B6m%20unit.txt",
+        ),
+    ])
+    def test_content_disposition_header(self, client, filename, expected):
+        resource = DownloadableResource(filename)
+        client.app.add_route('/', resource)
+        resp = client.simulate_get()
+
+        assert resp.status_code == 200
+        assert resp.headers['Content-Disposition'] == expected
+
     def test_unicode_location_headers(self, client):
         client.app.add_route('/', LocationHeaderUnicodeResource())