Jump to content

Downloads Screenshots FileExtension Move() method broken

Go to solution Solved by bfarber,

Recommended Posts


You do not account for all screenshot image copies; specifically you do not move the original not-watermarked screenshot images when those files are watermarked. 

record_no_watermark is not accounted for during the move process

End result is record_location and record_thumb files are moved, but record_no_watermark, the only copy of the screenshot not watermarked, is left behind.

If the actual OLD file storage location is still valid, that record_no_watermark file will still be retrieved as the file and url remain valid since they were not moved. Obviously nothing ideal happening here though.

Link to comment
Share on other sites

Addendum:  isValidFile() also doesn't account for record_no_watermark files, so if Downloads or the system ever hit that with a record_no_watermark file it would return FALSE of course.

fixUrls() as well but we are probably WAY beyond the need for any changes there. Or rather, I hope no one is tooling around with 4.0 right now.

EDIT: Also, in Downloads settings, the watermark image file upload helper is using core_Theme for the file extension, which it probably shouldn't.

I'll show myself out now.

Edited by All Astronauts
Link to comment
Share on other sites

@bfarber One more, than I'm really out on this.

RebuildScreenshotWatermarks() queue task

Line 91 you run the code for when there is a no_watermark original copy. The method copies that no_watermark copy to an $original variable, deletes the location and thumb files, and then if there is no $watermark (meaning this rebuild method is running when someone is removing watermarks from their screenshots), updates the downloads_files_records table row by storing the url to $original in location, creates a new thumbnail copy off of $original and stores that url in thumb, and then sets the no_watermark column to NULL.

At no time do you actually delete the no_watermark file in this situation so those files are now zombies with no url stored anywhere for a FileHandler to grab them.


Link to comment
Share on other sites

  • Recently Browsing   0 members

    • No registered users viewing this page.
  • Create New...