Extracted download response logic to its own class

Cleans up base controller and groups up download & streaming logic for
potential future easier addition of range request support.

app/Http/Controllers/Api/BookExportApiController.php

@@ -26,7 +26,7 @@ class BookExportApiController extends ApiController
         $book = Book::visible()->findOrFail($id);
         $pdfContent = $this->exportFormatter->bookToPdf($book);
 
-        return $this->downloadResponse($pdfContent, $book->slug . '.pdf');
+        return $this->download()->directly($pdfContent, $book->slug . '.pdf');
     }
 
     /**
@@ -39,7 +39,7 @@ class BookExportApiController extends ApiController
         $book = Book::visible()->findOrFail($id);
         $htmlContent = $this->exportFormatter->bookToContainedHtml($book);
 
-        return $this->downloadResponse($htmlContent, $book->slug . '.html');
+        return $this->download()->directly($htmlContent, $book->slug . '.html');
     }
 
     /**
@@ -50,7 +50,7 @@ class BookExportApiController extends ApiController
         $book = Book::visible()->findOrFail($id);
         $textContent = $this->exportFormatter->bookToPlainText($book);
 
-        return $this->downloadResponse($textContent, $book->slug . '.txt');
+        return $this->download()->directly($textContent, $book->slug . '.txt');
     }
 
     /**
@@ -61,6 +61,6 @@ class BookExportApiController extends ApiController
         $book = Book::visible()->findOrFail($id);
         $markdown = $this->exportFormatter->bookToMarkdown($book);
 
-        return $this->downloadResponse($markdown, $book->slug . '.md');
+        return $this->download()->directly($markdown, $book->slug . '.md');
     }
 }

app/Http/Controllers/Api/ChapterExportApiController.php

@@ -29,7 +29,7 @@ class ChapterExportApiController extends ApiController
         $chapter = Chapter::visible()->findOrFail($id);
         $pdfContent = $this->exportFormatter->chapterToPdf($chapter);
 
-        return $this->downloadResponse($pdfContent, $chapter->slug . '.pdf');
+        return $this->download()->directly($pdfContent, $chapter->slug . '.pdf');
     }
 
     /**
@@ -42,7 +42,7 @@ class ChapterExportApiController extends ApiController
         $chapter = Chapter::visible()->findOrFail($id);
         $htmlContent = $this->exportFormatter->chapterToContainedHtml($chapter);
 
-        return $this->downloadResponse($htmlContent, $chapter->slug . '.html');
+        return $this->download()->directly($htmlContent, $chapter->slug . '.html');
     }
 
     /**
@@ -53,7 +53,7 @@ class ChapterExportApiController extends ApiController
         $chapter = Chapter::visible()->findOrFail($id);
         $textContent = $this->exportFormatter->chapterToPlainText($chapter);
 
-        return $this->downloadResponse($textContent, $chapter->slug . '.txt');
+        return $this->download()->directly($textContent, $chapter->slug . '.txt');
     }
 
     /**
@@ -64,6 +64,6 @@ class ChapterExportApiController extends ApiController
         $chapter = Chapter::visible()->findOrFail($id);
         $markdown = $this->exportFormatter->chapterToMarkdown($chapter);
 
-        return $this->downloadResponse($markdown, $chapter->slug . '.md');
+        return $this->download()->directly($markdown, $chapter->slug . '.md');
     }
 }

app/Http/Controllers/Api/PageExportApiController.php

@@ -26,7 +26,7 @@ class PageExportApiController extends ApiController
         $page = Page::visible()->findOrFail($id);
         $pdfContent = $this->exportFormatter->pageToPdf($page);
 
-        return $this->downloadResponse($pdfContent, $page->slug . '.pdf');
+        return $this->download()->directly($pdfContent, $page->slug . '.pdf');
     }
 
     /**
@@ -39,7 +39,7 @@ class PageExportApiController extends ApiController
         $page = Page::visible()->findOrFail($id);
         $htmlContent = $this->exportFormatter->pageToContainedHtml($page);
 
-        return $this->downloadResponse($htmlContent, $page->slug . '.html');
+        return $this->download()->directly($htmlContent, $page->slug . '.html');
     }
 
     /**
@@ -50,7 +50,7 @@ class PageExportApiController extends ApiController
         $page = Page::visible()->findOrFail($id);
         $textContent = $this->exportFormatter->pageToPlainText($page);
 
-        return $this->downloadResponse($textContent, $page->slug . '.txt');
+        return $this->download()->directly($textContent, $page->slug . '.txt');
     }
 
     /**
@@ -61,6 +61,6 @@ class PageExportApiController extends ApiController
         $page = Page::visible()->findOrFail($id);
         $markdown = $this->exportFormatter->pageToMarkdown($page);
 
-        return $this->downloadResponse($markdown, $page->slug . '.md');
+        return $this->download()->directly($markdown, $page->slug . '.md');
     }
 }

app/Http/Controllers/AttachmentController.php

@@ -233,10 +233,10 @@ class AttachmentController extends Controller
         $attachmentStream = $this->attachmentService->streamAttachmentFromStorage($attachment);
 
         if ($request->get('open') === 'true') {
-            return $this->streamedInlineDownloadResponse($attachmentStream, $fileName);
+            return $this->download()->streamedInline($attachmentStream, $fileName);
         }
 
-        return $this->streamedDownloadResponse($attachmentStream, $fileName);
+        return $this->download()->streamedDirectly($attachmentStream, $fileName);
     }
 
     /**

app/Http/Controllers/BookExportController.php

@@ -31,7 +31,7 @@ class BookExportController extends Controller
         $book = $this->bookRepo->getBySlug($bookSlug);
         $pdfContent = $this->exportFormatter->bookToPdf($book);
 
-        return $this->downloadResponse($pdfContent, $bookSlug . '.pdf');
+        return $this->download()->directly($pdfContent, $bookSlug . '.pdf');
     }
 
     /**
@@ -44,7 +44,7 @@ class BookExportController extends Controller
         $book = $this->bookRepo->getBySlug($bookSlug);
         $htmlContent = $this->exportFormatter->bookToContainedHtml($book);
 
-        return $this->downloadResponse($htmlContent, $bookSlug . '.html');
+        return $this->download()->directly($htmlContent, $bookSlug . '.html');
     }
 
     /**
@@ -55,7 +55,7 @@ class BookExportController extends Controller
         $book = $this->bookRepo->getBySlug($bookSlug);
         $textContent = $this->exportFormatter->bookToPlainText($book);
 
-        return $this->downloadResponse($textContent, $bookSlug . '.txt');
+        return $this->download()->directly($textContent, $bookSlug . '.txt');
     }
 
     /**
@@ -66,6 +66,6 @@ class BookExportController extends Controller
         $book = $this->bookRepo->getBySlug($bookSlug);
         $textContent = $this->exportFormatter->bookToMarkdown($book);
 
-        return $this->downloadResponse($textContent, $bookSlug . '.md');
+        return $this->download()->directly($textContent, $bookSlug . '.md');
     }
 }

app/Http/Controllers/ChapterExportController.php

@@ -33,7 +33,7 @@ class ChapterExportController extends Controller
         $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
         $pdfContent = $this->exportFormatter->chapterToPdf($chapter);
 
-        return $this->downloadResponse($pdfContent, $chapterSlug . '.pdf');
+        return $this->download()->directly($pdfContent, $chapterSlug . '.pdf');
     }
 
     /**
@@ -47,7 +47,7 @@ class ChapterExportController extends Controller
         $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
         $containedHtml = $this->exportFormatter->chapterToContainedHtml($chapter);
 
-        return $this->downloadResponse($containedHtml, $chapterSlug . '.html');
+        return $this->download()->directly($containedHtml, $chapterSlug . '.html');
     }
 
     /**
@@ -60,7 +60,7 @@ class ChapterExportController extends Controller
         $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
         $chapterText = $this->exportFormatter->chapterToPlainText($chapter);
 
-        return $this->downloadResponse($chapterText, $chapterSlug . '.txt');
+        return $this->download()->directly($chapterText, $chapterSlug . '.txt');
     }
 
     /**
@@ -70,10 +70,9 @@ class ChapterExportController extends Controller
      */
     public function markdown(string $bookSlug, string $chapterSlug)
     {
-        // TODO: This should probably export to a zip file.
         $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
         $chapterText = $this->exportFormatter->chapterToMarkdown($chapter);
 
-        return $this->downloadResponse($chapterText, $chapterSlug . '.md');
+        return $this->download()->directly($chapterText, $chapterSlug . '.md');
     }
 }

app/Http/Controllers/Controller.php

@@ -4,15 +4,13 @@ namespace BookStack\Http\Controllers;
 
 use BookStack\Exceptions\NotifyException;
 use BookStack\Facades\Activity;
+use BookStack\Http\Responses\DownloadResponseFactory;
 use BookStack\Interfaces\Loggable;
 use BookStack\Model;
-use BookStack\Util\WebSafeMimeSniffer;
 use Illuminate\Foundation\Bus\DispatchesJobs;
 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
 {
@@ -110,74 +108,11 @@ abstract class Controller extends BaseController
     }
 
     /**
-     * Create a response that forces a download in the browser.
+     * Create and return a new download response factory using the current request.
      */
-    protected function downloadResponse(string $content, string $fileName): Response
+    protected function download(): DownloadResponseFactory
     {
-        return response()->make($content, 200, [
+        return new DownloadResponseFactory(request());
-            'Content-Type'           => 'application/octet-stream',
-            '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, 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);
-        }, 200, [
-            'Content-Type'           => 'application/octet-stream',
-            'Content-Disposition'    => 'attachment; 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.
-     */
-    protected function inlineDownloadResponse(string $content, string $fileName): Response
-    {
-        $mime = (new WebSafeMimeSniffer())->sniff($content);
-
-        return response()->make($content, 200, [
-            'Content-Type'           => $mime,
-            '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/Http/Controllers/PageExportController.php

@@ -36,7 +36,7 @@ class PageExportController extends Controller
         $page->html = (new PageContent($page))->render();
         $pdfContent = $this->exportFormatter->pageToPdf($page);
 
-        return $this->downloadResponse($pdfContent, $pageSlug . '.pdf');
+        return $this->download()->directly($pdfContent, $pageSlug . '.pdf');
     }
 
     /**
@@ -51,7 +51,7 @@ class PageExportController extends Controller
         $page->html = (new PageContent($page))->render();
         $containedHtml = $this->exportFormatter->pageToContainedHtml($page);
 
-        return $this->downloadResponse($containedHtml, $pageSlug . '.html');
+        return $this->download()->directly($containedHtml, $pageSlug . '.html');
     }
 
     /**
@@ -64,7 +64,7 @@ class PageExportController extends Controller
         $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug);
         $pageText = $this->exportFormatter->pageToPlainText($page);
 
-        return $this->downloadResponse($pageText, $pageSlug . '.txt');
+        return $this->download()->directly($pageText, $pageSlug . '.txt');
     }
 
     /**
@@ -77,6 +77,6 @@ class PageExportController extends Controller
         $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug);
         $pageText = $this->exportFormatter->pageToMarkdown($page);
 
-        return $this->downloadResponse($pageText, $pageSlug . '.md');
+        return $this->download()->directly($pageText, $pageSlug . '.md');
     }
 }

app/Http/Responses/DownloadResponseFactory.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace BookStack\Http\Responses;
+
+use BookStack\Util\WebSafeMimeSniffer;
+use Illuminate\Http\Request;
+use Illuminate\Http\Response;
+use Symfony\Component\HttpFoundation\StreamedResponse;
+
+class DownloadResponseFactory
+{
+    protected Request $request;
+
+    public function __construct(Request $request)
+    {
+        $this->request = $request;
+    }
+
+    /**
+     * Create a response that directly forces a download in the browser.
+     */
+    public function directly(string $content, string $fileName): Response
+    {
+        return response()->make($content, 200, $this->getHeaders($fileName));
+    }
+
+    /**
+     * Create a response that forces a download, from a given stream of content.
+     */
+    public function streamedDirectly($stream, string $fileName): 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);
+        }, 200, $this->getHeaders($fileName));
+    }
+
+    /**
+     * 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.
+     */
+    public function streamedInline($stream, string $fileName): StreamedResponse
+    {
+        $sniffContent = fread($stream, 2000);
+        $mime = (new WebSafeMimeSniffer())->sniff($sniffContent);
+
+        return response()->stream(function () use ($sniffContent, $stream) {
+            echo $sniffContent;
+            fpassthru($stream);
+            fclose($stream);
+        }, 200, $this->getHeaders($fileName, $mime));
+    }
+
+    /**
+     * Get the common headers to provide for a download response.
+     */
+    protected function getHeaders(string $fileName, string $mime = 'application/octet-stream'): array
+    {
+        $disposition = ($mime === 'application/octet-stream') ? 'attachment' : 'inline';
+        $downloadName = str_replace('"', '', $fileName);
+
+        return [
+            'Content-Type'           => $mime,
+            'Content-Disposition'    => "{$disposition}; filename=\"{$downloadName}\"",
+            'X-Content-Type-Options' => 'nosniff',
+        ];
+    }
+}

app/Uploads/AttachmentService.php

@@ -63,16 +63,6 @@ class AttachmentService
         return 'uploads/files/' . $path;
     }
 
-    /**
-     * Get an attachment from storage.
-     *
-     * @throws FileNotFoundException
-     */
-    public function getAttachmentFromStorage(Attachment $attachment): string
-    {
-        return $this->getStorageDisk()->get($this->adjustPathForStorageDisk($attachment->path));
-    }
-
     /**
      * Stream an attachment from storage.
      *

app/Util/WebSafeMimeSniffer.php

@@ -24,6 +24,7 @@ class WebSafeMimeSniffer
         'audio/opus',
         'audio/wav',
         'audio/webm',
+        'audio/x-m4a',
         'image/apng',
         'image/bmp',
         'image/jpeg',