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

win: fix fs.realpath.native for long paths #44536

Merged
merged 1 commit into from
Sep 13, 2022

Conversation

StefanStojanovic
Copy link
Contributor

On windows when using fs.realpath.native, fs.realpathSync.native and fs.promises.realpath on long path, ENOENT is thrown. On the other hand, long paths work well with fs.realpath and fs.realpathSync. On Linux, all of the mentioned realpath versions work as expected.

The problem is that windows API functions do not work with paths longer then 260 characters by default. There is a way to enable long path support via application manifest file, but it only works starting from Windows 10 version 1607 and it relies on registry key HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem\LongPathsEnabled being set. Since that is not necessarily always the case with machines running node, this approach wouldn't work. The other way to enable working with long paths is to add a \\?\ prefix to the path prior to using it in windows API functions. This approach is used by all fs functions working with paths, with the exception of fs.realpath.native and fs.realpathSync.native.

Fix is made in fs.js implementation of fs.realpath.native, fs.realpathSync.native, so their path preprocessing matches to the other functions.

An existing windows long path test is improved to cover the issue that is fixed by these changes.

Fixes: #39721

Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: nodejs#39721
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Sep 6, 2022
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2022
@nodejs-github-bot

This comment was marked as outdated.

@lpinca
Copy link
Member

lpinca commented Sep 6, 2022

Supersedes #44522.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/46468/

@StefanStojanovic
Copy link
Contributor Author

Hello everyone, is there anything else I can do to help move this forward?

@Flarna Flarna added commit-queue Add this label to land a pull request using GitHub Actions. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Sep 13, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 13, 2022
@nodejs-github-bot nodejs-github-bot merged commit 8b50160 into nodejs:main Sep 13, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 8b50160

Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: nodejs#39721
PR-URL: nodejs#44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@RafaelGSS RafaelGSS mentioned this pull request Sep 26, 2022
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@juanarbol juanarbol mentioned this pull request Oct 4, 2022
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
juanarbol pushed a commit that referenced this pull request Oct 7, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
juanarbol pushed a commit that referenced this pull request Oct 10, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: #39721
PR-URL: #44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: nodejs/node#39721
PR-URL: nodejs/node#44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
Unlike other fs.js functions that work with paths, realpath.native isn't
using pathModule.toNamespacedPath prior to calling libuv function. This
is causing issues on windows.

Windows long path test is also improved to cover the mentioned issue.

Fixes: nodejs/node#39721
PR-URL: nodejs/node#44536
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
4 participants