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

fs.mkdir/mkdirSync recursive hang with invalid windows character #31177

Closed
fraxken opened this issue Jan 3, 2020 · 8 comments
Closed

fs.mkdir/mkdirSync recursive hang with invalid windows character #31177

fraxken opened this issue Jan 3, 2020 · 8 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.

Comments

@fraxken
Copy link
Member

fraxken commented Jan 3, 2020

Hi,

The following issue: #28599 doesn't seem to be resolved. I just encountered the problem on a project i work on since few days where we generate reports (some reports name has the ':' symbol in the name which is not good... i guess my collaborators are not working on Windows 😅)

Version: Node.js 13.3.0
Platform: Windows 10 - 64 Bit

How to reproduce (work with mkdirSync and mkdir promises):

const { mkdirSync } = require("fs");

console.log("A");
mkdirSync("test:lol", { recursive: true });
console.log("B");

mkdirSync never return. This should throw an Error.

Best Regards,
Thomas

@richardlau richardlau added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. labels Jan 3, 2020
@Trott
Copy link
Member

Trott commented Jan 4, 2020

@cjihrig @bcoe

@cjihrig
Copy link
Contributor

cjihrig commented Jan 4, 2020

Were the @ mentions because of recursive rmdir() (not mkdir()), or something else?

Also cc: @bzoz (for working on #28599).

@Trott
Copy link
Member

Trott commented Jan 4, 2020

Were the @ mentions because of recursive rmdir() (not mkdir()), or something else?

@cjihrig In your case, I @-mentioned you because the linked issue (#28599) was closed by #29070, which you authored, so I thought you might have insight as to what's going on here. (I didn't look too closely at things. Sorry if the ping was a bit over-eager of me.)

@bcoe
Copy link
Contributor

bcoe commented Jan 5, 2020

@addaleax is there any reason we can't upstream this libuv patch into Node?

@addaleax
Copy link
Member

addaleax commented Jan 5, 2020

@bcoe I think that libuv patch is already included in the relevant Node.js versions, so it’s probably a different (?) issue? I can try to check the next time I’m on Windows.

@targos
Copy link
Member

targos commented Jan 5, 2020

I'm trying to debug with visual studio (first time :D). The value of err for the invalid path here is UV_ENOENT. It then reaches the condition dirname != next_path, which is true so it adds dirname and next_path to the queue.
Then dirname is handled (it exists).
Then next_path is handled, and we're back to the previous behavior -> infinite loop

@vtjnash
Copy link
Contributor

vtjnash commented Aug 18, 2020

Please review libuv/libuv#2601

santigimeno pushed a commit to santigimeno/libuv that referenced this issue Dec 28, 2020
Makes uv_fs_mkdir return UV_EINVAL for invalid directory names instead
of UV_ENOENT.

Refs: nodejs/node#31177
PR-URL: libuv#2601
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
santigimeno pushed a commit to libuv/libuv that referenced this issue Dec 28, 2020
Makes uv_fs_mkdir return UV_EINVAL for invalid directory names instead
of UV_ENOENT.

Refs: nodejs/node#31177
PR-URL: #2601
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@targos targos added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Dec 30, 2020
JeffroMF pushed a commit to JeffroMF/libuv that referenced this issue May 16, 2022
Makes uv_fs_mkdir return UV_EINVAL for invalid directory names instead
of UV_ENOENT.

Refs: nodejs/node#31177
PR-URL: libuv#2601
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@santigimeno
Copy link
Member

This was already fixed in libuv and released in libuv@1.41.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

9 participants