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

process.cwd() casing inconsistent across platforms #8237

Closed
Urthen opened this issue Aug 23, 2016 · 21 comments
Closed

process.cwd() casing inconsistent across platforms #8237

Urthen opened this issue Aug 23, 2016 · 21 comments
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding. process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.

Comments

@Urthen
Copy link

Urthen commented Aug 23, 2016

  • Version: Node 6.4
  • Platform: OSX, Windows 10 64bit
  • Subsystems: process, path

In various case-insensitive platforms - namely, Windows and OSX - it is possible to cd into an incorrectly-cased path. Node handles this differently on Windows than it does in OSX when attempting to resolve process.cwd(). This affects other modules - namely, how path.resolve() cases its absolute paths.

In OSX, process.cwd() resolves to the correctly cased path, regardless of which path you have cded into. I believe this to be correct behavior.

~/my-app$ node
> process.cwd()
'/Users/michael.pratt/my-app`
~/my-app$ cd ../My-app
~/My-app$ node
> process.cwd()
'/Users/michael.pratt/my-app`

In Windows default console, it is not possible to cd into an incorrect path. However, using PowerShell or the new Bash for Windows, it is possible, but process.cwd() resolves to the incorrect path casing - not matching what is on disk.

PS C:\Users\Michael Pratt\my-app> node             
> process.cwd()                                   
'C:\\Users\\Michael Pratt\\my-app'                 
PS C:\Users\Michael Pratt\my-app> cd ../My-app   
PS C:\Users\Michael Pratt\My-app> node             
> process.cwd()                                   
 'C:\\Users\\Michael Pratt\\My-app'  

Again, I feel the OSX behavior is correct, as it matches what is on the filesystem.

@Fishrock123 Fishrock123 added process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform. labels Aug 23, 2016
@silverwind silverwind added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Aug 23, 2016
@silverwind
Copy link
Contributor

silverwind commented Aug 23, 2016

Nice find, I agree the behaviour is wrong there. Likely a Windows API issue. The fact that Powershell displays the "wrong" path doesn't give much hope, though.

cc: @nodejs/platform-windows @saghul.

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

I believe the behavior is correct, as it returns the path returned by Windows. Regarding the "real path" versus "different case path" issue, I've expressed my opinion in #2443 (comment). To summarize, if you don't expect process.cwd() to return the real path if symlinks are involved, then you shouldn't expect it to return the "correct case" path either.

@silverwind
Copy link
Contributor

I think it's generally adviceable to do case-insensitive path comparision on case-insensitive file systems. Maybe we should include that in docs somewhere?

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

Unfortunately, that raises problems when operating cross-platform.

The context here is that on a case-insensitive system (at least in Webpack), it is possible to do import Foo from '../foo' and/or import Foo from '../Foo'. This results in errors on a case-sensitive system. I've developed a Webpack plugin to check for these incorrect cases. If you have cded into the incorrectly-cased path on Windows, it (correctly, IMO) detects that absolute paths returned by path.resolve() do not match what is on the filesystem.

Right now I'm planning on working around this by checking what process.cwd() returns and accepting that as a "valid" case with the assumption that the user has probably cded into the wrongly-cased directory, but I'd prefer a more robust solution in node for path.resolve() to return the correct path.

@saghul
Copy link
Member

saghul commented Aug 23, 2016

For process.cwd() we use uv_cwd, which is implemented as a simp,e wrapper over GetCurrentDirectoryW, whatever case it returns.

IIRC the problem on Windows is that while NTFS is case sensitive, the Windows API is not, so ¯_(ツ)_/¯

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

To quote a coworker, if Node is supposed to be cross-platform, it should compensate for platform inconsistencies. Much of the path module is there specifically to resolve these kind of platform inconsistencies, in fact.

I admit this is an edge case, and one that I can probably work around, and even that there is logic in reporting the CWD that the OS reports regardless of whether it is the correct case. In this case that this cannot be resolved, would you at least accept a documentation pull request warning about the cross-platform inconsistency on both process.cwd() and path.resolve()?

@saghul
Copy link
Member

saghul commented Aug 23, 2016

But which is the correct case if both are valid then? Have you tested with OSX with case-sensitive HFS? Last: in order to fix this Node would need to know what the correct case is (when on Windows), since NTFS is case sensitive. But how do we know?

@silverwind
Copy link
Contributor

@saghul brings up a good point. I'm reminded of #2165 which is of a very similar nature, but in the unicode normalization case, it's at least somewhat accepted that the NFC form is superior to NFD. Not something that could be said about letter case 😉.

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

But how do we know?

By using GetFinalPathNameByHandle. However, I don't think it should be process.cwd()'s job to "normalize" the path. If the user wants the physical path, they should call fs.realpath themselves.

...Or so it should be in an ideal world. Unfortunately, the JS implementation of fs.realpath doesn't call GetFinalPathNameByHandle, so the casing remains as-is. During the brief period when Node.js used native realpath, it was changed to the "physical" casing:

> fs.realpathSync('C:\\Users\\Nikolai\\desktop')
'C:\\Users\\Nikolai\\Desktop'
> process.version
'v6.3.1'

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

I can't concur that correct case isn't determinate. Even on a case-insensitive system, while many different case combinations might be valid as representations, only one is correct - that is, what the filesystem is actually storing it as. It's the difference between require('./package.json') and require('./PACKAGE.json') - while both work on a case-insensitive FS, they won't work on a case-sensitive one.

Also, as a side note, I just found out possibly even further evidence in Node itself that case-correctness really does matter. require('./PACKAGE.JSON') doesn't work, because Node is apparently checking for a case-sensitive .json extension.

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

As for the "how to determine" question, it's been resolved a couple different ways. The true-case-path lib uses glob to determine the correct path. My plugin uses fs.readdirSync recursively along the path and some intelligent caching to reduce filesystem operations. Neither I would suggest doing on every path.resolve call, but it might be able to be done at startup time to populate a correctly-cased process.cwd().

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

@Urthen
Again, there is no such thing as a "correct" path. There is a path that was actually cd'd into and which the Windows API returns. And there is the corresponding physical path that may or may not match the former. And they may differ in more ways than just casing in case of symlinks. Example:

C:\Users\Nikolai\Desktop>mkdir real
C:\Users\Nikolai\Desktop>mklink /D link real
symbolic link created for link <<===>> real
C:\Users\Nikolai\Desktop>cd link
C:\Users\Nikolai\Desktop\link>node
> process.cwd()
'C:\\Users\\Nikolai\\Desktop\\link'
> fs.realpathSync(process.cwd())
'C:\\Users\\Nikolai\\Desktop\\real'

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

While there may not be a unique correct path with respect to symlinks, there is absolutely such thing as a correctly-cased path to a given file (regardless of symlinks) on, as far as I am aware, any modern filesystem. The correct casing is the one reported by fs.readdir, for example. The key difference is whether the file system allows incorrectly-cased paths to resolve properly as well.

@seishun Can I ask why GetFinalPathNameByHandle use was removed from fs.realpath? Was it to do with symlinks? With regard to casing, that behavior looks like what I would expect across all platforms.

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

There is absolutely such thing as a correctly-cased path to a given file on, as far as I am aware, any modern filesystem.

Please refer to my example above. Which path would you consider correct in that case?

There is absolutely such thing as a correctly-cased path to a given file on, as far as I am aware, any modern filesystem.

Because that caused massive breakage, see #7726.

While there may not be a unique correct path with respect to symlinks, there is absolutely such thing as a correctly-cased path to a given file (regardless of symlinks) on, as far as I am aware, any modern filesystem.

I fail to see a fundamental difference. I can't think of a use case where you need to account for different casing but not for symlinks, or vice versa.

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

Sorry, my assertion was unclear. I've clarified.

In your example, I'd consider both 'C:\\Users\\Nikolai\\Desktop\\link' and 'C:\\Users\\Nikolai\\Desktop\\real' both valid and correctly cased.

However, if you then (following the example above, assuming you're in Powershell) did cd ../../desktop/link and performed the same Node commands, you would report 'C:\\Users\\Nikolai\\desktop\\link' and'C:\\Users\\Nikolai\\desktop\\real'. Both would be valid, but neither would be correctly cased. This is my concern.

Given the massive breakage, it seems like the difficulties attempting to actually fix this outweigh the relative ease implementing a workaround for quite possibly the only implementation where it is actually enforced.

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

However, if you then (following the example above, assuming you're in Powershell) did cd ../../desktop/link and performed the same Node commands, you would report 'C:\Users\Nikolai\desktop\link'

That's the path you cd'd into, no surprises here.

and'C:\Users\Nikolai\desktop\real'.

Now that's the issue with realpath I mentioned before, and a bug in my books. Feel free to submit an issue about it (although I'm not sure if there is a non-massively-breaking way to fix it).

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

That's the path you cd'd into, no surprises here.

If you read my original issue description, however, you'll note that OSX does not behave that way. process.cwd() will return the correctly-cased path if you cd'd into a differently-cased path. That's the issue: the inconsistency between how the function operates between OSX and Windows.

@seishun
Copy link
Contributor

seishun commented Aug 23, 2016

That seems to be a quirk of OSX to me. What does pwd say? And perhaps Python's os.getcwd().

@Urthen
Copy link
Author

Urthen commented Aug 23, 2016

It looks like Node and Python agree between process.cwd and os.getcwd, respectively. On OSX, pwd reports the incorrect case, while both Node and Python report the correct case in their cwd functions. On Windows, both Node and Python report the incorrect case.

Looks like confirmed as just quirky OS behavior, probably not something Node can reasonably fix. Closing the issue. Thanks for the support.

@Urthen Urthen closed this as completed Aug 23, 2016
@saghul
Copy link
Member

saghul commented Aug 24, 2016

By using GetFinalPathNameByHandle. However, I don't think it should be process.cwd()'s job to "normalize" the path. If the user wants the physical path, they should call fs.realpath themselves.

We cannot use GetFinalPathNameByHandle, that's what we used for fs.realpath and look at the shitstorm. Long story short: it doesn't work properly, so it's a no go.

@zerg41
Copy link

zerg41 commented Jul 29, 2023

Check this : vitest-dev/vitest#1870 (comment)
It was helpful for me)

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

No branches or pull requests

6 participants