[24.0] Fix Archive header encoding #18583
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: lib/galaxy/util/zipstream.py
Did you find this useful? React with a 👍 or 👎 |
lib/galaxy/util/zipstream.py
Outdated
| @@ -41,7 +42,7 @@ def response(self) -> Iterator[bytes]: | |||
| def get_headers(self) -> Dict[str, str]: | |||
| headers = {} | |||
| if self.archive_name: | |||
| headers["Content-Disposition"] = f'attachment; filename="{self.archive_name}.zip"' | |||
| headers["Content-Disposition"] = f'attachment; filename="{decodify(self.archive_name)}.zip"' | |||
There was a problem hiding this comment.
Can we instead just do the inline call ? Looking at this I know exactly what it's going to do, looking at a function like decodify I don't.
| headers["Content-Disposition"] = f'attachment; filename="{decodify(self.archive_name)}.zip"' | |
| headers["Content-Disposition"] = f'attachment; filename="{self.archive_name.encode('latin-1', 'replace').decode('latin-1')}.zip"' |
?
There was a problem hiding this comment.
Because this problem does not just belong to this specific purpose (#18584), we could use it in other places.
There was a problem hiding this comment.
There was a problem hiding this comment.
Sure, I will make this PR specific to the issue we are addressing.
However, why are we using 'latin-1' encoding? It might not display the correct characters when downloading.
There was a problem hiding this comment.
lib/galaxy/util/__init__.py
Outdated
| @@ -1194,6 +1194,25 @@ def unicodify( | |||
| return value | |||
|
|
|||
|
|
|||
| def decodify(value, encoding="utf-8", encoding_error="replace", decoding="latin-1", decoding_error="replace"): | |||
There was a problem hiding this comment.
If we do decide for this I would narrow the type for value to string and really only do value.encode(encoding).decode(encoding). The rest we can do when and if we need it. I would not like to replicate unicodify logic here.
|
As a bugfix this should go to 24.0 |
arash77
force-pushed
the
decode-archive-header
branch
from
00cb14c to
5beb876
Compare
|
This PR was merged without a "kind/" label, please correct. |
nsoranzo
changed the title
Fix the encoding of the filename in the archive header when downloading a collection with non-English characters.
Fixes #18508.
How to test the changes?
(Select all options that apply)
License