fix: Enable getNode to follow directory links (fixes #248)
#318
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Follow-up question, why is the getFile (p, followLinks) {
followLinks = typeof followLinks === 'undefined' ? true : followLinksand not getFile (p, followLinks = true) {? |
AndreasFranek
changed the title
getNode to follow directory links (fixes #248)getNode to follow directory links (fixes #248)
|
Should probably land after #320 |
AndreasFranek
force-pushed
the
248-getNode-follow-links
branch
2 times, most recently
from
d114c2a to
0b22b1d
Compare
|
Rebased. |
|
Any further input on this? |
erikian
reviewed
Nov 1, 2024
test/api-spec.js
Outdated
| @@ -168,4 +168,8 @@ describe('api', function () { | |||
|
|
|||
| assert.deepStrictEqual(topLevelFunctions, defaultExportFunctions); | |||
| }); | |||
| it('should stat a symlinked file', async () => { | |||
| const stats = asar.statFile('test/input/stat-symlink.asar', 'real.txt', true); | |||
| return assert.strictEqual(stats.size, 19); | |||
There was a problem hiding this comment.
Can we assert that this is not a link instead of checking the size? I think it makes the intent clearer:
Suggested change
| return assert.strictEqual(stats.size, 19); | |
| return assert.strictEqual(stats.link, undefined); |
There was a problem hiding this comment.
Good point. Done.
This demonstrates issue electron#248
When called from stat, getNode should follow directory links to ensure that the correct node is returned. Otherwise, node.files[name] will be undefined, causing a TypeError.
AndreasFranek
force-pushed
the
248-getNode-follow-links
branch
from
0b22b1d to
a05f9b6
Compare
erikian
approved these changes
Nov 5, 2024
|
🎉 This PR is included in version 3.2.16 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #248
As reported in the issue, soft links (or symlinks/symbolic links) can cause a
TypeErrorto occur inasar.stat. This happens when a directory below the stat'ed path is actually a link.This commit adds a test demonstrating the problem and fixes the problem by introducing a
followLinksvariable ingetNodeand resolving directory links, if it is set.This is my first PR in this project so any feedback is welcome. I was not sure whether to implement the fix imperatively or recursively, for example. I.e., it is possible to follow the link like this instead: