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

Grid TIFF support is fundamentally broken and dangerous #3049

Closed
sihil opened this issue Nov 26, 2020 · 1 comment
Closed

Grid TIFF support is fundamentally broken and dangerous #3049

sihil opened this issue Nov 26, 2020 · 1 comment

Comments

@sihil
Copy link
Contributor

sihil commented Nov 26, 2020

Short description

When you upload a TIFF into the Grid it is silently converted to a PNG and the original file is NOT stored. The hash of the file stored does not match that of the image ID.

This means that it is currently impossible to reindex TIFF images in the Grid.

This was discovered whilst reviewing the work done in bbc#55

We are tracking this work internally on Trello

Steps to reproduce

  • Upload a TIFF to the grid

Actual results

  • Download the file: you will probably get a PNG file with a .jpg extension
  • Upload the file you downloaded: you will upload a new file rather than reupload a file with the same id
  • Note that the image ID is that of the original TIFF not the file you download

Expected results

  • Downloading the file will give you the original TIFF
  • Uploading the file you download will result in no actual upload taking place as you've uploaded the same image again

Analysis

This behaviour was inadvertently changed in https://github.com/guardian/grid/pull/2900/files#diff-3fa8fa8b16b2fa76a800844ca8df58a51ac954474da1b2612daa24f52d580b5bR261 by moving the place that the optimised file was generated in such a way that the optimised file is sometimes stored as the original instead of the original. The original is uploaded on line 249 of the above. It is overwritten on 257 by the optimised image (which is different in the case of it being a TIFF in our case).

Actions

  • Fix the underlying issue by not overwriting the original
  • Look at adding some tests to this functionality given that it is somewhat critical
  • Understand the impact by looking at how many TIFF images have been uploaded since this PR (Justinrowles/add more logging #2909) was merged (7th April 2020).
  • Carry out any remediation work
@paperboyo
Copy link
Contributor

paperboyo commented Dec 8, 2020

Fixed by #3050. Actions tracked by Trello ticket.

gribeiro added a commit to bbc/grid that referenced this issue Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants