Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

path: resolve() doesn't normalize drive letter on Windows #7799

Closed
wants to merge 1 commit into from

Conversation

mrfabbri
Copy link

Calling resolve() on Windows (in Node v0.11.0+) result in a path where drive
letter case isn't always lowercase (e.g. for a relative path like '.') while
normalize() always returns a path with a lower case drive letter.
This eventually breaks comparisons between results of resolve() and
join() (which relies on normalize()) for the same path.

Fixes #7031 also resolves #7806

Calling `resolve()` on Windows (in Node v0.11.0+) result in a path where drive
letter case isn't always lowercase (e.g. for a relative path like '.') while
`normalize()` always returns a path with a lower case drive letter.
This eventually breaks comparisons between results of `resolve()` and
`join()` (which relies on `normalize()`) for the same path.

Fixes nodejs#7031
@mrfabbri
Copy link
Author

PS: First node contribution. CLA signed.

@Fishrock123
Copy link

@mrfabbri just fyi, the CLA is no longer required: f6ba61b

@mrfabbri
Copy link
Author

@Fishrock123 thanks, missed that.

@mgol
Copy link

mgol commented Jun 25, 2014

I guess this may be the fix for #7806?

This lack of normalizing can completely break builds on Windows in hard to debug ways...

@mrfabbri
Copy link
Author

@mzgol yes, updated the pull request message accordingly.

@mgol
Copy link

mgol commented Jul 3, 2014

@mrfabbri Can you update commit message as well?

@jonschlinkert
Copy link

#7806 is a bug. this creates another bug. please explain the reasoning on this. what platform do you use @mrfabbri?

@mrfabbri
Copy link
Author

@jonschlinkert the problem is specific to Windows (as the changes in the PR, affecting a code path relative to Windows): on the documentation for path.resolve() is reported that "the resulting path is normalized" and starting from 0.11.0 (see #a05f973 ) path.normalize() behaviour on Windows is to lowercase drive letters while path.resolve() leaves the drive letter unchanged so a call to path.resolve() for the same path can result in a drive letter lower or upper case depending on how you invoked it.

The reasoning was to port the same behaviour of path.normalize() (relative to drive letter case) to path.resolve(). What bug does the PR introduce?

As a side note, I experienced the bug while testing a node-webkit (which uses node v0.11.x) app on Windows.

@jasnell
Copy link
Member

jasnell commented Aug 17, 2015

@orangemocha ... thoughts?

@orangemocha
Copy link
Contributor

I believe this was fixed in f6e5740. The behavior was later changed in 016e084, to avoid modifying the case of the drive letter.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants