fs: fix infinite loop with async recursive mkdir on Windows

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 <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com>

Author: richardlau

src/node_file.cc

@@ -1289,8 +1289,15 @@ int MKDirpSync(uv_loop_t* loop, uv_fs_t* req, const std::string& path, int mode,
  }
  default:
  uv_fs_req_cleanup(req);
+ int orig_err = err;
  err = uv_fs_stat(loop, req, next_path.c_str(), nullptr);
- if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) return UV_EEXIST;
+ if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) {
+ uv_fs_req_cleanup(req);
+ if (orig_err == UV_EEXIST && continuation_data.paths.size() > 0) {
+ return UV_ENOTDIR;
+ }
+ return UV_EEXIST;
+ }
  if (err < 0) return err;
  break;
  }
@@ -1357,23 +1364,32 @@ int MKDirpAsync(uv_loop_t* loop,
  break;
  }
  default:
- if (err == UV_EEXIST &&
- req_wrap->continuation_data->paths.size() > 0) {
- uv_fs_req_cleanup(req);
- MKDirpAsync(loop, req, path.c_str(),
- req_wrap->continuation_data->mode, nullptr);
- } else {
+ uv_fs_req_cleanup(req);
+ // Stash err for use in the callback.
+ req->data = reinterpret_cast<void*>(static_cast<intptr_t>(err));
+ int err = uv_fs_stat(loop, req, path.c_str(),
+ uv_fs_callback_t{[](uv_fs_t* req) {
+ FSReqBase* req_wrap = FSReqBase::from_req(req);
+ int err = req->result;
+ if (reinterpret_cast<intptr_t>(req->data) == UV_EEXIST &&
+ req_wrap->continuation_data->paths.size() > 0) {
+ if (err == 0 && S_ISDIR(req->statbuf.st_mode)) {
+ Environment* env = req_wrap->env();
+ uv_loop_t* loop = env->event_loop();
+ std::string path = req->path;
+ uv_fs_req_cleanup(req);
+ MKDirpAsync(loop, req, path.c_str(),
+ req_wrap->continuation_data->mode, nullptr);
+ return;
+ }
+ err = UV_ENOTDIR;
+ }
  // verify that the path pointed to is actually a directory.
+ if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) err = UV_EEXIST;
  uv_fs_req_cleanup(req);
- int err = uv_fs_stat(loop, req, path.c_str(),
- uv_fs_callback_t{[](uv_fs_t* req) {
- FSReqBase* req_wrap = FSReqBase::from_req(req);
- int err = req->result;
- if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) err = UV_EEXIST;
- req_wrap->continuation_data->Done(err);
- }});
- if (err < 0) req_wrap->continuation_data->Done(err);
- }
+ req_wrap->continuation_data->Done(err);
+ }});
+ if (err < 0) req_wrap->continuation_data->Done(err);
  break;
  }
  break;

test/parallel/test-fs-mkdir.js

@@ -132,7 +132,25 @@ function nextdir() {
  }
 }
 
-// mkdirp when folder does not yet exist.
+// mkdirpSync when part of the path is a file.
+{
+ const filename = path.join(tmpdir.path, nextdir(), nextdir());
+ const pathname = path.join(filename, nextdir(), nextdir());
+
+ fs.mkdirSync(path.dirname(filename));
+ fs.writeFileSync(filename, '', 'utf8');
+
+ try {
+ fs.mkdirSync(pathname, { recursive: true });
+ throw new Error('unreachable');
+ } catch (err) {
+ assert.notStrictEqual(err.message, 'unreachable');
+ assert.strictEqual(err.code, 'ENOTDIR');
+ assert.strictEqual(err.syscall, 'mkdir');
+ }
+}
+
+// `mkdirp` when folder does not yet exist.
 {
  const pathname = path.join(tmpdir.path, nextdir(), nextdir());
 
@@ -149,11 +167,25 @@ function nextdir() {
 
  fs.mkdirSync(path.dirname(pathname));
  fs.writeFileSync(pathname, '', 'utf8');
- fs.mkdir(pathname, { recursive: true }, (err) => {
+ fs.mkdir(pathname, { recursive: true }, common.mustCall((err) => {
  assert.strictEqual(err.code, 'EEXIST');
  assert.strictEqual(err.syscall, 'mkdir');
  assert.strictEqual(fs.statSync(pathname).isDirectory(), false);
- });
+ }));
+}
+
+// `mkdirp` when part of the path is a file.
+{
+ const filename = path.join(tmpdir.path, nextdir(), nextdir());
+ const pathname = path.join(filename, nextdir(), nextdir());
+
+ fs.mkdirSync(path.dirname(filename));
+ fs.writeFileSync(filename, '', 'utf8');
+ fs.mkdir(pathname, { recursive: true }, common.mustCall((err) => {
+ assert.strictEqual(err.code, 'ENOTDIR');
+ assert.strictEqual(err.syscall, 'mkdir');
+ assert.strictEqual(fs.existsSync(pathname), false);
+ }));
 }
 
 // mkdirpSync dirname loop

test/parallel/test-fs-promises.js

@@ -310,6 +310,22 @@ async function getHandle(dest) {
  }
  }
 
+ // `mkdirp` when part of the path is a file.
+ {
+ const file = path.join(tmpDir, nextdir(), nextdir());
+ const dir = path.join(file, nextdir(), nextdir());
+ await mkdir(path.dirname(file));
+ await writeFile(file);
+ try {
+ await mkdir(dir, { recursive: true });
+ throw new Error('unreachable');
+ } catch (err) {
+ assert.notStrictEqual(err.message, 'unreachable');
+ assert.strictEqual(err.code, 'ENOTDIR');
+ assert.strictEqual(err.syscall, 'mkdir');
+ }
+ }
+
  // mkdirp ./
  {
  const dir = path.resolve(tmpDir, `${nextdir()}/./${nextdir()}`);