Merge pull request #3365 from BookStackApp/data_streaming

Add data streaming where beneficial to reduce memory usage

app/Http/Controllers/Api/AttachmentApiController.php

@@ -87,14 +87,32 @@ class AttachmentApiController extends ApiController
             'markdown' => $attachment->markdownLink(),
         ]);
 
-        if (!$attachment->external) {
+        // Simply return a JSON response of the attachment for link-based attachments
-            $attachmentContents = $this->attachmentService->getAttachmentFromStorage($attachment);
+        if ($attachment->external) {
-            $attachment->setAttribute('content', base64_encode($attachmentContents));
-        } else {
             $attachment->setAttribute('content', $attachment->path);
+            return response()->json($attachment);
         }
 
-        return response()->json($attachment);
+        // Build and split our core JSON, at point of content.
+        $splitter = 'CONTENT_SPLIT_LOCATION_' . time() . '_' . rand(1, 40000);
+        $attachment->setAttribute('content', $splitter);
+        $json = $attachment->toJson();
+        $jsonParts = explode($splitter, $json);
+        // Get a stream for the file data from storage
+        $stream = $this->attachmentService->streamAttachmentFromStorage($attachment);
+
+        return response()->stream(function () use ($jsonParts, $stream) {
+            // Output the pre-content JSON data
+            echo $jsonParts[0];
+
+            // Stream out our attachment data as base64 content
+            stream_filter_append($stream, 'convert.base64-encode', STREAM_FILTER_READ);
+            fpassthru($stream);
+            fclose($stream);
+
+            // Output our post-content JSON data
+            echo $jsonParts[1];
+        }, 200, ['Content-Type' => 'application/json']);
     }
 
     /**

app/Http/Controllers/AttachmentController.php

@@ -15,8 +15,8 @@ use Illuminate\Validation\ValidationException;
 
 class AttachmentController extends Controller
 {
-    protected $attachmentService;
+    protected AttachmentService $attachmentService;
-    protected $pageRepo;
+    protected PageRepo $pageRepo;
 
     /**
      * AttachmentController constructor.
@@ -230,13 +230,13 @@ class AttachmentController extends Controller
         }
 
         $fileName = $attachment->getFileName();
-        $attachmentContents = $this->attachmentService->getAttachmentFromStorage($attachment);
+        $attachmentStream = $this->attachmentService->streamAttachmentFromStorage($attachment);
 
         if ($request->get('open') === 'true') {
-            return $this->inlineDownloadResponse($attachmentContents, $fileName);
+            return $this->streamedInlineDownloadResponse($attachmentStream, $fileName);
         }
 
-        return $this->downloadResponse($attachmentContents, $fileName);
+        return $this->streamedDownloadResponse($attachmentStream, $fileName);
     }
 
     /**

app/Http/Controllers/Controller.php

@@ -12,6 +12,7 @@ use Illuminate\Foundation\Validation\ValidatesRequests;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Response;
 use Illuminate\Routing\Controller as BaseController;
+use Symfony\Component\HttpFoundation\StreamedResponse;
 
 abstract class Controller extends BaseController
 {
@@ -115,7 +116,28 @@ abstract class Controller extends BaseController
     {
         return response()->make($content, 200, [
             'Content-Type'           => 'application/octet-stream',
-            'Content-Disposition'    => 'attachment; filename="' . $fileName . '"',
+            'Content-Disposition'    => 'attachment; filename="' . str_replace('"', '', $fileName) . '"',
+            'X-Content-Type-Options' => 'nosniff',
+        ]);
+    }
+
+    /**
+     * Create a response that forces a download, from a given stream of content.
+     */
+    protected function streamedDownloadResponse($stream, string $fileName): StreamedResponse
+    {
+        return response()->stream(function() use ($stream) {
+            // End & flush the output buffer otherwise we still seem to use memory.
+            // Ignore in testing since output buffers are used to gather a response.
+            if (!app()->runningUnitTests()) {
+                ob_end_clean();
+            }
+
+            fpassthru($stream);
+            fclose($stream);
+        }, 200, [
+            'Content-Type'           => 'application/octet-stream',
+            'Content-Disposition'    => 'attachment; filename="' . str_replace('"', '', $fileName) . '"',
             'X-Content-Type-Options' => 'nosniff',
         ]);
     }
@@ -130,7 +152,28 @@ abstract class Controller extends BaseController
 
         return response()->make($content, 200, [
             'Content-Type'           => $mime,
-            'Content-Disposition'    => 'inline; filename="' . $fileName . '"',
+            'Content-Disposition'    => 'inline; filename="' . str_replace('"', '', $fileName) . '"',
+            'X-Content-Type-Options' => 'nosniff',
+        ]);
+    }
+
+    /**
+     * Create a file download response that provides the file with a content-type
+     * correct for the file, in a way so the browser can show the content in browser,
+     * for a given content stream.
+     */
+    protected function streamedInlineDownloadResponse($stream, string $fileName): StreamedResponse
+    {
+        $sniffContent = fread($stream, 1000);
+        $mime = (new WebSafeMimeSniffer())->sniff($sniffContent);
+
+        return response()->stream(function() use ($sniffContent, $stream) {
+           echo $sniffContent;
+           fpassthru($stream);
+           fclose($stream);
+        }, 200, [
+            'Content-Type'           => $mime,
+            'Content-Disposition'    => 'inline; filename="' . str_replace('"', '', $fileName) . '"',
             'X-Content-Type-Options' => 'nosniff',
         ]);
     }

app/Uploads/AttachmentService.php

@@ -14,7 +14,7 @@ use Symfony\Component\HttpFoundation\File\UploadedFile;
 
 class AttachmentService
 {
-    protected $fileSystem;
+    protected FilesystemManager $fileSystem;
 
     /**
      * AttachmentService constructor.
@@ -73,6 +73,18 @@ class AttachmentService
         return $this->getStorageDisk()->get($this->adjustPathForStorageDisk($attachment->path));
     }
 
+    /**
+     * Stream an attachment from storage.
+     *
+     * @return resource|null
+     * @throws FileNotFoundException
+     */
+    public function streamAttachmentFromStorage(Attachment $attachment)
+    {
+
+        return $this->getStorageDisk()->readStream($this->adjustPathForStorageDisk($attachment->path));
+    }
+
     /**
      * Store a new attachment upon user upload.
      *
@@ -211,8 +223,6 @@ class AttachmentService
      */
     protected function putFileInStorage(UploadedFile $uploadedFile): string
     {
-        $attachmentData = file_get_contents($uploadedFile->getRealPath());
-
         $storage = $this->getStorageDisk();
         $basePath = 'uploads/files/' . date('Y-m-M') . '/';
 
@@ -221,10 +231,11 @@ class AttachmentService
             $uploadFileName = Str::random(3) . $uploadFileName;
         }
 
+        $attachmentStream = fopen($uploadedFile->getRealPath(), 'r');
         $attachmentPath = $basePath . $uploadFileName;
 
         try {
-            $storage->put($this->adjustPathForStorageDisk($attachmentPath), $attachmentData);
+            $storage->writeStream($this->adjustPathForStorageDisk($attachmentPath), $attachmentStream);
         } catch (Exception $e) {
             Log::error('Error when attempting file upload:' . $e->getMessage());
 

resources/views/common/export-styles.blade.php

@@ -1,5 +1,5 @@
 <style>
-    @if (!app()->environment('testing'))
+    @if (!app()->runningUnitTests())
         {!! file_get_contents(public_path('/dist/export-styles.css')) !!}
     @endif
 </style>

tests/Api/AttachmentsApiTest.php

@@ -5,6 +5,7 @@ namespace Tests\Api;
 use BookStack\Entities\Models\Page;
 use BookStack\Uploads\Attachment;
 use Illuminate\Http\UploadedFile;
+use Illuminate\Testing\AssertableJsonString;
 use Tests\TestCase;
 
 class AttachmentsApiTest extends TestCase
@@ -228,9 +229,11 @@ class AttachmentsApiTest extends TestCase
         $attachment = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->firstOrFail();
 
         $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
-
         $resp->assertStatus(200);
-        $resp->assertJson([
+        $resp->assertHeader('Content-Type', 'application/json');
+
+        $json = new AssertableJsonString($resp->streamedContent());
+        $json->assertSubset([
             'id'          => $attachment->id,
             'content'     => base64_encode(file_get_contents(storage_path($attachment->path))),
             'external'    => false,

tests/Uploads/AttachmentTest.php

@@ -128,7 +128,8 @@ class AttachmentTest extends TestCase
         $pageGet->assertSee($attachment->getUrl());
 
         $attachmentGet = $this->get($attachment->getUrl());
-        $attachmentGet->assertSee('Hi, This is a test file for testing the upload process.');
+        $content = $attachmentGet->streamedContent();
+        $this->assertStringContainsString('Hi, This is a test file for testing the upload process.', $content);
 
         $this->deleteUploads();
     }