CODEWITHSUNDEEP
javascriptnode.jswhile-loopsave
alyx
alyx

Reputation: 2733

Node.js File System - Saving unique file names

I'm using Node File System to save uploaded images, using a while loop to check for existing file names, incrementing++ until there's a unique file name.

The code is not working, getting SyntaxError: Illegal break statement errors for the placement of my break; line, as well as the while loop never properly reading my fs.exists() function within the loop.

Am I doing something completely wrong here logically? Is there an easier way to ensure fs.writeFile() doesn't overwrite existing files?

Code:

  var fileExist = true;
  var fileName = req.files.files[0].name.substr(0, req.files.files[0].name.lastIndexOf('.')) || req.files.files[0].name;
  var fileType = req.files.files[0].name.split('.').pop();
  var fileNumber = 1;

  while (fileExist) {

    fileNumber_str = fileNumber.toString(); 

    var current = fileName + fileNumber_str + '.' + fileType;

    fs.exists( __dirname + "/uploads/" + current, function(exists){

        if (exists) {
            fileNumber++;
        }

        if (!exists) {

            var newPath = __dirname + "/uploads/" + current;
            fs.writeFile(newPath, data, function (err) {
                res.send('saved');
            });

            break;
        }

    });

  }

Upvotes: 1

Views: 3323

Answers (3)

velocityzen
velocityzen

Reputation: 86

Checking if a file exists before opening is an anti-pattern that leaves you vulnerable to race conditions: another process can remove the file between the calls to fs.exists() and fs.open()

You can use fsu module https://github.com/velocityzen/fsu

Upvotes: 1

adotout
adotout

Reputation: 1190

You are trying to break out of the callback of fs.exists, which is indeed illegal. It might be easier to use fs.existsSync

Ex:

while (fileExist) {

    fileNumber_str = fileNumber.toString(); 

    var current = fileName + fileNumber_str + '.' + fileType;


    if (fs.existsSync(__dirname + "/uploads/" + current)) {
        fileNumber++;
    } else {
        var newPath = __dirname + "/uploads/" + current;
        fs.writeFile(newPath, data, function (err) {
            res.send('saved');
        });

        break;
    }
}

Upvotes: 2

AndyD
AndyD

Reputation: 5385

Answer

Your code is using the async version of fs.exists. You need to use the sync version fs.existsSync for your loop to work.

Warning

Storing uploaded files using the name provided by the request is a bad idea as it allows a hacker to put in relative paths and potentially store files in places you would not want them to end up.

Equally it's a bad idea to allow those uploaded files to be downloaded using a path in the querystring. A hacker could write this: http://example.com/download?fileName=../../somethingnotexposed/

Upvotes: 2

Related Questions