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

ENOENT thrown when trying to create a file with an invalid filename #8987

Closed
Daniel15 opened this issue Oct 9, 2016 · 13 comments
Closed

ENOENT thrown when trying to create a file with an invalid filename #8987

Daniel15 opened this issue Oct 9, 2016 · 13 comments
Labels
blocked PRs that are blocked by other issues or PRs. 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

@Daniel15
Copy link

Daniel15 commented Oct 9, 2016

Node v6.7.0, Windows 10

To reproduce the issue:

fs.writeFileSync('C:\\temp\\foo:bar:baz', 'fail')

Expected output: Some error message about the filename being invalid because it contains colons

Actual output:

Error: ENOENT: no such file or directory, open 'C:\temp\foo:bar:baz'
    at Error (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.fs.writeFileSync (fs.js:1333:33)
    at repl:1:4
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)
    at REPLServer.defaultEval (repl.js:313:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Oct 9, 2016
@mscdex mscdex added the windows Issues and PRs related to the Windows platform. label Oct 9, 2016
@silverwind
Copy link
Contributor

Pretty sure that error codes comes straight from the Windows API, and it even seems consistent with macOS, so I'd lean towards leaving it as-is. Doing a check for filename validity on Windows is costly and should probably be done by the user.

@Daniel15
Copy link
Author

Daniel15 commented Oct 9, 2016

Pretty sure that error codes comes straight from the Windows API

Are you sure? I tried the equivalent in C#:

File.WriteAllText("c:\\temp\\foo:bar:baz", "winning");

And got a better error message:

System.NotSupportedException: The given path's format is not supported.

Or:

System.ArgumentException: Illegal characters in path.

I didn't try raw Win32 API calls though.

@addaleax
Copy link
Member

addaleax commented Oct 9, 2016

So, just in case this is helpful: Colons are used by NTFS to name separate Alternate Data Streams, so if there’s a file named foo, creating foo:bar and writing to it (as an ADS) should work.

So it would seem to me that ENOENT would be the correct code for writing to foo:bar when foo doesn’t exist.

I’m not sure the overhead of checking for multiple colons would be worth it…

@mscdex
Copy link
Contributor

mscdex commented Oct 9, 2016

I believe it might be technically possible to provide a better error message since CreateFileW() on Windows probably sets the 'last error' code to ERROR_BAD_PATHNAME in such cases. From what I can tell, the original Windows-specific error code is stored by libuv in such a way that node should have access to it.

@silverwind silverwind added libuv Issues and PRs related to the libuv dependency or the uv binding. and removed libuv Issues and PRs related to the libuv dependency or the uv binding. labels Oct 9, 2016
@Daniel15
Copy link
Author

Daniel15 commented Oct 9, 2016

So it would seem to me that ENOENT would be the correct code for writing to foo:bar when foo doesn’t exist.

Interesting, I didn't know that.

The same error message happens for other invalid characters though, such as question marks:

> fs.writeFileSync('C:\\temp\\foo??', 'fail')
Error: ENOENT: no such file or directory, open 'C:\temp\foo??'
    at Error (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.fs.writeFileSync (fs.js:1333:33)
    at repl:1:4

@seishun seishun assigned seishun and unassigned seishun Oct 12, 2016
@seishun
Copy link
Contributor

seishun commented Oct 13, 2016

The issue here is that libuv maps ERROR_INVALID_NAME to UV_ENOENT here. This particular mapping was added by @piscisaureus way back in 2012.

The question is what it should map to instead. Would UV_EINVAL be better? Understandably, libuv doesn't have a dedicated error code for an invalid file name since Linux doesn't have invalid filenames.

@piscisaureus
Copy link
Contributor

You could change it (major version), but before you do so, do some research into what different "libc" implementations do on windows - at least try what error the open() call in msvcrt produces when you try to create a file with an invalid name, and try cygwin too.

@silverwind
Copy link
Contributor

Linux doesn't have invalid filenames.

A / is a invalid character in a filename on all likely all Linux filesystems. The returned error code depends on the location of the / in the filename:

fs.writeFile("invalid/")
// Error: EISDIR: illegal operation on a directory, open 'invalid/'
> fs.writeFile("invalid/file")
// Error: ENOENT: no such file or directory, open 'invalid/file'

@silverwind silverwind added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Oct 13, 2016
@seishun
Copy link
Contributor

seishun commented Oct 13, 2016

@piscisaureus

Good idea. I assume you meant fopen, not open. fopen("foo??", "w") sets errno to EINVAL in both CRT and MSYS2.

@silverwind

In both of your examples, / is interpreted as a path separator. I don't think it's even possible to make Linux interpret it as part of the file name.

@paulbjensen
Copy link

Thanks for reporting this issue, I had no idea that : were invalid file name characters in Windows, been debugging an Electron app for a bit now.

santigimeno pushed a commit to libuv/libuv that referenced this issue May 31, 2017
E.g. when trying to create a file named "foo??".

Refs: nodejs/node#8987
PR-URL: #1088
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member

Trott commented Jul 15, 2017

Should this issue remain open?

@seishun
Copy link
Contributor

seishun commented Jul 15, 2017

It's going to remain an issue until libuv is upgraded to 2.x.

@Trott Trott added the blocked PRs that are blocked by other issues or PRs. label Jul 15, 2017
Trott added a commit to Trott/io.js that referenced this issue Jul 11, 2019
Add a known_issues test for the Windows returning ENOTFOUND where EINVAL
is more appropriate. This happens with various functions in the `fs`
module when an invalid path is used.

Refs: nodejs#8987

PR-URL: nodejs#28569
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this issue Jul 20, 2019
Add a known_issues test for the Windows returning ENOTFOUND where EINVAL
is more appropriate. This happens with various functions in the `fs`
module when an invalid path is used.

Refs: #8987

PR-URL: #28569
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@jasnell
Copy link
Member

jasnell commented Jun 25, 2020

There's been no activity on this and it's been blocked indefinitely pending the update to libuv 2.x. Keeping the issue open does not benefit anyone but we also don't want to forget about it. I've added it to a Futures project board so it doesn't get lost.

@jasnell jasnell closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. 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