fix: relative link error #308

ziqiangai · 2024-03-27T14:36:24Z

fix the fllowing error in electron-forge framework

> my-electron-vue-app@1.0.0 make
> electron-forge make

✔ Checking your system
⠋ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
  ✔ Preparing to package application
  ❯ Running packaging hooks
    ✔ Running generateAssets hook
✔ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
✔ Loading configuration
✔ Resolving make targets
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
✔ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
  ✔ Preparing to package application
  ✔ Running packaging hooks
    ✔ Running generateAssets hook
    ✔ Running prePackage hook
      ✔ [plugin-vite] Building vite bundles
  ❯ Packaging application
    ❯ Packaging for arm64 on darwin
      ✔ Copying files
      ✔ Preparing native dependencies [0.5s]
      ✖ Finalizing package
        › /tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@vue/compiler-core/node_modules/.bin/parser: file "../../../../../../../../tmp/…
  ◼ Running postPackage hook
◼ Running preMake hook
◼ Making distributables
◼ Running postMake hook

An unhandled rejection has occurred inside Forge:
Error: /tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@vue/compiler-core/node_modules/.bin/parser: file "../../../../../../../../tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@babel/parser/bin/babel-parser.js" links out of the package
at Filesystem.insertLink (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/filesystem.js:106:13)
    at handleFile (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:132:20)
    at next (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:148:11)
    at next (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:149:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async MacApp.asarApp (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:200:9)
    at async MacApp.buildApp (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:124:9)
    at async MacApp.initialize (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:117:13)
    at async MacApp.create (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/mac.js:321:9)
    at async Promise.all (index 0)
    at async packager (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/packager.js:237:22)

BlackHole1

This modification is related to #301. Can you try the solution that @dsanders11 commented on in #301?

lib/filesystem.js

BlackHole1 · 2024-03-28T04:02:50Z

This problem is quite complex, and I think I need to provide a detailed description here so that more people can understand the whole context:

In previous versions, asar would ignore the “intermediate” symbolic links. For example, I have three files: A, B, and C, with the relationship being: A -> B -> C. After being processed by asar, B would be discarded, resulting in: A -> C .In most scenarios, this is not a problem. However, if your code hardcodes the path to B, it will result in a "file not found" error.

About a month ago, @jrainlau discovered this issue and submitted a related fix: #301. In his fix, he used fs.readlinkSync + path.dirname to avoid the problem caused by fs.realpathSync (because fs.realpathSync will convert the input path to a path without any symbolic links). To better understand, I will describe the differences between the two in detail:

	Input	Output
`realpathSync`	/tmp/A	/tmp/C
`fs.readlinkSync` + `path.dirname`	/tmp/A	/tmp/B

At that time, we didn’t notice any issues until 3 weeks ago when @viplifes reported a problem. When processing with asar, it incorrectly assumed that the target file was outside the src directory.

At that time, @dsanders11 proposed a fix but didn’t receive a response from @jrainlau until today when @ziqiangai submitted an RP and verified that the modification proposed by @dsanders11 was not feasible. Next, I will explain why the fix code submitted by @jrainlau has this issue.

Before we start, there are two prerequisites to understand:

In macOS, the /var directory actually points to /private/var, as shown in the following figure:
Before operating with asar, src is copied to a subdirectory in the /var directory.

  // p = /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker
  insertLink (p) {
    // symlink = ../is-docker/cli.js
    const symlink = fs.readlinkSync(p)
    // /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin
    const parentPath = path.dirname(p)
    // fs.realpathSync(this.src) = /private/var/.../Electron.app/Contents/Resources/app
    // path.join(parentPath, symlink) = /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js
    // link = ../../../../../../../../../../../../var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js
    const link = path.relative(fs.realpathSync(this.src), path.join(parentPath, symlink))
  }

I think everything will become clear after adding comments to the above code. Due to fs.realpathSync(this.src), the path we obtain is /private/var. To solve the issue of missing symbolic links, we cannot use fs.realpathSync for the second parameter, which causes the two prefixes of the paths to be completely inconsistent, ultimately triggering:

asar/lib/filesystem.js

Lines 105 to 107 in d50b03f

    
           if (link.substr(0, 2) === '..') { 
        
             throw new Error(`${p}: file "${link}" links out of the package`) 
        
           }

~~As for the fix I mentioned in #308 (comment), it involves splitting: Only use fs.realpathSync for the condition check, and use fs.readlinkSync + path.dirname for other places.~~

BlackHole1

LGTM. It would be better if you could add a unit test for this!

ziqiangai · 2024-03-28T09:28:59Z

LGTM. It would be better if you could add a unit test for this!

It's a bit difficult for me.

Signed-off-by: Kevin Cui <bh@bugs.cc>

BlackHole1 · 2024-03-28T13:05:09Z

LGTM. It would be better if you could add a unit test for this!

It's a bit difficult for me.

NP 😃. I have pushed the relevant unit tests.

In previous implementations, the result obtained from `path.relative(realSrcPath, target)` is likely to be `../../../../../../../../../var/`. Although it has been tested and does not affect the program's execution, it looks very messy. To avoid this issue, modify `realSrcPath` to `this.src`. Signed-off-by: Kevin Cui <bh@bugs.cc>

ziqiangai · 2024-03-28T17:18:57Z

🫡 Congratulations, thank you for letting me learn a lot.

sethyuan · 2024-04-16T13:26:24Z

The fix can be improved (it's not complete), there are symlinks that resolve to an absolute path too (I ran into this with my Electron project), in which case, it should be ignored (they're usually executable commands like 'python', some dependencies generate these symlinks on building).

The fix I propose is this:

insertLink (p) {
  const symlink = fs.readlinkSync(p)
  // Can't really package these absolute links, they tend to be executable scripts like 'python'.
  if (path.isAbsolute(symlink)) return
  const parentPath = path.dirname(p)
  const link = path.relative(fs.realpathSync(this.src), fs.realpathSync(path.join(parentPath, symlink)))
  if (link.substr(0, 2) === '..') {
    throw new Error(`${p}: file "${link}" links out of the package`)
  }
  const node = this.searchNodeFromPath(p)
  node.link = link
  return link
}

BlackHole1 · 2024-04-16T14:17:25Z

@sethyuan Thank you for your additional information! I will conduct research in the next few days.

It would be even better if you can provide a reproducible repo, as this can save me a lot of time.😃

sethyuan · 2024-04-17T01:04:39Z

@BlackHole1 My environment is macOS 14.4.1 (arm64), I have miniconda installed and I'm using an environment created with it. The following steps can reproduce the packaging issue with symlinks:

npm init electron-app@latest my-app -- --template=vite-typescript
cd my-app
yarn add better-sqlite3
yarn package

Basically, it uses electron-forge to build and package the app and better-sqlite3 will build the sqlite3 lib from code leaving a symlink (absolute path) to the 'python' used, which in this case, is the one from miniconda's environment.

Here is a screen capture of the error:

BlackHole1 · 2024-04-17T01:29:53Z

@sethyuan Thanks. If I have enough time today, I will follow up on this issue today.

bstst · 2024-04-23T08:01:00Z

I'm also encountering a similar issue on OSX 14.2.1 (arm64). The vite-typescript template won't make and of course package.

A fix to make it work in that instance was to add this to forge.config.ts:

  hooks: {
    packageAfterPrune: async (_config, buildPath) => {
      await fs.rm(path.join(
        buildPath,
        'node_modules/scheduler/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
      await fs.rm(path.join(
        buildPath,
        'node_modules/react/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
      await fs.rm(path.join(
        buildPath,
        'node_modules/react-dom/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
   }
  },

I suppose that would also work for node_gyp symlinked bins. But that is not a nice solution.

But in my particular case one of my app dependencies (@istanbuljs) is using esprima, which at make time shoots an error like cannot copy ../../../../../esprima.js to a subdirectory of itself, ../../../../esprima.js, which is related to this electron/forge#3549.

They all seem to be linked to something upstream, something related to symlink resolution.

Signed-off-by: Kevin Cui <bh@bugs.cc>

BlackHole1 · 2024-04-28T08:40:16Z

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by #308 (comment).

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

bstst · 2024-04-28T12:27:51Z

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by #308 (comment).

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

The latest solution fixes the issue on my local system 🎉

I do have another somewhat related issue, but am not sure who's to blame:

My project has a dependency on the @smile-cdr/fhirts package. I get this error ("src:" and "dst:" are my debug logs, just as the "catch me" tracer):

src: /Users/martin/Projects/CENSORED/node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml
dst: /var/folders/bm/ytnswbdd57b5dm28zg7x8ysc0000gn/T/electron-packager/tmp-8AxS5d/Electron.app/Contents/Resources/app/node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml

Trace: catch me
    at /Users/martin/Projects/CENSORED/node_modules/fs-extra/lib/copy/copy.js:213:19
    at FSReqCallback.oncomplete (node:fs:200:23)

An unhandled rejection has occurred inside Forge:
Error: Cannot copy '../../../../../../esprima/bin/esvalidate.js' to a subdirectory of itself, '../../../../../../esprima/bin/esvalidate.js'.
at /Users/martin/Projects/CENSORED/node_modules/fs-extra/lib/copy/copy.js:214:21
    at FSReqCallback.oncomplete (node:fs:200:23)

That comes from them having a dependency on "nyc" in their "dependencies" section. If I move it to "devDependencies" it works fine. Which makes sense since it's all tests.

But the actual error comes, I believe, from the fact that they're all symlinks and it tries to copy a symlink into a symlink or something like that (I didn't spend much time on that). Is the "fs-extra" package here to blame (jprichardson/node-fs-extra#1019)?

sethyuan · 2024-04-29T00:47:57Z

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by #308 (comment).

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

Yes, it does work for me, thanks!

BlackHole1 · 2024-04-29T03:40:12Z

I do have another somewhat related issue, but am not sure who's to blame:

@bstst Can you try again to use the @electron/asar@3.2.8 version? If the problem still exists, then it is unrelated to #301 and #308, you should submit a new issue.

BTW, thank you @bstst @sethyuan for the feedback :)

bstst · 2024-04-29T05:46:42Z

I do have another somewhat related issue, but am not sure who's to blame:

@bstst Can you try again to use the @electron/asar@3.2.8 version? If the problem still exists, then it is unrelated to #301 and #308, you should submit a new issue.

BTW, thank you @bstst @sethyuan for the feedback :)

Does not seem like this package is related to the other issue that I'm getting ("fs-extra".copy is being called directly from the @electron-forge/vite-plugin, so it looks like the issue is indeed on the "fs-extra" side, will continue with them).

So it looks like this PR does indeed fix the "links out of the package" issue. Thanks for the effort everyone!

erickzhao

Tested this manually:

filesystem-spec.js fails if changes to lib/filesystem.js are reverted.
With a sample Forge app that fails yarn package on @electron/asar@3.2.9, this fixes the error and the packaged module works correctly.

erickzhao

minor nits

test/filesystem-spec.js

continuous-auth · 2024-05-02T22:11:08Z

🎉 This PR is included in version 3.2.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

fix: relative link error

b6930bf

ziqiangai requested a review from a team as a code owner March 27, 2024 14:36

ziqiangai added 4 commits March 27, 2024 22:59

feat: use resolve method

44a7cda

feat: use resolve method

18b6e3f

fix: lint error

a173528

fix: lint error

4738c3f

BlackHole1 requested changes Mar 28, 2024

View reviewed changes

lib/filesystem.js Outdated Show resolved Hide resolved

BlackHole1 requested changes Mar 28, 2024

View reviewed changes

lib/filesystem.js Outdated Show resolved Hide resolved

fix: new implement handle link

b05b255

ziqiangai requested a review from BlackHole1 March 28, 2024 07:21

BlackHole1 approved these changes Mar 28, 2024

View reviewed changes

test(filesystem): add insertLink test

d278f71

Signed-off-by: Kevin Cui <bh@bugs.cc>

BlackHole1 force-pushed the fix/relative-link-error branch 3 times, most recently from a13b90e to 5899d16 Compare March 28, 2024 11:20

fix test

880d020

Signed-off-by: Kevin Cui <bh@bugs.cc>

BlackHole1 force-pushed the fix/relative-link-error branch from a340f59 to 880d020 Compare March 28, 2024 12:58

erickzhao mentioned this pull request Apr 18, 2024

Error during packaging on macOS electron/forge#3549

Closed

3 tasks

re-write logic

eaf970c

Signed-off-by: Kevin Cui <bh@bugs.cc>

BlackHole1 force-pushed the fix/relative-link-error branch from 3a584ae to eaf970c Compare April 28, 2024 08:26

BlackHole1 requested review from dsanders11 and erickzhao April 29, 2024 03:40

erickzhao approved these changes May 1, 2024

View reviewed changes

erickzhao reviewed May 1, 2024

View reviewed changes

test/filesystem-spec.js Outdated Show resolved Hide resolved

test/filesystem-spec.js Outdated Show resolved Hide resolved

erickzhao added 2 commits May 2, 2024 14:27

Update test/filesystem-spec.js

c23fb27

Update test/filesystem-spec.js

e901f4f

georgexu99 approved these changes May 2, 2024

View reviewed changes

erickzhao merged commit d8111fc into electron:main May 2, 2024
3 checks passed

continuous-auth bot added the released label May 2, 2024

BlackHole1 mentioned this pull request May 6, 2024

chore: bump @electron/asar from 3.2.9 to 3.2.10 electron/packager#1716

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: relative link error #308

fix: relative link error #308

ziqiangai commented Mar 27, 2024

BlackHole1 left a comment

BlackHole1 commented Mar 28, 2024 •

edited

BlackHole1 left a comment

ziqiangai commented Mar 28, 2024

BlackHole1 commented Mar 28, 2024

ziqiangai commented Mar 28, 2024

sethyuan commented Apr 16, 2024

BlackHole1 commented Apr 16, 2024

sethyuan commented Apr 17, 2024

BlackHole1 commented Apr 17, 2024

bstst commented Apr 23, 2024 •

edited

BlackHole1 commented Apr 28, 2024

bstst commented Apr 28, 2024

sethyuan commented Apr 29, 2024

BlackHole1 commented Apr 29, 2024

bstst commented Apr 29, 2024

erickzhao left a comment •

edited

erickzhao left a comment

continuous-auth bot commented May 2, 2024

fix: relative link error #308

fix: relative link error #308

Conversation

ziqiangai commented Mar 27, 2024

BlackHole1 left a comment

Choose a reason for hiding this comment

BlackHole1 commented Mar 28, 2024 • edited

BlackHole1 left a comment

Choose a reason for hiding this comment

ziqiangai commented Mar 28, 2024

BlackHole1 commented Mar 28, 2024

ziqiangai commented Mar 28, 2024

sethyuan commented Apr 16, 2024

BlackHole1 commented Apr 16, 2024

sethyuan commented Apr 17, 2024

BlackHole1 commented Apr 17, 2024

bstst commented Apr 23, 2024 • edited

BlackHole1 commented Apr 28, 2024

bstst commented Apr 28, 2024

sethyuan commented Apr 29, 2024

BlackHole1 commented Apr 29, 2024

bstst commented Apr 29, 2024

erickzhao left a comment • edited

Choose a reason for hiding this comment

erickzhao left a comment

Choose a reason for hiding this comment

continuous-auth bot commented May 2, 2024

BlackHole1 commented Mar 28, 2024 •

edited

bstst commented Apr 23, 2024 •

edited

erickzhao left a comment •

edited