Repository navigation
path.posix.resolve, path.posix.relative return wrong path on Windows #13887
Description
Activity
Hope I can find a good way to fix it. 😂
I had an idea, add a new "strict mode" to
path:const path = require('path'); path.posix.resolve('a/b/c'); // returns: 'C:\\Users\\fei\\Desktop\\share\\node/a/b/c'; path.strict = true; path.posix.resolve('a/b/c'); // throws
Reacted by Tobias NießenCan you clarify why it's wrong?
Can you clarify why it's wrong?
path.posix.resolve('a/b/c'); // returns: 'C:\\Users\\fei\\Desktop\\share\\node/a/b/c';
The result is not a valid path nor on windows (the host) and definitely not on POSIX (the expected target).
The root cause is thatresolveusesprocess.cwd()which is the only point where thepathmodule does something that is not pure string manipulation.See discussion at: #13683 (comment)
Reacted by Mark StosbergI am against introducing a strict mode. Instead, I would prefer to always throw if
process.cwd()needs to be used and the host platform is not the same as the API platform. If we care about breakage too much, we can introduce a deprecation cycle such thatpath.posix.resolveandpath.win32.resolvewill print warnings instead of throwing.I could put together a PR if this gets approval.
I am against introducing a strict mode. Instead, I would prefer to always throw if process.cwd() needs to be used and the host platform is not the same as the API platform. If we care about breakage too much, we can introduce a deprecation cycle such that path.posix.resolve and path.win32.resolve will print warnings instead of throwing.
@tniessen I had the same opinion, but @mscdex and @addaleax are strongly against. The point they both raised is that this is not strictly a bug but a weird behavior that people are used to, and throwing will block "valid" code paths. With that in mind,
strictmode would allow us in the future change the default behavior while allowing opting-out by settingpath.strict = false;Introducing a global setting for such a "strict mode" sounds like an unnecessary side effect to me (unless you want to make that setting per module or similar), e.g. if an application enables strict mode and a dependency expects non-strict mode or vice versa. If you really want to implement different modes, I would suggest
require('path').strict().I would like to include @addaleax in this discussion so we don't have to split it and continue parts of it in #13683.
Throwing an error instead of fixing the bug seems like a really bad idea; I think there is no reason inherent to the API why
path.posix.relative('a/b/c', '../../x')wouldn’t be able to return a correct result on Windows.
(#13683 (comment))I understand that; but that should work just fine on Windows, no?
(#13683 (comment))Let's look at something simple:
path.posix.resolve('foo/bar')
So what would be the "correct result" on Windows? Let's say
process.cwd()isF:\node. Would you expectF:\node/foo/bar? Not a good idea, it is not a valid posix path. How aboutF:/node/foo/bar? Looks good, butresolvemust return an absolute path, andpath.posix.isAbsolute('F:/node/foo/bar')isfalse, and there is no way to change that, as this is a relative path on posix systems. How about/F:/node/foo/bar? Too bad, now it is not even a valid path on Windows anymore.how about starting a vote?
I also really don’t like the idea of strict mode; all
pathfunctions currently are pure, up to their dependency oncwd(), and that’s a good thing. If you want it, add an extra parameter or add new methods, don’t introduce global mutable state.Re: always throwing when
process.cwd()is used… people are doing weird stuff withpath, and the thing is, the end result isn’t always dependent on all of the input. For example, one could usepath.resolve()on two different paths, where the cwd may or may not be used, and then callpath.relative()and get a result that doesn’t actually depend on the cwd or only parts of it, even though the intermediate results do.It’s a nice thing that explicit
path.posix.resolve()is likely used much less frequently than justpath.resolve(); but I am afraid users might just use that e.g. if they need URL-like paths for some reason, or basically just “want to get forward slashes” for any possible reason, rather than using the Windows method and then fixing up the result to fit their format.How about
F:/node/foo/bar? Looks good, butresolvemust return an absolute path, andpath.posix.isAbsolute('F:/node/foo/bar')isfalseI would accept that; it’s strictly better than the current state of things, and even though
path.posix.isAbsolute('F:/node/foo/bar')returnsfalse, it is still an absolute path for all other purposes.how about starting a vote?
Our process is consensus-seeking, so usually things don’t work that way; we’ll keep discussing the issue (and maybe coming up with new solutions) until we either reach consensus or it becomes clear that we won’t reach consensus. I don’t think we’re at either point yet.
If we won’t be able to get consensus, we’ll likely put this on the CTC meeting agenda, where it will be discussed, and we’ll only vote if the CTC won’t get consensus either. I think it’s unlikely that it will come to that.
don’t introduce global mutable state.
Definitely +1!
and the thing is, the end result isn’t always dependent on all of the input
Good point, we might need to come up with a better solution.
I would accept that; it’s strictly better than the current state of things, and even though
path.posix.isAbsolute('F:/node/foo/bar')returnsfalse, it is still an absolute path for all other purposes.Then let's have a look at
path.posix.resolve(process.cwd(), 'foo'). Everything is fine when executed under linux, but it will return something like'F:\\node/F:\\node/foo'on Windows, and there is no way to change that, because the posix API considers the result ofprocess.cwd()relative.
Sure, you could say that it does not make sense to do this, using a Windows API call with the posix API... But that's exactly what we are doing in the currentpathmodule.If we decide not to throw an
Errorin these situations, we should at least add notes to the docs explaining the current behavior (after fixing bugs as in #13683).don’t introduce global mutable state.
Definitely +1!
Offf I forgot about the globalness of modules... How about something like...
const path = require('path'); path.strict.posix.resolve('.\\gaga') // throws const { strict: spath } = path; spath.posix.resolve('.\\gaga') // throws
Again it's opt-in, and gives us a path to deprecation with explicit and opt-out in the future.
Another idea: an implementation that's pure strings, with an optional
cwdas argconst { Pure } = require('path'); const purePath1 = new Pure('C:\\fakepath') purePath1.resolve('gaga') === 'C:\\fakepath\\gaga'; const purePath2 = new Pure(); purePath1.resolve('gaga') === Pure.resolve('gaga') // throws or returns just 'gaga'
[refack: tweaked code]
I also prefer the pure function, but if this is done, does it mean that the history code needs to be modified？
@DuanPengfei the idea in not to change the behaviour of the old code, but add a new way to use it:
const path = require('path'); path.Pure.resolve('gaga') === path.Pure.posix.resolve('gaga') === path.Pure.windows.resolve('gaga') // either all throw or all returns just 'gaga' // old code stays the same path.posix.resolve('gaga') === 'C:\\bin\\dev\\node/gaga';
@refack I see
I will create a new PR do the same thing as the PR #13738. And then we discuss whether it is reasonable?
With just the improvements? Sounds good.
path.posix.isAbsolute('//?/X:/yyy.txt')===true😘https://docs.microsoft.com/windows/desktop/FileIO/naming-a-file#win32-file-namespaces
I keep coming across this type of issue on github and the wild. The assumption that on Windows, the path separation and parsing desired is not POSIX is a bad assumption. People that develop in the wild use things like bash for windows, mingw, msys, etc. The API constraining you based on the OS used makes
pathamong the least portable modules in NodeJS for developing cross platform scripts.Reacted by Justin Dalrymple- added a commit that references this issue
on May 2, 2023 - added a commit that references this issue
on May 3, 2023
path.posix.resolve,path.posix.relativereturn wrong path on Windows.For example:
Ref: #13683