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

Fix fingerprint fallback when filename collision #793

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Commits on Sep 14, 2023

  1. Fix fingerprint fallback when filename collision

    This is a fix for: rails#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.
    flivni committed Sep 14, 2023
    Configuration menu
    Copy the full SHA
    56493da View commit details
    Browse the repository at this point in the history