Merge pull request #15689 from snipe/better_handle_inline_files

Better handle inline files in file listing
snipe · Oct 22, 2024 · 252d994 · 252d994
2 parents 5767a98 + db81701
commit 252d994
Show file tree

Hide file tree

Showing 20 changed files with 325 additions and 945 deletions.
diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php
@@ -1123,6 +1123,7 @@ public static function filetype_icon($filename)
             'png'   => 'far fa-image',
             'webp'   => 'far fa-image',
             'avif'   => 'far fa-image',
+            'svg' => 'fas fa-vector-square',
             // word
             'doc'   => 'far fa-file-word',
             'docx'   => 'far fa-file-word',
@@ -1135,7 +1136,7 @@ public static function filetype_icon($filename)
             //Text
             'txt'   => 'far fa-file-alt',
             'rtf'   => 'far fa-file-alt',
-            'xml'   => 'far fa-file-alt',
+            'xml'   => 'fas fa-code',
             // Misc
             'pdf'   => 'far fa-file-pdf',
             'lic'   => 'far fa-save',
@@ -1148,41 +1149,7 @@ public static function filetype_icon($filename)
         return 'far fa-file';
     }
 
-    public static function show_file_inline($filename)
-    {
-        $extension = substr(strrchr($filename, '.'), 1);
-
-        if ($extension) {
-            switch ($extension) {
-                case 'jpg':
-                case 'jpeg':
-                case 'gif':
-                case 'png':
-                case 'webp':
-                case 'avif':
-                    return true;
-                    break;
-                default:
-                    return false;
-            }
-        }
 
-        return false;
-    }
-
-    /**
-     * Generate a random encrypted password.
-     *
-     * @author Wes Hulette <[email protected]>
-     *
-     * @since 5.0.0
-     *
-     * @return string
-     */
-    public static function generateEncyrptedPassword(): string
-    {
-        return bcrypt(self::generateUnencryptedPassword());
-    }
 
     /**
      * Get a random unencrypted password.

diff --git a/app/Helpers/StorageHelper.php b/app/Helpers/StorageHelper.php
@@ -7,6 +7,7 @@
 use Illuminate\Http\RedirectResponse;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\StreamedResponse;
+use Illuminate\Contracts\Filesystem\FileNotFoundException;
 class StorageHelper
 {
     public static function downloader($filename, $disk = 'default') : BinaryFileResponse | RedirectResponse | StreamedResponse
@@ -25,4 +26,64 @@ public static function downloader($filename, $disk = 'default') : BinaryFileResp
                 return Storage::disk($disk)->download($filename);
         }
     }
+
+
+    /**
+     * This determines the file types that should be allowed inline and checks their fileinfo extension
+     * to determine that they are safe to display inline.
+     *
+     * @author <A. Gianotto> [<[email protected]]>
+     * @since v7.0.14
+     * @param $file_with_path
+     * @return bool
+     */
+    public static function allowSafeInline($file_with_path) {
+
+        $allowed_inline = [
+            'pdf',
+            'svg',
+            'jpg',
+            'gif',
+            'svg',
+            'avif',
+            'webp',
+            'png',
+        ];
+
+
+        // The file exists and is allowed to be displayed inline
+        if (Storage::exists($file_with_path) && (in_array(pathinfo($file_with_path, PATHINFO_EXTENSION), $allowed_inline))) {
+            return true;
+        }
+        return false;
+
+    }
+
+    /**
+     * Decide whether to show the file inline or download it.
+     */
+    public static function showOrDownloadFile($file, $filename) {
+
+        $headers = [];
+
+        if (request('inline') == 'true') {
+
+            $headers = [
+                'Content-Disposition' => 'inline',
+            ];
+
+            // This is NOT allowed as inline - force it to be displayed as text in the browser
+            if (self::allowSafeInline($file) != true) {
+                $headers = array_merge($headers, ['Content-Type' => 'text/plain']);
+            }
+        }
+
+        // Everything else seems okay, but the file doesn't exist on the server.
+        if (Storage::missing($file)) {
+            throw new FileNotFoundException();
+        }
+
+        return Storage::download($file, $filename, $headers);
+
+    }
 }
diff --git a/app/Http/Controllers/Accessories/AccessoriesFilesController.php b/app/Http/Controllers/Accessories/AccessoriesFilesController.php
@@ -106,50 +106,29 @@ public function destroy($accessoryId = null, $fileId = null) : RedirectResponse
      * @param int $accessoryId
      * @param int $fileId
      */
-    public function show($accessoryId = null, $fileId = null, $download = true) : View | RedirectResponse | Response | BinaryFileResponse | StreamedResponse
+    public function show($accessoryId = null, $fileId = null) : View | RedirectResponse | Response | BinaryFileResponse | StreamedResponse
     {
 
-        Log::debug('Private filesystem is: '.config('filesystems.default'));
-        $accessory = Accessory::find($accessoryId);
-
-
 
         // the accessory is valid
-        if (isset($accessory->id)) {
+        if ($accessory = Accessory::find($accessoryId)) {
             $this->authorize('view', $accessory);
             $this->authorize('accessories.files', $accessory);
 
-            if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $accessory->id)->find($fileId)) {
-                return redirect()->route('accessories.index')->with('error',  trans('admin/users/message.log_record_not_found'));
-            }
-
-            $file = 'private_uploads/accessories/'.$log->filename;
-
-            if (Storage::missing($file)) {
-                Log::debug('FILE DOES NOT EXISTS for '.$file);
-                Log::debug('URL should be '.Storage::url($file));
+            if ($log = Actionlog::whereNotNull('filename')->where('item_id', $accessory->id)->find($fileId)) {
+                $file = 'private_uploads/accessories/'.$log->filename;
 
-                return response('File '.$file.' ('.Storage::url($file).') not found on server', 404)
-                    ->header('Content-Type', 'text/plain');
-            } else {
-
-                // Display the file inline
-                if (request('inline') == 'true') {
-                    $headers = [
-                        'Content-Disposition' => 'inline',
-                    ];
-                    return Storage::download($file, $log->filename, $headers);
+                try {
+                    return StorageHelper::showOrDownloadFile($file, $log->filename);
+                } catch (\Exception $e) {
+                    return redirect()->route('accessories.show', ['accessory' => $accessory])->with('error',  trans('general.file_not_found'));
                 }
+            }
 
+            return redirect()->route('accessories.show', ['accessory' => $accessory])->with('error',  trans('general.log_record_not_found'));
 
-                // We have to override the URL stuff here, since local defaults in Laravel's Flysystem
-                // won't work, as they're not accessible via the web
-                if (config('filesystems.default') == 'local') { // TODO - is there any way to fix this at the StorageHelper layer?
-                    return StorageHelper::downloader($file);
-                }
-            }
         }
 
-        return redirect()->route('accessories.index')->with('error', trans('general.file_does_not_exist', ['id' => $fileId]));
+        return redirect()->route('accessories.index')->with('error', trans('general.file_not_found'));
     }
 }
diff --git a/app/Http/Controllers/Assets/AssetFilesController.php b/app/Http/Controllers/Assets/AssetFilesController.php
@@ -61,43 +61,30 @@ public function store(UploadFileRequest $request, $assetId = null) : RedirectRes
      */
     public function show($assetId = null, $fileId = null) : View | RedirectResponse | Response | StreamedResponse | BinaryFileResponse
     {
-        $asset = Asset::find($assetId);
-        // the asset is valid
-        if (isset($asset->id)) {
-            $this->authorize('view', $asset);
-
-            if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $asset->id)->find($fileId)) {
-                return response('No matching record for that asset/file', 500)
-                    ->header('Content-Type', 'text/plain');
-            }
-
-            $file = 'private_uploads/assets/'.$log->filename;
+        if ($asset = Asset::find($assetId)) {
 
-            if ($log->action_type == 'audit') {
-                $file = 'private_uploads/audits/'.$log->filename;
-            }
+            $this->authorize('view', $asset);
 
-            if (! Storage::exists($file)) {
-                return response('File '.$file.' not found on server', 404)
-                    ->header('Content-Type', 'text/plain');
-            }
+            if ($log = Actionlog::whereNotNull('filename')->where('item_id', $asset->id)->find($fileId)) {
+                $file = 'private_uploads/assets/'.$log->filename;
 
-            if (request('inline') == 'true') {
+                if ($log->action_type == 'audit') {
+                    $file = 'private_uploads/audits/'.$log->filename;
+                }
 
-                $headers = [
-                    'Content-Disposition' => 'inline',
-                ];
+                try {
+                     return StorageHelper::showOrDownloadFile($file, $log->filename);
+                } catch (\Exception $e) {
+                    return redirect()->route('hardware.show', ['hardware' => $asset])->with('error',  trans('general.file_not_found'));
+                }
 
-                return Storage::download($file, $log->filename, $headers);
             }
 
-            return StorageHelper::downloader($file);
+            return redirect()->route('hardware.show', ['hardware' => $asset])->with('error',  trans('general.log_record_not_found'));
         }
-        // Prepare the error message
-        $error = trans('admin/hardware/message.does_not_exist', ['id' => $fileId]);
 
-        // Redirect to the hardware management page
-        return redirect()->route('hardware.index')->with('error', $error);
+        return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist'));
+
     }
 
     /**

diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php
@@ -17,7 +17,6 @@
 use App\Models\Setting;
 use App\Models\Statuslabel;
 use App\Models\User;
-use Illuminate\Support\Facades\Auth;
 use App\View\Label;
 use Carbon\Carbon;
 use Illuminate\Support\Facades\DB;

diff --git a/app/Http/Controllers/Components/ComponentsFilesController.php b/app/Http/Controllers/Components/ComponentsFilesController.php
@@ -112,40 +112,25 @@ public function destroy($componentId = null, $fileId = null)
     public function show($componentId = null, $fileId = null)
     {
         Log::debug('Private filesystem is: '.config('filesystems.default'));
-        $component = Component::find($componentId);
+
 
         // the component is valid
-        if (isset($component->id)) {
+        if ($component = Component::find($componentId)) {
             $this->authorize('view', $component);
             $this->authorize('components.files', $component);
 
-            if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $component->id)->find($fileId)) {
-                return response('No matching record for that asset/file', 500)
-                    ->header('Content-Type', 'text/plain');
-            }
-
-            $file = 'private_uploads/components/'.$log->filename;
+            if ($log = Actionlog::whereNotNull('filename')->where('item_id', $component->id)->find($fileId)) {
 
-            if (Storage::missing($file)) {
-                Log::debug('FILE DOES NOT EXISTS for '.$file);
-                Log::debug('URL should be '.Storage::url($file));
+                $file = 'private_uploads/components/'.$log->filename;
 
-                return response('File '.$file.' ('.Storage::url($file).') not found on server', 404)
-                    ->header('Content-Type', 'text/plain');
-            } else {
-
-                // Display the file inline
-                if (request('inline') == 'true') {
-                    $headers = [
-                        'Content-Disposition' => 'inline',
-                    ];
-                    return Storage::download($file, $log->filename, $headers);
+                try {
+                    return StorageHelper::showOrDownloadFile($file, $log->filename);
+                } catch (\Exception $e) {
+                    return redirect()->route('components.show', ['component' => $component])->with('error',  trans('general.file_not_found'));
                 }
-
-                if (config('filesystems.default') == 'local') { // TODO - is there any way to fix this at the StorageHelper layer?
-                    return StorageHelper::downloader($file);
-                } 
             }
+            return redirect()->route('components.show', ['component' => $component])->with('error',  trans('general.log_record_not_found'));
+
         }
 
         return redirect()->route('components.index')->with('error', trans('general.file_does_not_exist', ['id' => $fileId]));

diff --git a/app/Http/Controllers/Consumables/ConsumablesFilesController.php b/app/Http/Controllers/Consumables/ConsumablesFilesController.php
@@ -104,7 +104,6 @@ public function destroy($consumableId = null, $fileId = null)
      * @since [v1.4]
      * @param int $consumableId
      * @param int $fileId
-     * @return \Symfony\Consumable\HttpFoundation\Response
      * @throws \Illuminate\Auth\Access\AuthorizationException
      */
     public function show($consumableId = null, $fileId = null)
@@ -116,36 +115,18 @@ public function show($consumableId = null, $fileId = null)
             $this->authorize('view', $consumable);
             $this->authorize('consumables.files', $consumable);
 
-            if (! $log = Actionlog::whereNotNull('filename')->where('item_id', $consumable->id)->find($fileId)) {
-                return response('No matching record for that asset/file', 500)
-                    ->header('Content-Type', 'text/plain');
-            }
-
-            $file = 'private_uploads/consumables/'.$log->filename;
-
-            if (Storage::missing($file)) {
-                Log::debug('FILE DOES NOT EXISTS for '.$file);
-                Log::debug('URL should be '.Storage::url($file));
-
-                return response('File '.$file.' ('.Storage::url($file).') not found on server', 404)
-                    ->header('Content-Type', 'text/plain');
-            } else {
-
-                // Display the file inline
-                if (request('inline') == 'true') {
-                    $headers = [
-                        'Content-Disposition' => 'inline',
-                    ];
-                    return Storage::download($file, $log->filename, $headers);
-                }
-
+            if ($log = Actionlog::whereNotNull('filename')->where('item_id', $consumable->id)->find($fileId)) {
+                $file = 'private_uploads/consumables/'.$log->filename;
 
-                // We have to override the URL stuff here, since local defaults in Laravel's Flysystem
-                // won't work, as they're not accessible via the web
-                if (config('filesystems.default') == 'local') { // TODO - is there any way to fix this at the StorageHelper layer?
-                    return StorageHelper::downloader($file);
+                try {
+                    return StorageHelper::showOrDownloadFile($file, $log->filename);
+                } catch (\Exception $e) {
+                    return redirect()->route('consumables.show', ['consumable' => $consumable])->with('error',  trans('general.file_not_found'));
                 }
             }
+            // The log record doesn't exist somehow
+            return redirect()->route('consumables.show', ['consumable' => $consumable])->with('error',  trans('general.log_record_not_found'));
+
         }
 
         return redirect()->route('consumables.index')->with('error', trans('general.file_does_not_exist', ['id' => $fileId]));