Skip to content

Filename can be a nested path #355

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

Merged
sebdeckers merged 1 commit into from
Jun 11, 2017
Merged

Filename can be a nested path #355

Show file tree
Hide file tree
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ someModule.stream(function(err, stdout, stderr) {
var form = new FormData();

form.append('file', stdout, {
filename: 'unicycle.jpg',
filename: 'unicycle.jpg', // ... or:
filepath: 'photos/toys/unicycle.jpg',
contentType: 'image/jpg',
knownLength: 19806
});
Expand All @@ -144,6 +145,8 @@ someModule.stream(function(err, stdout, stderr) {
});
```

The `filepath` property overrides `filename` and may contain a relative path. This is typically used when uploading [multiple files from a directory](https://wicg.github.io/entries-api/#dom-htmlinputelement-webkitdirectory).

For edge cases, like POST request to URL with query string or to pass HTTP auth credentials, object can be passed to `form.submit()` as first parameter:

``` javascript
Expand Down
31 changes: 18 additions & 13 deletions lib/form_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,20 +209,25 @@ FormData.prototype._multiPartHeader = function(field, value, options) {

FormData.prototype._getContentDisposition = function(value, options) {

var contentDisposition;

// custom filename takes precedence
// fs- and request- streams have path property
// formidable and the browser add a name property.
var filename = options.filename || value.name || value.path;
var filename
, contentDisposition
;

// or try http response
if (!filename && value.readable && value.hasOwnProperty('httpVersion')) {
filename = value.client._httpMessage.path;
if (typeof options.filepath === 'string') {
// custom filepath for relative paths
filename = path.normalize(options.filepath).replace(/\\/g, '/');
} else if (options.filename || value.name || value.path) {
// custom filename take precedence
// formidable and the browser add a name property
// fs- and request- streams have path property
filename = path.basename(options.filename || value.name || value.path);
} else if (value.readable && value.hasOwnProperty('httpVersion')) {
// or try http response
filename = path.basename(value.client._httpMessage.path);
}

if (filename) {
contentDisposition = 'filename="' + path.basename(filename) + '"';
contentDisposition = 'filename="' + filename + '"';
}

return contentDisposition;
Expand All @@ -248,9 +253,9 @@ FormData.prototype._getContentType = function(value, options) {
contentType = value.headers['content-type'];
}

// or guess it from the filename
if (!contentType && options.filename) {
contentType = mime.lookup(options.filename);
// or guess it from the filepath or filename
if (!contentType && (options.filepath || options.filename)) {
contentType = mime.lookup(options.filepath || options.filename);
}

// fallback to the default content type if `value` is not simple value
Expand Down
12 changes: 10 additions & 2 deletions test/integration/test-custom-filename.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ var assert = common.assert;
var mime = require('mime-types');
var http = require('http');
var fs = require('fs');
var path = require('path');

var FormData = require(common.dir.lib + '/form_data');
var IncomingForm = require('formidable').IncomingForm;

var knownFile = common.dir.fixture + '/unicycle.jpg';
var unknownFile = common.dir.fixture + '/unknown_file_type';
var knownFile = path.join(common.dir.fixture, 'unicycle.jpg');
var unknownFile = path.join(common.dir.fixture, 'unknown_file_type');
var relativeFile = path.relative(path.join(knownFile, '..', '..'), knownFile);

var options = {
filename: 'test.png',
Expand All @@ -35,6 +37,10 @@ var server = http.createServer(function(req, res) {
assert.strictEqual(files['custom_filename'].name, options.filename, 'Expects custom filename');
assert.strictEqual(files['custom_filename'].type, mime.lookup(knownFile), 'Expects original content-type');

assert('custom_filepath' in files);
assert.strictEqual(files['custom_filepath'].name, relativeFile.replace(/\\/g, '/'), 'Expects custom filename');

@alexindigo alexindigo Jun 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I remember struggles like that, with no local access to windows :)

You could try something like:

assert.strictEqual(files['custom_filepath'].name, relativeFile.replace(/\\/g, '/'), 'Expects custom filename:' + files['custom_filepath'].name + ' -vs- ' + relativeFile.replace(/\\/g, '/') + ' -from- ' + relativeFile + ' -end-');

:)

@sebdeckers sebdeckers Jun 11, 2017

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hah this is too funny. 😂

Anyway I think it's because I did path.resolve after the backslash replace. Seems on Windows path.resolve expects to still see the native OS separator. Makes sense, I suppose.

Should be fixed now. Also cleaned up some path concatenation in text/common.js, just to be on the safe side.

@alexindigo alexindigo Jun 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. Native nodejs methods do convert slashes for windows, so it should be fine.

@alexindigo alexindigo Jun 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, do you mind to change 'Expects custom filename' to 'Expects custom filepath', it will be more clear which line is failed.

assert.strictEqual(files['custom_filepath'].type, mime.lookup(knownFile), 'Expects original content-type');

assert('unknown_with_filename' in files);
assert.strictEqual(files['unknown_with_filename'].name, options.filename, 'Expects custom filename');
assert.strictEqual(files['unknown_with_filename'].type, mime.lookup(options.filename), 'Expects filename-derived content-type');
Expand Down Expand Up @@ -67,6 +73,8 @@ server.listen(common.port, function() {
form.append('unknown_with_filename', fs.createReadStream(unknownFile), options.filename);
// Filename only with unknown file
form.append('unknown_with_filename_as_object', fs.createReadStream(unknownFile), {filename: options.filename});
// Filename with relative path
form.append('custom_filepath', fs.createReadStream(knownFile), {filepath: relativeFile});
// No options or implicit file type from extension on name property.
var customNameStream = fs.createReadStream(unknownFile);
customNameStream.name = options.filename;
Expand Down