Range requests: Extracted stream output handling to new class

diff --git a/app/Http/DownloadResponseFactory.php b/app/Http/DownloadResponseFactory.php
index 20032f525344b439fc563c64da0eb8c4843d528e..f8c10165c80857bbe01f988e279b8c7503ad0ee4 100644 (file)
--- a/app/Http/DownloadResponseFactory.php
+++ b/app/Http/DownloadResponseFactory.php
@@ -9,11 +9,9 @@ use Symfony\Component\HttpFoundation\StreamedResponse;
 
 class DownloadResponseFactory
 {
 
 class DownloadResponseFactory
 {

-    protected Request $request;
-
-    public function __construct(Request $request)
-    {
-        $this->request = $request;
+    public function __construct(
+        protected Request $request
+    ) {

     }
 
     /**
     }
 
     /**


@@ -27,19 +25,11 @@ class DownloadResponseFactory
     /**
      * Create a response that forces a download, from a given stream of content.
      */
     /**
      * Create a response that forces a download, from a given stream of content.
      */

-    public function streamedDirectly($stream, string $fileName): StreamedResponse
+    public function streamedDirectly($stream, string $fileName, int $fileSize): StreamedResponse

     {
     {

-        return response()->stream(function () use ($stream) {
-
-            // End & flush the output buffer, if we're in one, otherwise we still use memory.
-            // Output buffer may or may not exist depending on PHP `output_buffering` setting.
-            // Ignore in testing since output buffers are used to gather a response.
-            if (!empty(ob_get_status()) && !app()->runningUnitTests()) {
-                ob_end_clean();
-            }
-
-            fpassthru($stream);
-            fclose($stream);
+        $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request->headers);
+        return response()->stream(function () use ($rangeStream) {
+            $rangeStream->outputAndClose();

         }, 200, $this->getHeaders($fileName));
     }
 
         }, 200, $this->getHeaders($fileName));
     }
 


@@ -48,15 +38,13 @@ class DownloadResponseFactory
      * correct for the file, in a way so the browser can show the content in browser,
      * for a given content stream.
      */
      * correct for the file, in a way so the browser can show the content in browser,
      * for a given content stream.
      */

-    public function streamedInline($stream, string $fileName): StreamedResponse
+    public function streamedInline($stream, string $fileName, int $fileSize): StreamedResponse

     {
     {

-        $sniffContent = fread($stream, 2000);
-        $mime = (new WebSafeMimeSniffer())->sniff($sniffContent);
+        $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request->headers);
+        $mime = $rangeStream->sniffMime();

 
 

-        return response()->stream(function () use ($sniffContent, $stream) {
-            echo $sniffContent;
-            fpassthru($stream);
-            fclose($stream);
+        return response()->stream(function () use ($rangeStream) {
+            $rangeStream->outputAndClose();

         }, 200, $this->getHeaders($fileName, $mime));
     }
 
         }, 200, $this->getHeaders($fileName, $mime));
     }
 

diff --git a/app/Http/RangeSupportedStream.php b/app/Http/RangeSupportedStream.php
new file mode 100644 (file)
index 0000000..dc31050
--- /dev/null
+++ b/app/Http/RangeSupportedStream.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace BookStack\Http;
+
+use BookStack\Util\WebSafeMimeSniffer;
+use Symfony\Component\HttpFoundation\HeaderBag;
+
+class RangeSupportedStream
+{
+    protected string $sniffContent;
+
+    public function __construct(
+        protected $stream,
+        protected int $fileSize,
+        protected HeaderBag $requestHeaders,
+    ) {
+    }
+
+    /**
+     * Sniff a mime type from the stream.
+     */
+    public function sniffMime(): string
+    {
+        $offset = min(2000, $this->fileSize);
+        $this->sniffContent = fread($this->stream, $offset);
+
+        return (new WebSafeMimeSniffer())->sniff($this->sniffContent);
+    }
+
+    /**
+     * Output the current stream to stdout before closing out the stream.
+     */
+    public function outputAndClose(): void
+    {
+        // End & flush the output buffer, if we're in one, otherwise we still use memory.
+        // Output buffer may or may not exist depending on PHP `output_buffering` setting.
+        // Ignore in testing since output buffers are used to gather a response.
+        if (!empty(ob_get_status()) && !app()->runningUnitTests()) {
+            ob_end_clean();
+        }
+
+        $outStream = fopen('php://output', 'w');
+        $offset = 0;
+
+        if (!empty($this->sniffContent)) {
+            fwrite($outStream, $this->sniffContent);
+            $offset = strlen($this->sniffContent);
+        }
+
+        $toWrite = $this->fileSize - $offset;
+        stream_copy_to_stream($this->stream, $outStream, $toWrite);
+        fpassthru($this->stream);
+
+        fclose($this->stream);
+        fclose($outStream);
+    }
+}

diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index ddabec09f2733663486fdc301d4b8dbb35ceed11..72f78e347bf54cd61d5f084d3fd7a456e6d07c0d 100644 (file)
--- a/app/Uploads/AttachmentService.php
+++ b/app/Uploads/AttachmentService.php
@@ -66,8 +66,6 @@ class AttachmentService
     /**
      * Stream an attachment from storage.
      *
     /**
      * Stream an attachment from storage.
      *

-     * @throws FileNotFoundException
-     *

      * @return resource|null
      */
     public function streamAttachmentFromStorage(Attachment $attachment)
      * @return resource|null
      */
     public function streamAttachmentFromStorage(Attachment $attachment)


@@ -75,6 +73,14 @@ class AttachmentService
         return $this->getStorageDisk()->readStream($this->adjustPathForStorageDisk($attachment->path));
     }
 
         return $this->getStorageDisk()->readStream($this->adjustPathForStorageDisk($attachment->path));
     }
 

+    /**
+     * Read the file size of an attachment from storage, in bytes.
+     */
+    public function getAttachmentFileSize(Attachment $attachment): int
+    {
+        return $this->getStorageDisk()->size($this->adjustPathForStorageDisk($attachment->path));
+    }
+

     /**
      * Store a new attachment upon user upload.
      *
     /**
      * Store a new attachment upon user upload.
      *

diff --git a/app/Uploads/Controllers/AttachmentController.php b/app/Uploads/Controllers/AttachmentController.php
index 92f23465d2234f8ef8469a14efdbe0a75f8081eb..e61c1033884586d10957236b2bfe2668cfa690aa 100644 (file)
--- a/app/Uploads/Controllers/AttachmentController.php
+++ b/app/Uploads/Controllers/AttachmentController.php
@@ -226,12 +226,13 @@ class AttachmentController extends Controller
 
         $fileName = $attachment->getFileName();
         $attachmentStream = $this->attachmentService->streamAttachmentFromStorage($attachment);
 
         $fileName = $attachment->getFileName();
         $attachmentStream = $this->attachmentService->streamAttachmentFromStorage($attachment);

+        $attachmentSize = $this->attachmentService->getAttachmentFileSize($attachment);

 
         if ($request->get('open') === 'true') {
 
         if ($request->get('open') === 'true') {

-            return $this->download()->streamedInline($attachmentStream, $fileName);
+            return $this->download()->streamedInline($attachmentStream, $fileName, $attachmentSize);

         }
 
         }
 

-        return $this->download()->streamedDirectly($attachmentStream, $fileName);
+        return $this->download()->streamedDirectly($attachmentStream, $fileName, $attachmentSize);

     }
 
     /**
     }
 
     /**