Summary: | Commas in file names of uploaded files cause inconsistently broken 856$u links | ||
---|---|---|---|
Product: | Koha | Reporter: | Andrew Fuerste-Henry <andrew> |
Component: | Cataloging | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Nick Clemens (kidclamp) <nick> |
Severity: | normal | ||
Priority: | P5 - low | CC: | dcook, fridolin.somers, jonathan.druart, m.de.rooy, nick, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
21.05.00,20.11.05,20.05.11
|
Circulation function: | |
Attachments: |
Bug 28123: Quote filename value in Content-Disposition header
Bug 28123: Fix expected value in unit test Bug 28123: Quote filename value in Content-Disposition header Bug 28123: Fix expected value in unit test Bug 28123: Quote filename value in Content-Disposition header Bug 28123: Fix expected value in unit test |
Description
Andrew Fuerste-Henry
2021-04-09 14:33:05 UTC
Working OK for me on master, tested with Firefox and Chromium (87). Ah yes I know this problem from working on DSpace. I think that it's a Chrome-only bug. This is actually just a problem for PDF downloads. Created attachment 119494 [details] [review] Bug 28123: Quote filename value in Content-Disposition header Chrome throws a ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION error if a Content-Disposition header includes an unquoted filename that contains commas. This patch adds quotes around the filename in the Content-Disposition header, which fixes the problem. Test plan: 0. Do not apply the patch yet 1. Obtain a PDF and rename it to include a comma in its name 2. Enable the upload.pl plugin in the 856$u of the Default framework 3. Add/edit a record in the Default Framework and upload a file using the 856$u upload plugin. 4. Click on the resulting file URL in the saved file 5. Note the error ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION is returned instead of the file 6. Apply the patch and restart your Plack instance 7. Click on the resulting file URL in the saved file 8. Note that you now get the PDF file instead of the error Created attachment 119495 [details] [review] Bug 28123: Fix expected value in unit test Created attachment 119610 [details] [review] Bug 28123: Quote filename value in Content-Disposition header Chrome throws a ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION error if a Content-Disposition header includes an unquoted filename that contains commas. This patch adds quotes around the filename in the Content-Disposition header, which fixes the problem. Test plan: 0. Do not apply the patch yet 1. Obtain a PDF and rename it to include a comma in its name 2. Enable the upload.pl plugin in the 856$u of the Default framework 3. Add/edit a record in the Default Framework and upload a file using the 856$u upload plugin. 4. Click on the resulting file URL in the saved file 5. Note the error ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION is returned instead of the file 6. Apply the patch and restart your Plack instance 7. Click on the resulting file URL in the saved file 8. Note that you now get the PDF file instead of the error Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 119611 [details] [review] Bug 28123: Fix expected value in unit test Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 119684 [details] [review] Bug 28123: Quote filename value in Content-Disposition header Chrome throws a ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION error if a Content-Disposition header includes an unquoted filename that contains commas. This patch adds quotes around the filename in the Content-Disposition header, which fixes the problem. Test plan: 0. Do not apply the patch yet 1. Obtain a PDF and rename it to include a comma in its name 2. Enable the upload.pl plugin in the 856$u of the Default framework 3. Add/edit a record in the Default Framework and upload a file using the 856$u upload plugin. 4. Click on the resulting file URL in the saved file 5. Note the error ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION is returned instead of the file 6. Apply the patch and restart your Plack instance 7. Click on the resulting file URL in the saved file 8. Note that you now get the PDF file instead of the error Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 119685 [details] [review] Bug 28123: Fix expected value in unit test Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.05 Pushed to 20.05.x for 20.05.11 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |