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.

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

Invalid dest causes runaway node process #139

Closed
roncli opened this issue Jun 3, 2015 · 7 comments
Closed

Invalid dest causes runaway node process #139

roncli opened this issue Jun 3, 2015 · 7 comments

Comments

@roncli
Copy link

roncli commented Jun 3, 2015

Given the following code:

var express = require("express"),
    multer = require("multer"),
    app = express();

app.use(multer({dest: "h:\\drive\\does\\not\\exist"}));

I would expect an exception to be thrown stating that the destination does not exist.

Instead, the node process goes to 100% CPU and starts a runaway memory leak that very quickly locks up the system if not killed.

@LinusU
Copy link
Member

LinusU commented Jun 15, 2015

Hmm, I guess that this is an windows only issue. I tried it on OS X and it created the directories necessary. Since UNIX dosen't have the concept of drives there isn't a way to replicate this.

I'll attach my test script below if it helps someone.

var express = require('express')
var multer = require('multer')

var app = express()
var upload = multer({ dest: '/tmp/why/u/no/exist' })

app.get('/', function (req, res) {
  res.setHeader('Content-Type', 'text/html')
  res.write('<form action="/" method="POST" enctype="multipart/form-data">')
  res.write('  <input type="file" />')
  res.write('  <input type="submit" />')
  res.write('</form>')
  res.end()
})

app.post('/', upload, function (req, res) {
  res.end('It worked!')
})

app.listen(3000)
console.log('http://localhost:3000')

@roncli Could you try the following script on Windows and tell me what it outputs?

var mkdirp = require('mkdirp')

mkdirp('h:\\why\\u\\no\\exist', function (err) {
  if (err) throw err;

  console.log('It worked!')
})

It will need mkdirp (npm install mkdirp), thanks!

@roncli
Copy link
Author

roncli commented Jun 15, 2015

Note, I have an H drive. I don't have a Q drive. H drive succeeded, Q drive started sucking down memory. Looks like the issue is with mkdirp, and conveniently they have an issue open about this already, substack/node-mkdirp#70 - Thanks for your time!

> var mkdirp = require("mkdirp");
undefined
> mkdirp("h:\\why\\u\\no\\exist", function(err) { if (err) throw err; console.log("It worked!"); });
undefined
> It worked!
> mkdirp("q:\\why\\u\\no\\exist", function(err) { if (err) throw err; console.log("It worked!"); });
undefined
>

@roncli roncli closed this as completed Jun 15, 2015
@LinusU
Copy link
Member

LinusU commented Jun 16, 2015

No problem, let me know if they fix it and I'll bump the dependency 👍

@roncli
Copy link
Author

roncli commented May 2, 2018

I'm going to reopen this, because the dependency node-mkdirp is no longer supported. Check out https://github.com/sindresorhus/make-dir for a replacement. :)

@jonchurch
Copy link
Member

jonchurch commented Jan 17, 2020

@roncli I pushed a PR with your suggested change, if you feel like testing it out I'd be much obliged 🙏

@jonchurch
Copy link
Member

Opened a PR w/ mkdirp to fix this on their 0.x branch isaacs/node-mkdirp#15

@jonchurch
Copy link
Member

This has been resolved with the publishing of mkdirp 0.5.4 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants