rails / sprockets

Rack-based asset packaging system
MIT License
932 stars 792 forks source link

Fix fingerprint fallback when filename collision #793

Open flivni opened 10 months ago

flivni commented 10 months ago

This is a fix for: https://github.com/rails/sprockets/issues/749

The bug is when you have two assets in the same path where one looks like the fingerprint-stripped version of the other, e.g. “landing.png” and “landing-splashimage.png”. In this case, if landing-splashimage.png is requested, a 404 is returned rather than the asset.

The old behavior: The code had logic to first strip anything that looked like a fingerprint from the filename. Then, if the asset was not found with the stripped path, it would attempt to find it with the full_path. This “fallback” was to handle the case where the code mistakenly thought a filename with a dash actually had a fingerprint. But, if the stripped path had already matched a different asset, then a 404 would be returned since the matched asset’s etag wouldn’t match the fingerprint.

The new behavior: This PR first looks up the asset using the original path. Only in the case where the asset is not found does the code strip what might be a fingerprint from the path and re-lookup.

This new behavior fixes the bug and I think is more true to the spirit of what this code should be doing.

Other changes / side effects: In the case where both the original asset and the fingerprinted asset exist, the new code returns the fingerprinted-asset while the old code returns the original asset. This should never result in any actual difference of bytes served.

I think this is worth fixing because if the bug is encountered, it can be deeply confusing to the uninitiated and could waste hours of time.