Skip to content

Reintroduce achievements link (and fix accidental hash collision in vue SFC compiler)

Lukas 'ai-pi' Eipert requested to merge leipert-reintroduce-achievements-link into master

What does this MR do and why?

Link to group achievements from achievement popup

Increase entropy in @vue/compiler cache key hash

Apparently we hit a very unlikely case that lead to our builds to be unstable:

  1. The @vue/compiler uses a small LRU cache with the latest 100 vue files it parsed.
  2. It uses a hash(filename + source + constant) for the key of the cache. The hashing function has 2^32 different outcomes

With our code base we have a pure chance of a hash collision of 1.3 per thousand. This collision happens with the two files at this commit: 850d6792

  • app/assets/javascripts/work_items/components/work_item_state_badge.vue
  • app/assets/javascripts/profile/components/user_achievements.vue

Now two more conditions needed to be fulfilled for it to become problematic:

  1. The two files needed to be parsed within a short period, otherwise the collision in the LRU cache would have been avoided. This explains why not every job was failing.
  2. In order for the jobs to fail, the colliding files needed to use relative imports. If work_item_state_badge.vue accidentally loaded the content of user_achievements.vue, the relative import ./graphql/get_user_achievements.query.graphql didn't exist. Vice versa if user_achievements.vue loaded the wrong contents, the relative import ../constants didn't exist.

If neither of the colliding files would had relative imports, the components might have been swapped silently, leading to potentially undetected runtime errors.

We mitigate this issue by patching the hashing of the key to be: hash(a) + hash(b) + hash(c) + hash(d) rather than hash(a+b+c). This decreases the likelyhood of collisions from 1.3 * 10^-3 to 2.3 * 10^-9, making it 570000 times less likely to hit a collision.

We probably should follow this up with an upstream contribution, so that other large vue projects are not hit by this.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

See: !151647 (merged)

How to set up and validate locally

See: !151647 (merged)

Edited by Lukas 'ai-pi' Eipert

Merge request reports