From abc283fc64b7ed3e79f5e499f934130b6146a02b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 8 Jun 2022 23:50:42 +0100 Subject: [PATCH] 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. --- .../Api/BookExportApiController.php | 8 +- .../Api/ChapterExportApiController.php | 8 +- .../Api/PageExportApiController.php | 8 +- app/Http/Controllers/AttachmentController.php | 4 +- app/Http/Controllers/BookExportController.php | 8 +- .../Controllers/ChapterExportController.php | 9 +-- app/Http/Controllers/Controller.php | 73 +----------------- app/Http/Controllers/PageExportController.php | 8 +- .../Responses/DownloadResponseFactory.php | 77 +++++++++++++++++++ app/Uploads/AttachmentService.php | 10 --- app/Util/WebSafeMimeSniffer.php | 1 + 11 files changed, 108 insertions(+), 106 deletions(-) create mode 100644 app/Http/Responses/DownloadResponseFactory.php diff --git a/app/Http/Controllers/Api/BookExportApiController.php b/app/Http/Controllers/Api/BookExportApiController.php index 028bc3a81..84090befb 100644 --- a/app/Http/Controllers/Api/BookExportApiController.php +++ b/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'); } } diff --git a/app/Http/Controllers/Api/ChapterExportApiController.php b/app/Http/Controllers/Api/ChapterExportApiController.php index 5715ab2e3..faf5d812e 100644 --- a/app/Http/Controllers/Api/ChapterExportApiController.php +++ b/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'); } } diff --git a/app/Http/Controllers/Api/PageExportApiController.php b/app/Http/Controllers/Api/PageExportApiController.php index ce5700c79..f2edffba3 100644 --- a/app/Http/Controllers/Api/PageExportApiController.php +++ b/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'); } } diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php index 0a092b63a..03e362f4a 100644 --- a/app/Http/Controllers/AttachmentController.php +++ b/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); } /** diff --git a/app/Http/Controllers/BookExportController.php b/app/Http/Controllers/BookExportController.php index 7f6dd8017..cc8d48a35 100644 --- a/app/Http/Controllers/BookExportController.php +++ b/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'); } } diff --git a/app/Http/Controllers/ChapterExportController.php b/app/Http/Controllers/ChapterExportController.php index 480280c99..fd56d91b3 100644 --- a/app/Http/Controllers/ChapterExportController.php +++ b/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'); } } diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 5b2221fc1..f6dc1dbca 100644 --- a/app/Http/Controllers/Controller.php +++ b/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, [ - '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', - ]); + return new DownloadResponseFactory(request()); } /** diff --git a/app/Http/Controllers/PageExportController.php b/app/Http/Controllers/PageExportController.php index 0287916de..62101d339 100644 --- a/app/Http/Controllers/PageExportController.php +++ b/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'); } } diff --git a/app/Http/Responses/DownloadResponseFactory.php b/app/Http/Responses/DownloadResponseFactory.php new file mode 100644 index 000000000..cdf8e311f --- /dev/null +++ b/app/Http/Responses/DownloadResponseFactory.php @@ -0,0 +1,77 @@ +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', + ]; + } +} \ No newline at end of file diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index 9d1f96ae4..6a92cb5a5 100644 --- a/app/Uploads/AttachmentService.php +++ b/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. * diff --git a/app/Util/WebSafeMimeSniffer.php b/app/Util/WebSafeMimeSniffer.php index 6861add72..b182d8ac1 100644 --- a/app/Util/WebSafeMimeSniffer.php +++ b/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',