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.

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

Jump to bottom

[BUG] npm arborist realpath.js with UNC paths #7309

Open
2 tasks done
danFbach opened this issue Mar 21, 2024 · 2 comments · May be fixed by #7757
Open
2 tasks done

[BUG] npm arborist realpath.js with UNC paths #7309

danFbach opened this issue Mar 21, 2024 · 2 comments · May be fixed by #7757
Labels
Bug thing that needs fixing Needs Triage needs review for next steps

Comments

@danFbach
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Try to install something on global npm install -g [package] an error is thrown:

37 verbose stack RangeError: Maximum call stack size exceeded
37 verbose stack     at isPathSeparator (node:path:52:25)
37 verbose stack     at resolve (node:path:217:13)
37 verbose stack     at realpathCached (C:\Program Files\nodejs\node_modules\npm\node_modules\@npmcli\arborist\lib\realpath.js:18:10)
37 verbose stack     at realpathCached (C:\Program Files\nodejs\node_modules\npm\node_modules\@npmcli\arborist\lib\realpath.js:38:10)

I added some logging to realpath.js to see what it was doing...

console.log(depth, dir, path, dir == path, base);
undefined \\SOURCE2\Users\danF\AppData\Roaming \\SOURCE2\Users\danF\AppData\Roaming\npm false npm
NaN \\SOURCE2\Users\danF\AppData \\SOURCE2\Users\danF\AppData\Roaming false Roaming
NaN \\SOURCE2\Users\danF \\SOURCE2\Users\danF\AppData false AppData
NaN \\SOURCE2\Users\ \\SOURCE2\Users\danF false danF
NaN \\SOURCE2\Users\ \\SOURCE2\Users\ true Users
NaN \\SOURCE2\Users\ \\SOURCE2\Users\ true Users
NaN \\SOURCE2\Users\ \\SOURCE2\Users\ true Users
NaN \\SOURCE2\Users\ \\SOURCE2\Users\ true Users
NaN \\SOURCE2\Users\ \\SOURCE2\Users\ true Users
...repeats until stack overflow or whatever

Expected Behavior

Expected bahavior...it works? i guess.
Given this the repeating directory error, I changed line 33 from:

  if (!base) {
    rpcache.set(dir, dir)
    return Promise.resolve(dir)
  }

to

  if (!base || dir == path) {
    rpcache.set(dir, dir)
    return Promise.resolve(dir)
  }

so that it recognizes that it has ascended as far as it can and reached the effective 'base'.

I'm no node.js/npm master, so i don't claim that this is a good fix but, i can install things globally now.

Steps To Reproduce

Windows 11. local Domain joined to Windows server, using folder redirection.

I'm not sure what other information to provide here. In this environment, all you have to do is try to install something globally and it fails.

Environment

  • npm: 10.5.0
  • Node.js: 20.11.1
  • OS Name: Windows 11
  • System Model Name: Dell generic desktop
  • npm config:
; "builtin" config from \\SOURCE2\Users\danF\AppData\Roaming\npm\node_modules\npm\npmrc

prefix = "\\\\SOURCE2\\Users\\danF\\AppData\\Roaming\\npm"

; "user" config from C:\Users\danF.LEE\.npmrc

loglevel = "verbose"

; node bin location = C:\Program Files\nodejs\node.exe
; node version = v20.11.1
; npm local prefix = C:\Users\danF.LEE
; npm version = 10.5.0
; cwd = C:\Users\danF.LEE
; HOME = C:\Users\danF.LEE
@danFbach danFbach added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Mar 21, 2024
@mizzi-sesh
Copy link

Encountered the same error, did the exact same change and NPM is up and running. Thanks dan.

@BenMcLean
Copy link

BenMcLean commented Aug 28, 2024 •
edited
Loading

I have also encountered this same issue on Windows 11, node.js version v22.7.0, npm version 10.8.2, default config so I believe it has still persisted across versions. The same modification as the OP appears to have fixed the issue. Isn't it about time we made this a PR?

BenMcLean added a commit to BenMcLean/cli that referenced this issue Aug 28, 2024
@BenMcLean
Update realpath.js to fix npm#7309
e23dc8b
@BenMcLean BenMcLean linked a pull request Aug 28, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug thing that needs fixing Needs Triage needs review for next steps
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: prevent infinite recursion when looking up real paths BenMcLean/cli
4 participants
@lukekarrys @BenMcLean @danFbach @mizzi-sesh