Fixed ch15966 - attempts to download extremely large files (Backups) exhausts all memory

[uberbrady]

Laravel has a Storage::download() method which we use, but that breaks pretty hard on large enough files (basically, anything bigger than the memory limit, or so). Backups in particular, for customers who have lots of images, can easily exceed that.

This adds a new StorageHelper Helper class (just a single static method right now) and uses it where it makes sense. Though, see caveats below. I called my method downloader rather than download to make sure we can easily visually distinguish between Storage::download() and StorageHelper::downloader().

I tested it against local storage and against S3, and it was able to download my 1GB file with ease in all the sections where I made modifications.

Some caveats:

• S3 storage doesn't show image previews properly, because our MIME-type-determining code is tied to local storage. I figure that's okay, because it was already like this.
• There are a few remaining places where we have hardcoded references to the local filesystem's storage/private_uploads - most frequently using config('app.private_uploads'). Again, it was already like this already so I figured this isn't any worse.
• Spatie Backup running under S3 local 'private' storage seemed to get very confused, but my stuff didn't mess with any of that (just the 'download' side) so I don't think that's on me; I think it was already that way. Spatie Backup is disk-aware (a.k.a. Flysystem-aware) with the version we're using now, so this could potentially be fixable in the future.

[snipe]

S3 storage doesn't show image previews properly

Not sure what you mean? Thumbnails show okay in the app. Can you clarify?

Spatie Backup is disk-aware (a.k.a. Flysystem-aware) with the version we're using now, so this could potentially be fixable in the future.

This is likely due to the included_dirs stuff we do (specifically the base_path stuff), since we don't want to include allllll of Snipe-IT (including vendors, app files, etc) in backups, as there is no need, and it would bloat out the backup files.

// This is janky, but necessary to figure out whether to include the .env in the backup
$included_dirs = [
    base_path('public/uploads'),
    base_path('storage/private_uploads'),
    base_path('storage/oauth-private.key'),
    base_path('storage/oauth-public.key'),

];

if (env('BACKUP_ENV')=='true') {
    $included_dirs[] = base_path('.env');
}

[uberbrady]

I have two images attached to an asset. One exists fine locally, and one exists fine on S3. When you set the filesystem disk to local, the remote one is broken (of course), and vice-versa. But we should have an image preview, and be able to download the image for whatever private filesystem disk we use. That does work - I can download the local image when my .env is set to use the local private filesystem. I can download the S3-based image when my .env is set to use S3 for private storage.

But the little image previews don't work on S3. Here's some screenshots:

when using a 'local' disk, it look like this:

Which is correct.

When using an 's3' disk, it looks like this:

Which is incorrect - I should see an image preview on the other image. But I suspect it was this way already, because Flysystem has no MIME helpers and the S3 image is in a private bucket - so file_get_contents() is not going to work, which is how we determine MIME-type, which is how we determine whether or not to show an image preview.

[snipe]

Hmm... something must have changed then. Image thumbnail previews were working when we released v5. Either way, it has nothing to do with downloads, so it's not your problem for this PR - but it's definitely weird. I am certain it was working.

[snipe]

I'm a little weirded out that 'local' and 'S3' works for both private and public, but we can try to sort through some of that on develop I think.