search

ImperialCollegeLondon / django-drf-filepond

A Django app providing a server implemention for the Filepond file upload library
BSD 3-Clause "New" or "Revised" License
105 stars 40 forks source link

Support cyrillic in filenames: escape_uri_path for Content-Disposition header in responses #76

Closed roman-tiukh closed 11 months ago

jcohen02 commented 2 years ago

Hi @roman-tiukh, many thanks for this PR, a very useful fix. I think it's important that the test suite also includes tests to check valid processing of filenames both with and without Cyrillic letters. Would you be happy to add some tests to verify this?

At present, I believe the views that would need testing are django_drf_filepond.views.LoadView (GET), django_drf_filepond.views.RestoreView (GET) and django_drf_filepond.views.FetchView (HEAD and GET).

These views are currently tested in tests.test_load_view, tests.test_restore_view and tests.test_fetch_view respectively.

For example in tests.test_fetch_view there's a test test_fetch_with_filename, where the filename is set to test.txt, I guess we could simply duplicate that test and include a filename with Cyrillic letters? Or maybe just bundle the testing of filenames with and without Cyrillic into the same test?

Currently the test suite can be run with python setup.py test.

If you're happy to add these tests, that would be great, otherwise I can have a go at doing this but it's likely to take me a few days to get to this.

roman-tiukh commented 2 years ago

ok I'll do this

jcohen02 commented 2 years ago

Hi @roman-tiukh, thanks for contributing to this project. I note that you're pushing a number of changes relating to thumbnails to this PR which is related to adding support for cyrillic text in filenames - is this intentional or this was done in error?

It would be good to keep this PR focused on the original intended change. I was waiting for you to add tests but I can try and find the time to do this if you're busy with other things.

With regard to thumbnail support, the aim is for django-drf-filepond to remain generic and handle the core upload functionality to support filepond clients. Things like thumbnail storage or generation would normally be handled indepdently of this app within your code.

If you have a specific use case where you feel this would be useful, it would be great if you could open an issue explaining the rationale for these updates so that we can discuss and see if this fits within the intended usage of django-drf-filepond.

Many thanks.

jcohen02 commented 11 months ago

@roman-tiukh, thanks again for highlighting this issue and contributing a suggested fix. Apologies for the long delay in getting this resolved. This has now been addressed in #108 which provides a solution that works across all supported Python versions for this library and generates an RFC6266-compliant content disposition header.