Downloads Screenshots FileExtension Move() method broken

[All Astronauts]

4.5.3

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.

[bfarber]

Thanks, I've logged an internal bug report to get the issue checked on.

[All Astronauts]

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 October 1, 2020 by All Astronauts

[bfarber]

Thanks, I've added those notes to the internal bug report.

[All Astronauts]

@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.

 

[bfarber]

Thanks, updated the report with that information as well.