Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allows (escaped) double quotes and semicolons in the filename. #86

Closed
wants to merge 1 commit into from
Closed

Allows (escaped) double quotes and semicolons in the filename. #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/incoming_form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ IncomingForm.prototype._initMultipart = function(boundary) {
part.name = m[1];
}

if (m = headerValue.match(/filename="([^;]+)"/i)) {
if (m = headerValue.match(/filename="([^"\\]*(?:\\.[^"\\]*)*)"/i)) {
m[1] = m[1].replace(/\\"/ig, '"');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

The main thing is Line 281 which does nothing else than prevent formidable from crashing when a " occurs in the filename (and it allows semicolons too) because it literally just takes everyhing between the quotes of filename="......"

I handle the encoding for FF only. On Line 282 I turn " into " but I didn't knew that there are difference between browsers at this time.

So there are two options:

  1. Handle all browser specific differences inside of formidable (means detecting user agent etc.)
  2. Remove line 282 so formidable delivers the raw string between the quotes of filename="....."

I'd vote for second solution so the coder is responsible for decoding the filename properly. So my patch just allows escaped quotes in the filename and prevents cutting of the filename if a semicolon occurs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solution 2 sounds good. Want to modify the patch for that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

sorry I was wrong you need Line 282 because of Line 283:

283: part.filename = m[1].substr(m[1].lastIndexOf('\\') + 1);

this cuts off leading paths like c:\windows\file.pdf but if the filename is a \"simple\" file.pdf it would result in " file.pdf if you remove Line 282.

So this means you can apply the patch as it is if all tests pass.

part.filename = m[1].substr(m[1].lastIndexOf('\\') + 1);
}
} else if (headerField == 'content-type') {
Expand Down