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

Already on GitHub? Sign in to your account

Added disable rename files #158

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants

request for my issue
#154

set isAutoRename = false for disable rename uploading files

@felixge felixge commented on the diff Jul 29, 2012

lib/incoming_form.js
util.inherits(IncomingForm, EventEmitter);
exports.IncomingForm = IncomingForm;
IncomingForm.UPLOAD_DIR = (function() {
var dirs = [process.env.TMP, '/tmp', process.cwd()];
- for (var i = 0; i < dirs.length; i++) {
+ for (var i = 0, l = dirs.length; i < l; i++) {
var dir = dirs[i];
@Laboratory

Laboratory Jul 29, 2012

small optimization :)

@felixge felixge commented on the diff Jul 29, 2012

lib/incoming_form.js
this.bytesReceived = null;
this.bytesExpected = null;
this._parser = null;
this._flushing = 0;
this._fieldsSize = 0;
-};
+}
@felixge

felixge Jul 29, 2012

Owner

Semicolon thief!

@felixge felixge commented on the diff Jul 29, 2012

lib/incoming_form.js
@@ -10,12 +10,12 @@ var util = require('./util'),
EventEmitter = require('events').EventEmitter,
Stream = require('stream').Stream;
-function IncomingForm(opts) {
+function IncomingForm(opts) {
@felixge

felixge Jul 29, 2012

Owner

Thanks, trailing whitespace is evil : )!

Owner

felixge commented Jul 29, 2012

A few things:

  • Split unrelated changes (like whitespace) in separate commits
  • Explain your use case
  • Add a test demonstrating that this cannot be exploited (what happens if my file is named '../../my.jpeg'?)

Explain your use case:

When I upload files to the server, such as his name game.apk, after passing through IncomingForm name becomes 666a0d1b2d94b15f82b311193b694abf.apk. And when the user downloads the game on the phone, he always asks: What a strange file name?

if my file is named '../../my.jpeg' and param isAutoRename=true name will be 7a7a7..aa7.jpeg
if isAutoRename=false name will be my.jpeg

I added test in test/unit/test-incoming-form.js. His name is '#_uploadPath with disable rename files (isAutoRename)'

@Laboratory Laboratory commented on an outdated diff Jul 29, 2012

test/unit/test-incoming-form.js
@@ -56,6 +56,17 @@ test('IncomingForm', {
var ext = path.extname(form._uploadPath('super.bar'));
assert.equal(ext, '.bar');
},
+ '#_uploadPath with disable rename files (isAutoRename)': function() {
+ var fileName = "sample.txt";
@Laboratory

Laboratory Jul 29, 2012

for example: my.jpeg

@Laboratory Laboratory commented on an outdated diff Jul 29, 2012

test/unit/test-incoming-form.js
@@ -56,6 +56,17 @@ test('IncomingForm', {
var ext = path.extname(form._uploadPath('super.bar'));
assert.equal(ext, '.bar');
},
+ '#_uploadPath with disable rename files (isAutoRename)': function() {
+ var fileName = "sample.txt";
+
+ form.isAutoRename = true;
+ var _path = form._uploadPath(fileName);
+ assert.notEqual(_path, '/tmp/' + fileName);
@Laboratory

Laboratory Jul 29, 2012

_path='/tmp/alk8a9dfa9sdfa9sdfa9sdf.jpeg'

@Laboratory Laboratory commented on an outdated diff Jul 29, 2012

test/unit/test-incoming-form.js
@@ -56,6 +56,17 @@ test('IncomingForm', {
var ext = path.extname(form._uploadPath('super.bar'));
assert.equal(ext, '.bar');
},
+ '#_uploadPath with disable rename files (isAutoRename)': function() {
+ var fileName = "sample.txt";
+
+ form.isAutoRename = true;
+ var _path = form._uploadPath(fileName);
+ assert.notEqual(_path, '/tmp/' + fileName);
+
+ form.isAutoRename = false;
+ _path = form._uploadPath(fileName);
+ assert.equal(_path, '/tmp/' + fileName);
@Laboratory

Laboratory Jul 29, 2012

_path='/tmp/sample.text' or '/tmp/my.jpeg'

Owner

felixge commented Jul 29, 2012

Add a test demonstrating that this cannot be exploited (what happens if my file is named '../../my.jpeg'?)

^-- you have not done that, please add such a test.

added test

If the exec() fails here, you'll get an error.

Owner

Laboratory replied Jul 29, 2012

What is the condition for the error?

If the regex doesn't match the input, null will be returned. In this case, an exception will occur because null isn't an array that contains an index of 1.

Owner

Laboratory replied Jul 29, 2012

ok. But null will never be.
var filename = null;
filename && (/(.)$/.exec(filename)[1]); - return null, (filename && - false)
var filename = undefined;
filename && (/(.
)$/.exec(filename)[1]); - return undefined (filename && - false)
var filename = "";
filename && (/(.*)$/.exec(filename)[1]); - return "" again

I can add a condition to test, if u want.

(regex.exec(filename))

This is the part that can return null. If it does return null, then the subsequent null[1] will raise an exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment