File uploads on local storage with nil secret in the DB are broken
Note - the title was updated to better reflect that old file uploads are broken in a number of places (such as images showing as 404 even on non-Geo setups), see #215991 (closed) as related.
Previous title for reference: Geo fails to sync file uploads with nil secret in the DB
Summary
As a follow-up from #209940 (closed) - it looks like this didn't fully fix the syncing issue and might affect more now.
Follow-up investigation leads to Uploads
that don't have a secret
on the model, not parsing the secret from the path but instead generating a random one each time.
To better explain this, here's a sample snippet from local tests on a 12.10.3 setup.
# an example upload with a valid secret, length 10
irb(main):091:0> u = Upload.find(2); u.path
=> "06670adf08/test2.txt"
irb(main):092:0> u.secret
=> nil
# getting random secret and paths
irb(main):095:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
941f19bcd827bd4f82a52f7d7f220706
941f19bcd827bd4f82a52f7d7f220706
=> nil
irb(main):096:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
122a352e4504671a0f42a9fcdda55245
122a352e4504671a0f42a9fcdda55245
=> nil
# using an invalid secret throws an error (not 10 length or 32 length)
irb(main):098:0> u.secret = "testinvalidsecret"; u.save; u.reload; nil
=> nil
irb(main):100:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
...
FileUploader::InvalidSecret (FileUploader::InvalidSecret)
# hardcoding (a valid) secret uses this instead of parsing the path
irb(main):104:0> u.secret = "1234567890"; u.save; u.reload; nil
=> nil
irb(main):105:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
1234567890
1234567890
# actual, new, freshly uploaded file, actually has `secret` set, and works as intended
irb(main):107:0> u = Upload.find(1); puts u.path; puts u.secret; nil
bb848e947ee582015e8b2dbbda26a968/test.png
bb848e947ee582015e8b2dbbda26a968
=> nil
irb(main):110:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
bb848e947ee582015e8b2dbbda26a968
bb848e947ee582015e8b2dbbda26a968
irb(main):113:0> u.secret = nil; u.save; u.reload; nil
=> nil
# which is exactly the issue we bump into, despite the path being 32-length now
irb(main):114:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
ef331be80749c88d59c22acf5519d709
ef331be80749c88d59c22acf5519d709
=> nil
irb(main):115:0> ul = u.retrieve_uploader; puts ul.secret; puts ul.send(:dynamic_segment)
4097e3287215a64a7d118151e7fc4246
4097e3287215a64a7d118151e7fc4246
=> nil
That was the initial observation, however we are supposed to fallback to extracting from the path, which seems not to happen in this case?
It looks like we use upload.path
for the fallback, and the matching regex has a /
in the beginning, however upload.path is relative, so it won't match them at all now:
irb(main):058:0> Upload.find(2).path
=> "06670adf08/test2.txt"
I think the workaround might be to remove the initial slash from the regex.
Experienced by Large Premium customer in ZD (internal-only)
cc @dbalexandre