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.

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

Generated asset id is prone to collisions -- breaking imports #3823

Closed
ArsenyYankovsky opened this issue Nov 21, 2019 · 8 comments
Closed

Generated asset id is prone to collisions -- breaking imports #3823

ArsenyYankovsky opened this issue Nov 21, 2019 · 8 comments
Labels
🐛 Bug Stale Inactive issues

Comments

@ArsenyYankovsky
Copy link

ArsenyYankovsky commented Nov 21, 2019

🐛 bug report

The asset id generation method used in

? t.toIdentifier(md5(this.relativeName, 'base64')).slice(0, 4)
is prone to collisions. One example of relative paths that will generate same id is:

../../node_modules/lodash-es/clone.js
../../node_modules/core-js/library/modules/_an-object.js

both of these assets will generate asset id "ToFw". Having different assets with the same id could and does break some applications.

🎛 Configuration (.babelrc, package.json, cli command)

The build should be a production build or have scope hoisting enabled. This affects only Parcel 1.

🤔 Expected Behavior

Asset ids should not have collisions.

😯 Current Behavior

Asset ids collide leading to importing of wrong modules. I.e. importing the _an-object results in importing the lodash-es/clone.

💁 Possible Solution

Check if the hash already exists in the cache with the same relative name and use more characters from the full md5 if required.

@mischnic
Copy link
Member

This also affects both v1 and v2 of the parcel.

Parcel 2 uses the full hash, do you have an example where it still fails in this case?

@ArsenyYankovsky
Copy link
Author

ArsenyYankovsky commented Nov 21, 2019

The link above is for the file in the v2 branch. I assumed it is the branch used for the v2 development. Was I wrong in that assumption?

@mischnic
Copy link
Member

mischnic commented Nov 21, 2019

v2 is branched off of master and Parcel 1 is still in packages/core/parcel-bundler so that we can easily merge v2 back into master without rewriting the whole git history.
Parcel 2 is basically everything in the v2 branch except for packages/core/parcel-bundler 😄

@ArsenyYankovsky
Copy link
Author

Could you please point me to the updated Asset.js from the Parcel 2 then?

@mischnic
Copy link
Member

options.id != null
? options.id
: md5FromString(
idBase + options.type + getEnvironmentHash(options.env) + uniqueKey
),
hash: options.hash,

@ArsenyYankovsky
Copy link
Author

Ok, thank you. I updated the issue description.

@MatAlps
Copy link

MatAlps commented Jan 8, 2020

Hi @mischnic ,

I have had the exact same issue. Now fixed by changing a folder name in my project to avoid collision.

Could this be fixed in parcel 1?

Thx!

@github-actions
Copy link

github-actions bot commented Jul 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Stale Inactive issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants