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

.ensureDir(..) fails silently when passed an invalid path... #237

Closed
flynx opened this issue Apr 17, 2016 · 5 comments
Closed

.ensureDir(..) fails silently when passed an invalid path... #237

flynx opened this issue Apr 17, 2016 · 5 comments

Comments

@flynx
Copy link

flynx commented Apr 17, 2016

To reproduce:

fs.ensureDir('l:/tmp/foo:moo', err => console.log('>>>', err) )

Expected:

A error is generated and passed to the callback.

Result:

  • The callback is never called
  • CPU loaded -- looks like an infinite loop (did not investigate deeper)

NOTE: for now tested only on windows.

flynx added a commit to flynx/ImageGrid that referenced this issue Apr 17, 2016
@jprichardson
Copy link
Owner

Funny, I just started looking at this yesterday. Related: #209, #93.

@jprichardson
Copy link
Owner

jprichardson commented Apr 17, 2016

Suggestions on a clean fix? My initial plan would be to just test for invalid characters in the path...

@flynx
Copy link
Author

flynx commented Apr 18, 2016

@jprichardson I did not dive into the implementation, but in general I think it's obvious, pass an error to the callback in the same manner as native node .mkdir(..) does -- on first dir that fails... and infinite loops and recursion are definitely out of the questions ;)

fs.mkdir('l:/tmp/foo:moo', err => console.log('>>>', err) )

UPDATE:
I'd also keep the directories up to the error, it's more logical and less error-prone than trying to remove directories that some other thread might also be using already.

Another solution would be to syntax-check the path before creating anything, this way one would avoid leaks and managing stray directories, but here we'll have to implement/use OS-specific check, plus there are things that can't be checked easily before actually touching the file system, like access rights/ACLs, so the first approach still seems preferable (IMHO).

@jprichardson
Copy link
Owner

but in general I think it's obvious, pass an error to the callback in the same manner as native node .mkdir(..) does -- on first dir that fails... and infinite loops and recursion are definitely out of the questions ;)

This is oversimplifying the problem ;) I'll come up with something.

@flynx
Copy link
Author

flynx commented Apr 18, 2016

Appears to be working fine now, thanks!!

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

No branches or pull requests

2 participants