electron / asar

Simple extensive tar-like archive format with indexing
MIT License
2.55k stars 248 forks source link

fix: fix symlink pointing error in filesystem.insertLink #301

Closed jrainlau closed 7 months ago

jrainlau commented 8 months ago

In MacOS, if I run asar.createPackage() to pack a folder into .asar, which contains "symlink", it would get wrong after I run asar.extractAll() from the generated .asar.

Let's look at this Beyond Compare result:

企业微信截图_0ef755c7-3d85-4037-8d72-906e533b1fe1

The folder in left is the origin one, and the folder in right was generated by using asar.extractAll() from the origin one's .asar ( folderLeft --> folderLeft.asar --> folderRight).

Inside the left folder, file Resources were pointed to Versions/Current/Resources, but the same file inside the right folder were pointed to Versions/A/Resources, it's wrong.

I fixed this bug and added a test unit. I create a new folder in test/input/packthis-with-symlink, then create a /Current symlink folder from folder /A, then create a real.txt from folder /Current:

image

We could see that the new real.txt were pointing to Current/real.txt.

After preparing these materials, I made a new test unit:

it('should extract an archive with symlink', async () => {
    await asar.createPackageWithOptions('test/input/packthis-with-symlink/', 'tmp/packthis-with-symlink.asar', { dot: false })
    asar.extractAll('tmp/packthis-with-symlink.asar', 'tmp/packthis-with-symlink/')
    return compFiles('tmp/packthis-with-symlink/real.txt', 'test/input/packthis-with-symlink/real.txt')
  })

and updated the compFiles() function:

module.exports = async function (actualFilePath, expectedFilePath) {
  if (process.env.ELECTRON_ASAR_SPEC_UPDATE) {
    await fs.writeFile(expectedFilePath, await fs.readFile(actualFilePath))
  }
  const [actualFileContent, expectedFileContent] = await Promise.all([fs.readFile(actualFilePath, 'utf8'), fs.readFile(expectedFilePath, 'utf8')])
  assert.strictEqual(actualFileContent, expectedFileContent)

  const [actualIsSymlink, expectedIsSymlink] = [isSymbolicLinkSync(actualFilePath), isSymbolicLinkSync(expectedFilePath)]
  assert.strictEqual(actualIsSymlink, expectedIsSymlink)

  if (actualIsSymlink && expectedIsSymlink) {
    const [actualSymlinkPointer, expectedSymlinkPointer] = [fs.readlinkSync(actualFilePath), fs.readlinkSync(expectedFilePath)]
    assert.strictEqual(actualSymlinkPointer, expectedSymlinkPointer)
  }
}

function isSymbolicLinkSync (path) {
  const stats = fs.lstatSync(path)
  return stats.isSymbolicLink()
}

The new compFiles() function would detect whether the file is a symlink, and compare the real pointing by fs.readlinkSync(). With the bug in filesystem.js before I fixed, the test would throw an error:

image

After fixing this bug, everything runs well.

image
BlackHole1 commented 7 months ago

Thank you for your contribution.


I have two questions regarding the current modifications:

  1. In what scenarios do you need to put *.framework into app.asar? (In my opinion, the correct approach would be to put it in the app.asar.unpacked folder or the Resources folder.)
  2. Your screenshot confuses me. Why isn’t there a Current file in the left-hand file tree? Shouldn't the correct file tree be like this? 00-18 14 00@2x

If you are using electron-builder / electron-forge, you can use the extraResources configuration to place *.framework files in the Resources folder.

jrainlau commented 7 months ago

Happy Chinese new year to @BlackHole1! Let me answer the two questions you asked.

In our scenarios, we use .asar just for "archiving", such as "tar" or "zip", etc. However, when we try to archiving the same folder into .tar or .zip, the final product would got different MD5 from different OS, different archive time, and so on. We must keep the product's MD5 consistent across all archiving enviroment. After some test, .asar would be a good choice to us.

continuous-auth[bot] commented 7 months ago

:tada: This PR is included in version 3.2.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

viplifes commented 7 months ago

Hi, after this change I can't package my project

[FAILED] /var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker: file "../../../../../../../../../../../../var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js" links out of the package
[FAILED] /var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker: file "../../../../../../../../../../../../var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js" links out of the package
[FAILED] /var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker: file "../../../../../../../../../../../../var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js" links out of the package

An unhandled rejection has occurred inside Forge:
Error: /var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker: file "../../../../../../../../../../../../var/folders/nr/c9zr4xxd6sxfcj2rq3z7vnb80000gn/T/electron-packager/tmp-ehj9F8/Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js" links out of the package
at Filesystem.insertLink (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/asar/lib/filesystem.js:106:13)
    at handleFile (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/asar/lib/asar.js:132:20)
    at next (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/asar/lib/asar.js:148:11)
    at next (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/asar/lib/asar.js:149:12)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async MacApp.asarApp (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/packager/src/platform.ts:245:5)
    at async MacApp.buildApp (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/packager/src/platform.ts:150:5)
    at async MacApp.initialize (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/packager/src/platform.ts:141:7)
    at async MacApp.create (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/packager/src/mac.ts:435:5)
    at async Promise.all (index 0)
    at async packager (/Users/dimir/projects/simulator/simulator-launcher/app/node_modules/@electron/packager/src/packager.ts:246:20)
erickzhao commented 7 months ago

@viplifes thanks for the report, we'll take a look.

For the time being, you can downgrade the @electron/asar version in your project's lockfile to v3.2.8.

dsanders11 commented 7 months ago

@viplifes, I'm not able to repro this locally. Could you give more detailed information about the versions of packages you're using? Specifically versions of Electron Forge and @electron/packager.

viplifes commented 7 months ago

@dsanders11 Here is an example of a project that reproduces this problem https://github.com/viplifes/testapp_asar

I think there is a problem module: open -> is-inside-container -> is-docker

dsanders11 commented 7 months ago

Thank you @viplifes! That repro case is very helpful.

It appears the issue on macOS is one of the paths starting with /var/ and the other /private/var/ resulting in the confusion on relative paths. A potential fix is just using fs.realpathSync on parentPath so that they both end up with /private/var/. @jrainlau, any thoughts on this potential fix?