Repository: falconry/falcon
Pull request #1775 (2020-09-14 10:44:17)

fix(Response.downloadable_as): properly handle comma in unicode filenames
When `dowloadable_as` was updated to handle unicode in filenames in https://github.com/falconry/falcon/pull/1750 a bug was introduced when the file has unicode character and a has comma in them.

This fixes the bug by escaping also the commas.

Closes https://github.com/falconry/falcon/issues/1749 (again)

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1775?src=pr&el=h1) Report
> Merging [#1775](https://codecov.io/gh/falconry/falcon/pull/1775?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/6a962daf779fe580e43e4c5c7dae0abc6737c29e?el=desc) will **decrease** coverage by `32.03%`.
> The diff coverage is `n/a`.

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

```diff
@@             Coverage Diff              @@
##            master    #1775       +/-   ##
============================================
- Coverage   100.00%   67.96%   -32.04%     
============================================
  Files           53       53               
  Lines         4445     4445               
  Branches       701      701               
============================================
- Hits          4445     3021     -1424     
- Misses           0     1414     +1414     
- Partials         0       10       +10     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1775?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/response\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlX2hlbHBlcnMucHk=) | `100.00% <ø> (ø)` | |
| [falcon/asgi/app.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/reader.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVhZGVyLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/stream.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RyZWFtLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/request.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVxdWVzdC5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/response.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVzcG9uc2UucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/multipart.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvbXVsdGlwYXJ0LnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/structures.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RydWN0dXJlcy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/\_request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvX3JlcXVlc3RfaGVscGVycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `47.58% <0.00%> (-52.42%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/falconry/falcon/pull/1775/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1775?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/1775?src=pr&el=footer). Last update [6a962da...21cb65d](https://codecov.io/gh/falconry/falcon/pull/1775?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

vytas7: Nice catch! :+1: 

Patch:
diff --git a/falcon/response_helpers.py b/falcon/response_helpers.py
index 9337803..e206699 100644
--- a/falcon/response_helpers.py
+++ b/falcon/response_helpers.py
@@ -102,7 +102,7 @@ def format_content_disposition(value):
     #     "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))
+        secure_filename(value), uri.encode_value(value))
 
 
 def format_etag_header(value):


Test patch:
diff --git a/tests/test_headers.py b/tests/test_headers.py
index ce72a22..fe514e7 100644
--- a/tests/test_headers.py
+++ b/tests/test_headers.py
@@ -500,6 +500,18 @@ class TestHeaders:
             'attachment; filename=A_ngstro_m_unit.txt; '
             "filename*=UTF-8''%C3%85ngstr%C3%B6m%20unit.txt",
         ),
+        ('one,two.txt', 'attachment; filename="one,two.txt"'),
+        (
+            '½,²⁄₂.txt',
+            'attachment; filename=1_2_2_2.txt; '
+            "filename*=UTF-8''%C2%BD%2C%C2%B2%E2%81%84%E2%82%82.txt"
+        ),
+        ('[foo] @ bar.txt', 'attachment; filename="[foo] @ bar.txt"'),
+        (
+            '[fòó]@bàr,bäz.txt',
+            'attachment; filename=_fo_o___ba_r_ba_z.txt; '
+            "filename*=UTF-8''%5Bf%C3%B2%C3%B3%5D%40b%C3%A0r%2Cb%C3%A4z.txt"
+        ),
     ])
     def test_content_disposition_header(self, client, filename, expected):
         resource = DownloadableResource(filename)