Skip to content

Don't follow Windows ContainerMappedDirectories #9475

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Don't follow Windows ContainerMappedDirectories #9475

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,9 @@ fs.realpathSync = function realpathSync(p, options) {
if (linkTarget === null) {
fs.statSync(base);
linkTarget = fs.readlinkSync(base);
if (linkTarget.startsWith('\\ContainerMappedDirectories')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit hacky. Wouldn't this prevent any application from using \ContainerMappedDirectories? I imagine an isWindows check should be added too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cjihrig is being too kind. This is not an acceptable solution.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the effort and had a look where things go wrong. I don't have expected that this PR gets merged in. It's just tried to get started understand what Node.js is doing here and start a discussion and giving a first idea to find a fix for Node.js in Windows containers.

The idea is to just stop the symlink expansion at the container volume shares.

I believe the work should be done by some Node.js core maintainer with more knowledge of the code base. I just forked the node repo for the first time, tried to compile the sources and find a fix and not just filing issues.

As a user of Node.js I just wonder why Node.js does so many symlink expansion for such simple tasks like node.exe c:\mount\hello.js and not finding the source code. I just wonder why this is all needed ( just thinking "code that does not exist could not have bugs" ). I'm sure there is a good reason why this is done in node.exe and npm at a lot of places, but it seems that it complicate things at some point.

continue;
}
}
resolvedLink = pathModule.resolve(previous, linkTarget);

Expand Down