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

Inconsistent behavior with fs module accross platforms #17801

Open
ghost opened this issue Dec 21, 2017 · 3 comments · Fixed by #54160
Open

Inconsistent behavior with fs module accross platforms #17801

ghost opened this issue Dec 21, 2017 · 3 comments · Fixed by #54160
Labels
fs Issues and PRs related to the fs subsystem / file system. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. windows Issues and PRs related to the Windows platform.

Comments

@ghost
Copy link

ghost commented Dec 21, 2017

  • Version: v9.2.0
  • Platform: Windows 8.1, Windows 10, Ubuntu 17.10
  • Subsystem: fs

In Windows, if a file name ends with forward slash (/), fs interprets it as regular file (like there is no slash at all) and does not throw any errors. In Linux, it throws error

EISDIR: illegal operation on a directory, open

IMO, fs should either throw the same error in Windows, or don't throw the error in Linux.

Of course, when we talk about file name consistency, there will always be some characters which are allowed in Linux, but not allowed in Windows (like \u0001), but that inconsistency is a result of the way OS handles these situations. However, if a file name ends with a forward slash, it either means that someone mistakenly tried to open directory as a file, or just have forgotten to remove the slash. Therefore, fs should be consistent with the way it handles forward slash at the end of the file name. It means: either throw error always, or behave like there is no slash.

I made a script which shows how it currently works.

@bzoz
Copy link
Contributor

bzoz commented Dec 21, 2017

Thanks for reporting this issue!

So

require('fs').wrtieFileSync('test/', 'smth');
  • on Windows creates file test with smth inside,
  • on Linux it fails with Error: EISDIR: illegal operation on a directory, open 'test/'

Yep, this looks like a bug for me.

@nodejs/fs

@bzoz bzoz added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. labels Dec 21, 2017
@timotew
Copy link
Contributor

timotew commented Dec 31, 2017

@bzoz I don't know if am looking at the right place, But after looking at this line commet https://github.com/libuv/libuv/blob/2b32e77bb6f41e2786168ec0f32d1f0fcc78071b/src/win/fs.c#L519
at libuv windows implementation it seem the same code is used to access files and directories.
So I fixed it in the JS implementation, I have made a PR.

timotew added a commit to timotew/node that referenced this issue Jan 11, 2018
Throw Error if path ends with `/` only on windows linux is behaving as expected.
As suggested by issues nodejs#17801
@targos targos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jun 10, 2020
lundibundi added a commit to lundibundi/node that referenced this issue Aug 22, 2020
Previously upon calling file-only fs functions with a directory (or
presumably a directory of path ending with a forward slash '/') it would
result in EISDIR error on Linux but Windows would completely ignore trailing '/'
and perform operation on a file excluding it.

This introduces additional checks in fs code to make sure the trailing
slash is handled the same way across systems.

Fixes: nodejs#17801
huseyinacacak-janea added a commit to JaneaSystems/node that referenced this issue Aug 1, 2024
huseyinacacak-janea added a commit to JaneaSystems/node that referenced this issue Aug 1, 2024
nodejs-github-bot pushed a commit that referenced this issue Oct 10, 2024
Fixes: #17801
Refs: #33831
PR-URL: #54160
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs-github-bot pushed a commit that referenced this issue Nov 2, 2024
This reverts commit 00b2f07.

PR-URL: #55527
Fixes: #17801
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
Fixes: nodejs#17801
Refs: nodejs#33831
PR-URL: nodejs#54160
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
This reverts commit 00b2f07.

PR-URL: nodejs#55527
Fixes: nodejs#17801
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
@StefanStojanovic
Copy link
Contributor

Reopening because of #55527

aduh95 pushed a commit that referenced this issue Nov 5, 2024
This reverts commit 00b2f07.

PR-URL: #55527
Fixes: #17801
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
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. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. windows Issues and PRs related to the Windows platform.
Projects
None yet
4 participants