lib: Fix infinite loop on paths with nested links#61391
lib: Fix infinite loop on paths with nested links#61391mattskel wants to merge 4 commits intonodejs:mainfrom
Conversation
Get the link target head realpath. Then resolve realpath and link target tail. Resolve tail after resolving the link head. Fixes: nodejs#60295
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61391 +/- ##
==========================================
+ Coverage 88.52% 88.54% +0.02%
==========================================
Files 704 704
Lines 208796 208819 +23
Branches 40312 40324 +12
==========================================
+ Hits 184828 184891 +63
+ Misses 15955 15908 -47
- Partials 8013 8020 +7
🚀 New features to boost your workflow:
|
Remove debugger.
|
Please add a test. |
test: cover realpathSync hang with symlink and ..
|
@targos please see test added. |
Remove sync test.
|
@nodejs/fs PTAL |
|
Can you explain how the fix works? |
Fix When a symlink target is read, only the first path component is resolved immediately. The remaining segments are stored as an By preventing .. from being applied before intermediate symlinks are resolved, the algorithm guarantees that symlinks are expanded before relative path segments are normalised, which prevents the path from resolving back to itself. |
| if (unresolvedTail !== '') { | ||
| p = pathModule.resolve(p + unresolvedTail); | ||
| unresolvedTail = ''; | ||
| } |
There was a problem hiding this comment.
flush the deferred tail as soon as it’s safe
| const nextPathDelimiterIndex = nextPart(linkTarget, 0); | ||
| if (nextPathDelimiterIndex >= 1) { | ||
| unresolvedTail = StringPrototypeSlice(linkTarget, nextPathDelimiterIndex) + unresolvedTail; | ||
| linkTarget = StringPrototypeSlice(linkTarget, 0, nextPathDelimiterIndex); | ||
| } |
There was a problem hiding this comment.
This code splits a symlink target into two parts: the first path segment, which is resolved immediately, and the remaining path segments, which are deferred.
| const result = nextPart(p, pos); | ||
| let unresolvedTail = ''; | ||
| while (true) { | ||
| if (pos >= p.length) { | ||
| if (unresolvedTail === '') { | ||
| break; | ||
| } | ||
|
|
||
| p = pathModule.resolve(p + unresolvedTail); | ||
| unresolvedTail = ''; | ||
| current = base = splitRoot(p); | ||
| pos = current.length; | ||
| continue; | ||
| } | ||
|
|
||
| const result = nextPart(p + unresolvedTail, pos); |
There was a problem hiding this comment.
Previously, the loop terminated once pos >= p.length. With the introduction of unresolvedTail, reaching the end of p does not necessarily mean resolution is complete. If the path has been fully scanned but deferred segments remain, the traversal state is reset and the unresolved tail is applied so resolution can continue.
Otherwise, path segmentation is performed on p + unresolvedTail, allowing deferred symlink target segments to be traversed as a continuation of the original path.
Fixes #60295