Mahara

Escape filenames with quotes in them, in Content-Disposition:attachment headers

Bug #1578512 reported by Aaron Wells on 2016-05-05

This bug affects 1 person

	Status	Importance	Assigned to	Milestone
Mahara	Fix Released	Medium	Unassigned
15.04	Fix Released	Medium	Unassigned	Mahara 15.04.8
15.10	Fix Released	Medium	Unassigned	Mahara 15.10.4
16.04	Fix Released	Medium	Unassigned	Mahara 16.04.1
16.10	Fix Released	Medium	Unassigned	Mahara 16.10.0

Bug Description

If you give a file in Mahara a name that has doublequotes in it and try to download it, your browser will name the download "download.php" instead of the correct file name. This is because we're not properly escaping the double quotes in the filenames in the "Content-Disposition" header.

In fact, in the relevant code in htdocs/lib/file.php, there's a note from 2007: "@todo possibly need addslashes on the filename, but I'm unsure on exactly how the browsers will handle it."

Well, I tracked it down to the RFCs recently to find out the right format for this. The Content-Disposition header's usage in HTTP headers is defined in RFC 6266: https://tools.ietf.org/html/rfc6266#section-4.1 . The filename there is defined to be a "quoted-string". The format of quoted-string is defined in RFC2616: https://tools.ietf.org/html/rfc2616#section-2.2 There, it says that if your quoted string contains double quotes, you can escape them with a single backslash.

So there you go. We just need to replace any " in the filename with \"

Revision history for this message

Aaron Wells (u-aaronw) wrote on 2016-05-05:

To replicate:

1. Go to File -> Contents
2. Upload a file called test.txt
3. Once uploaded, click the pencil icon to edit test.txt
4. Change its name to t"est.txt (or something else with double quotes in or around it). Press "Save changes"
5. Click on the title of the file, to trigger a download

Expected result: The browser offers to download a file called t"est.txt

Actual result: The browser offers to download a file called t, or whatever portion of the filename preceeds the first double quote. If the first character in the filename is a double quote, the browser offers to download a file called "download.php".

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05: A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/6457

Revision history for this message

Aaron Wells (u-aaronw) wrote on 2016-05-05:

To clarify, with this patch in place the filename will be properly escaped in the HTTP response header, like this:

Content-Disposition: attachment; filename="t\"est.txt"

Depending on your browser and operating system, the quote marks in the filename may not actually wind up as quote marks. For instance, running my test in Firefox on Ubuntu, the double quotes are changed into underscores. (Which is still an improvement over the filename being truncated or omitted as it was before this patch.)

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05: A change has been merged

Reviewed: https://reviews.mahara.org/6457
Committed: https://git.mahara.org/mahara/mahara/commit/aa8c67601ec68f2b831335b394ba6731d4cf4d55
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit aa8c67601ec68f2b831335b394ba6731d4cf4d55
Author: Aaron Wells <email address hidden>
Date: Thu May 5 18:47:52 2016 +1200

Escape double-quotes in filname, in Content-Disposition header

Bug 1578512: As specified in RFC 6266, the filename is a
"quoted-string", and as specified in RFC 2616 double quotes
within a quoted-string should be escaped with a backslash.

Change-Id: Id9d069a976406a82a6f0b6db92c696f700e00469
behatnotneeded: Can't test file uploads in behat yet

Revision history for this message

Robert Lyon (robertl-9) wrote on 2016-05-05:

And in Chromium for Ubuntu it changes the double quote to a hyphen

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05: A patch has been submitted for review

Patch for "16.04_STABLE" branch: https://reviews.mahara.org/6458

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05:

Patch for "15.10_STABLE" branch: https://reviews.mahara.org/6459

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05:

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/6460

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05: A change has been merged

Reviewed: https://reviews.mahara.org/6458
Committed: https://git.mahara.org/mahara/mahara/commit/d84ee5d63b65bc1a62ec0a4ab4ac5c34626b0e57
Submitter: Robert Lyon (<email address hidden>)
Branch: 16.04_STABLE

commit d84ee5d63b65bc1a62ec0a4ab4ac5c34626b0e57
Author: Aaron Wells <email address hidden>
Date: Thu May 5 18:47:52 2016 +1200

Escape double-quotes in filname, in Content-Disposition header

Bug 1578512: As specified in RFC 6266, the filename is a
"quoted-string", and as specified in RFC 2616 double quotes
within a quoted-string should be escaped with a backslash.

Change-Id: Id9d069a976406a82a6f0b6db92c696f700e00469
behatnotneeded: Can't test file uploads in behat yet
(cherry picked from commit aa8c67601ec68f2b831335b394ba6731d4cf4d55)

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05:

#10

Reviewed: https://reviews.mahara.org/6459
Committed: https://git.mahara.org/mahara/mahara/commit/b3b57485042ca5f0776244007b2d9c82d3cee764
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit b3b57485042ca5f0776244007b2d9c82d3cee764
Author: Aaron Wells <email address hidden>
Date: Thu May 5 18:47:52 2016 +1200

Escape double-quotes in filname, in Content-Disposition header

Bug 1578512: As specified in RFC 6266, the filename is a
"quoted-string", and as specified in RFC 2616 double quotes
within a quoted-string should be escaped with a backslash.

Change-Id: Id9d069a976406a82a6f0b6db92c696f700e00469
behatnotneeded: Can't test file uploads in behat yet
(cherry picked from commit aa8c67601ec68f2b831335b394ba6731d4cf4d55)

Revision history for this message

Mahara Bot (dev-mahara) wrote on 2016-05-05:

#11

Reviewed: https://reviews.mahara.org/6460
Committed: https://git.mahara.org/mahara/mahara/commit/f62f9619c19d5679244d350e69d3351d661b5199
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.04_STABLE

commit f62f9619c19d5679244d350e69d3351d661b5199
Author: Aaron Wells <email address hidden>
Date: Thu May 5 18:47:52 2016 +1200

Escape double-quotes in filname, in Content-Disposition header

Bug 1578512: As specified in RFC 6266, the filename is a
"quoted-string", and as specified in RFC 2616 double quotes
within a quoted-string should be escaped with a backslash.

Change-Id: Id9d069a976406a82a6f0b6db92c696f700e00469
behatnotneeded: Can't test file uploads in behat yet
(cherry picked from commit aa8c67601ec68f2b831335b394ba6731d4cf4d55)

Robert Lyon (robertl-9) on 2016-10-21

Changed in mahara:
milestone:	16.10.0 → none
status:	Fix Committed → Fix Released

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.