Fixed #19428 - added StorageHelper for EULA and labels in S3

app/Helpers/StorageHelper.php

@@ -132,4 +132,72 @@ class StorageHelper
         return null;
 
     }
+
+    /**
+     * Return a local filesystem path the caller can hand to native PHP
+     * methods like `getimagesize()`, `fopen()`, or TCPDF's image writer,
+     * regardless of the underlying disk driver.
+     *
+     * Returns null when the file doesn't exist on the disk, matching
+     * the existing `->exists()` semantics used elsewhere in this class.
+     *
+     * @param  string  $filename  path relative to the disk root
+     * @param  string  $disk  filesystem disk name (defaults to `public`)
+     * @return string|null local path, or null if the file is missing
+     */
+    public static function readablePath(string $filename, string $disk = 'public'): ?string
+    {
+        if (!Storage::disk($disk)->exists($filename)) {
+            return null;
+        }
+
+        // Local disk: return the real filesystem path directly. No temp file
+        if (config("filesystems.disks.$disk.driver") === 'local') {
+            return Storage::disk($disk)->path($filename);
+        }
+
+        // Non-local disk: stream the object into a temp file so callers can treat it
+        // like a local path.
+        // Preserving the original extension matters for methods like
+        // getimagesize() that sniff the file type via the extension
+        // before reading bytes.
+        $extension = pathinfo($filename, PATHINFO_EXTENSION);
+        $tmp = tempnam(sys_get_temp_dir(), 'snipeit-readable-');
+        if ($tmp === false) {
+            return null;
+        }
+        if ($extension !== '') {
+            $tmpWithExt = $tmp . '.' . $extension;
+            if (!@rename($tmp, $tmpWithExt)) {
+                @unlink($tmp);
+
+                return null;
+            }
+            $tmp = $tmpWithExt;
+        }
+
+        $stream = Storage::disk($disk)->readStream($filename);
+        if ($stream === null) {
+            @unlink($tmp);
+
+            return null;
+        }
+        $handle = fopen($tmp, 'wb');
+        if ($handle === false) {
+            fclose($stream);
+            @unlink($tmp);
+
+            return null;
+        }
+        stream_copy_to_stream($stream, $handle);
+        fclose($handle);
+        fclose($stream);
+
+        // Auto-clean at request end so the caller doesn't own the
+        // lifecycle. Register once per file so many calls in the same
+        // request each get their own cleanup.
+        register_shutdown_function(fn() => @unlink($tmp));
+
+        return $tmp;
+    }
 }

app/Http/Controllers/Account/AcceptanceController.php

@@ -192,11 +192,12 @@ class AcceptanceController extends Controller
             }
         }
 
-        // Convert PDF logo to base64 for TCPDF
+        // Convert PDF logo to base64 for TCPDF. Reading via the disk (rather
-        // This is needed for TCPDF to properly embed the image if it's a png and the cache isn't writable
+        // than file_get_contents on a local path) keeps this working when
+        // uploads live on s3 or another non-local filesystem.
         $encoded_logo = null;
         if (($settings->acceptance_pdf_logo) && (Storage::disk('public')->exists($settings->acceptance_pdf_logo))) {
-            $encoded_logo = base64_encode(file_get_contents(public_path().'/uploads/'.basename($settings->acceptance_pdf_logo)));
+            $encoded_logo = base64_encode(Storage::disk('public')->get($settings->acceptance_pdf_logo));
         }
 
         // Get the data array ready for the notifications and PDF generation

app/View/Label.php

@@ -2,12 +2,12 @@
 
 namespace App\View;
 
+use App\Helpers\StorageHelper;
 use App\Models\Labels\Field;
 use App\Models\Labels\Label as LabelModel;
 use App\Models\Labels\Sheet;
 use Illuminate\Contracts\View\View;
 use Illuminate\Support\Collection;
-use Illuminate\Support\Facades\Storage;
 use Illuminate\Support\Traits\Macroable;
 use TCPDF;
 
@@ -106,11 +106,15 @@ class Label implements View
 
                     $logo = null;
                     // Should we use the assets assigned company logo? (A.K.A. "Is `Labels > Use Asset Logo` enabled?"), and do we have a company logo?
+                    // StorageHelper::readablePath returns a local path for any disk driver:
+                    // direct passthrough on local, temp-file creation on s3. TCPDF and the
+                    // downstream getimagesize() both need a real path, so it has to land on the
+                    // disk.
                     if ($settings->label2_asset_logo && $asset->company && $asset->company->image != '') {
-                        $logo = Storage::disk('public')->path('companies/'.e($asset->company->image));
+                        $logo = StorageHelper::readablePath('companies/' . e($asset->company->image));
                     } elseif (! empty($settings->label_logo)) {
                         // Use the general site label logo, if available
-                        $logo = Storage::disk('public')->path('/'.e(basename($settings->label_logo)));
+                        $logo = StorageHelper::readablePath(e(basename($settings->label_logo)));
                     } elseif (! empty($asset->is_label_preview)) {
                         $logo = public_path('img/label-preview-logo.png');
                     }

tests/Unit/Helpers/StorageHelperTest.php

@@ -0,0 +1,68 @@
+<?php
+
+namespace Tests\Unit\Helpers;
+
+use App\Helpers\StorageHelper;
+use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+class StorageHelperTest extends TestCase
+{
+    public function test_readable_path_returns_null_when_file_missing(): void
+    {
+        Storage::fake('public');
+
+        $this->assertNull(StorageHelper::readablePath('does-not-exist.png'));
+    }
+
+    public function test_readable_path_returns_direct_disk_path_on_local_driver(): void
+    {
+        $disk = Storage::fake('public');
+        $disk->put('logos/site-logo.png', 'PNGDATA');
+
+        $result = StorageHelper::readablePath('logos/site-logo.png');
+
+        $this->assertSame($disk->path('logos/site-logo.png'), $result);
+        $this->assertFileExists($result);
+        $this->assertSame('PNGDATA', file_get_contents($result));
+    }
+
+    public function test_readable_path_materializes_to_temp_file_on_non_local_driver(): void
+    {
+        // Storage::fake registers a local-backed disk. Overriding the driver
+        // config to 's3' exercises the non-local branch while the underlying
+        // Flysystem adapter still functions normally for reads.
+        $disk = Storage::fake('public');
+        Config::set('filesystems.disks.public.driver', 's3');
+        $disk->put('companies/acme.jpg', 'JPEGBYTES');
+
+        $result = StorageHelper::readablePath('companies/acme.jpg');
+
+        $this->assertIsString($result);
+        $this->assertNotSame($disk->path('companies/acme.jpg'), $result);
+        $this->assertStringEndsWith('.jpg', $result);
+        $this->assertFileExists($result);
+        $this->assertSame('JPEGBYTES', file_get_contents($result));
+
+        @unlink($result);
+    }
+
+    public function test_readable_path_preserves_missing_extension_on_non_local_driver(): void
+    {
+        // getimagesize() sniffs by content, not extension, when the extension
+        // is absent, so we should still return a usable temp file.
+        $disk = Storage::fake('public');
+        Config::set('filesystems.disks.public.driver', 's3');
+        $disk->put('blobs/nofext', 'RAW');
+
+        $result = StorageHelper::readablePath('blobs/nofext');
+
+        $this->assertIsString($result);
+        $this->assertFileExists($result);
+        $this->assertSame('RAW', file_get_contents($result));
+        $this->assertDoesNotMatchRegularExpression('/\.[a-z0-9]+$/i', basename($result));
+
+        @unlink($result);
+    }
+}