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

mkdir no ERROR when parent is a file with native recursive option #27198

Closed
thisconnect opened this issue Apr 12, 2019 · 7 comments · Fixed by #27207
Closed

mkdir no ERROR when parent is a file with native recursive option #27198

thisconnect opened this issue Apr 12, 2019 · 7 comments · Fixed by #27207
Assignees
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.

Comments

@thisconnect
Copy link

On windows Node.js v10.x hangs when trying to mkdir in a path where the parent is a file.
(sorry tested only on appveyor and travis)

Node8 on windows errors with EEXIST
Node10 on non-windows errors ENOTDIR
Node10 on windows hangs NO error 😿

  • Version: v10.15.3
  • Platform: Windows
  • Subsystem: fs
const fs = require('fs')
fs.writeFileSync('./test2.txt', '')
fs.mkdirSync('./test2.txt/sub/dir', {
    recursive: true
})

See hanging test on travis https://travis-ci.org/sindresorhus/make-dir
Related PR that adds test to make-dir sindresorhus/make-dir#15

@thisconnect thisconnect changed the title mkdir no ENOTDIR error with native recursive option when parent is a file mkdir no ERROR when parent is a file with native recursive option Apr 12, 2019
@thisconnect
Copy link
Author

I tested a bit more on appveyor and now I am not 100% sure if it might be a make-dir issue

@thisconnect
Copy link
Author

Update, I think it only happens with the callback, mkdirSync I believe is fine (the example above).
This should hang:

  const fs = require('fs');
  fs.writeFileSync('./test2.txt', '');
  fs.mkdir('./test2.txt/sub/dir/dir/dir', {
    recursive: true
  }, function() {
    console.log('never calls back')
  });

@targos
Copy link
Member

targos commented Apr 12, 2019

Confirmed on my Windows PC:

> fs.mkdirSync('package.json/a/b', { recursive: true })
Thrown:
{ Error: EEXIST: file already exists, mkdir 'package.json/a/b'
    at Object.mkdirSync (fs.js:773:3)
  errno: -4075,
  syscall: 'mkdir',
  code: 'EEXIST',
  path: 'package.json/a/b' }
> fs.mkdir('package.json/a/b', { recursive: true }, console.log)
undefined
> var p = fs.promises.mkdir('package.json/a/b', { recursive: true })
undefined
> p
Promise { <pending> }

@targos targos added confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. labels Apr 12, 2019
@targos
Copy link
Member

targos commented Apr 12, 2019

It's actually going into an infinite loop (the process uses 100% of my CPU)

@targos
Copy link
Member

targos commented Apr 12, 2019

/cc @nodejs/fs @bcoe

@bcoe bcoe self-assigned this Apr 12, 2019
@bcoe
Copy link
Contributor

bcoe commented Apr 12, 2019

@targos if no one else has a moment to dig into this, I can try to tackle it early next week.

@richardlau
Copy link
Member

@targos if no one else has a moment to dig into this, I can try to tackle it early next week.

PR: #27207

richardlau added a commit to richardlau/node-1 that referenced this issue Apr 16, 2019
If `file` is a file then on Windows `mkdir` on `file/a` returns an
`ENOENT` error while on POSIX the equivalent returns `ENOTDIR`. On the
POSIX systems `ENOTDIR` would break out of the loop but on Windows the
`ENOENT` would strip off the `a` and attempt to make `file` as a
directory. This would return `EEXIST` but the code wasn't detecting
that the existing path was a file and attempted to make `file/a` again.

PR-URL: nodejs#27207
Fixes: nodejs#27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
richardlau added a commit to richardlau/node-1 that referenced this issue Apr 16, 2019
Replace try-catch blocks in tests with `assert.rejects()` and
`assert.throws()`.

PR-URL: nodejs#27207
Fixes: nodejs#27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
MylesBorins pushed a commit that referenced this issue May 16, 2019
If `file` is a file then on Windows `mkdir` on `file/a` returns an
`ENOENT` error while on POSIX the equivalent returns `ENOTDIR`. On the
POSIX systems `ENOTDIR` would break out of the loop but on Windows the
`ENOENT` would strip off the `a` and attempt to make `file` as a
directory. This would return `EEXIST` but the code wasn't detecting
that the existing path was a file and attempted to make `file/a` again.

PR-URL: #27207
Fixes: #27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
MylesBorins pushed a commit that referenced this issue May 16, 2019
Replace try-catch blocks in tests with `assert.rejects()` and
`assert.throws()`.

PR-URL: #27207
Fixes: #27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
MylesBorins pushed a commit that referenced this issue May 16, 2019
If `file` is a file then on Windows `mkdir` on `file/a` returns an
`ENOENT` error while on POSIX the equivalent returns `ENOTDIR`. On the
POSIX systems `ENOTDIR` would break out of the loop but on Windows the
`ENOENT` would strip off the `a` and attempt to make `file` as a
directory. This would return `EEXIST` but the code wasn't detecting
that the existing path was a file and attempted to make `file/a` again.

PR-URL: #27207
Fixes: #27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
MylesBorins pushed a commit that referenced this issue May 16, 2019
Replace try-catch blocks in tests with `assert.rejects()` and
`assert.throws()`.

PR-URL: #27207
Fixes: #27198
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants