From b4d9029dc301f73f0e87026b8eff78cf2cda6164 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 7 Jan 2024 14:03:13 +0000 Subject: [PATCH 1/5] Range requests: Extracted stream output handling to new class --- app/Http/DownloadResponseFactory.php | 36 ++++-------- app/Http/RangeSupportedStream.php | 57 +++++++++++++++++++ app/Uploads/AttachmentService.php | 10 +++- .../Controllers/AttachmentController.php | 5 +- 4 files changed, 80 insertions(+), 28 deletions(-) create mode 100644 app/Http/RangeSupportedStream.php diff --git a/app/Http/DownloadResponseFactory.php b/app/Http/DownloadResponseFactory.php index 20032f525..f8c10165c 100644 --- a/app/Http/DownloadResponseFactory.php +++ b/app/Http/DownloadResponseFactory.php @@ -9,11 +9,9 @@ use Symfony\Component\HttpFoundation\StreamedResponse; 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. */ - 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)); } @@ -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. */ - 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)); } diff --git a/app/Http/RangeSupportedStream.php b/app/Http/RangeSupportedStream.php new file mode 100644 index 000000000..dc3105035 --- /dev/null +++ b/app/Http/RangeSupportedStream.php @@ -0,0 +1,57 @@ +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 ddabec09f..72f78e347 100644 --- a/app/Uploads/AttachmentService.php +++ b/app/Uploads/AttachmentService.php @@ -66,8 +66,6 @@ class AttachmentService /** * Stream an attachment from storage. * - * @throws FileNotFoundException - * * @return resource|null */ public function streamAttachmentFromStorage(Attachment $attachment) @@ -75,6 +73,14 @@ class AttachmentService 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. * diff --git a/app/Uploads/Controllers/AttachmentController.php b/app/Uploads/Controllers/AttachmentController.php index 92f23465d..e61c10338 100644 --- 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); + $attachmentSize = $this->attachmentService->getAttachmentFileSize($attachment); 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); } /** From d94762549a3ef364d4486a27f1585122f60c10ec Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 7 Jan 2024 20:34:03 +0000 Subject: [PATCH 2/5] Range requests: Added basic HTTP range support --- app/Http/DownloadResponseFactory.php | 28 ++++---- app/Http/RangeSupportedStream.php | 95 +++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 20 deletions(-) diff --git a/app/Http/DownloadResponseFactory.php b/app/Http/DownloadResponseFactory.php index f8c10165c..f29aaa2e4 100644 --- a/app/Http/DownloadResponseFactory.php +++ b/app/Http/DownloadResponseFactory.php @@ -2,7 +2,6 @@ namespace BookStack\Http; -use BookStack\Util\WebSafeMimeSniffer; use Illuminate\Http\Request; use Illuminate\Http\Response; use Symfony\Component\HttpFoundation\StreamedResponse; @@ -19,7 +18,7 @@ class DownloadResponseFactory */ public function directly(string $content, string $fileName): Response { - return response()->make($content, 200, $this->getHeaders($fileName)); + return response()->make($content, 200, $this->getHeaders($fileName, strlen($content))); } /** @@ -27,10 +26,13 @@ class DownloadResponseFactory */ public function streamedDirectly($stream, string $fileName, int $fileSize): StreamedResponse { - $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request->headers); - return response()->stream(function () use ($rangeStream) { - $rangeStream->outputAndClose(); - }, 200, $this->getHeaders($fileName)); + $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request); + $headers = array_merge($this->getHeaders($fileName, $fileSize), $rangeStream->getResponseHeaders()); + return response()->stream( + fn() => $rangeStream->outputAndClose(), + $rangeStream->getResponseStatus(), + $headers, + ); } /** @@ -40,24 +42,28 @@ class DownloadResponseFactory */ public function streamedInline($stream, string $fileName, int $fileSize): StreamedResponse { - $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request->headers); + $rangeStream = new RangeSupportedStream($stream, $fileSize, $this->request); $mime = $rangeStream->sniffMime(); + $headers = array_merge($this->getHeaders($fileName, $fileSize, $mime), $rangeStream->getResponseHeaders()); - return response()->stream(function () use ($rangeStream) { - $rangeStream->outputAndClose(); - }, 200, $this->getHeaders($fileName, $mime)); + return response()->stream( + fn() => $rangeStream->outputAndClose(), + $rangeStream->getResponseStatus(), + $headers, + ); } /** * Get the common headers to provide for a download response. */ - protected function getHeaders(string $fileName, string $mime = 'application/octet-stream'): array + protected function getHeaders(string $fileName, int $fileSize, string $mime = 'application/octet-stream'): array { $disposition = ($mime === 'application/octet-stream') ? 'attachment' : 'inline'; $downloadName = str_replace('"', '', $fileName); return [ 'Content-Type' => $mime, + 'Content-Length' => $fileSize, 'Content-Disposition' => "{$disposition}; filename=\"{$downloadName}\"", 'X-Content-Type-Options' => 'nosniff', ]; diff --git a/app/Http/RangeSupportedStream.php b/app/Http/RangeSupportedStream.php index dc3105035..b51d4a549 100644 --- a/app/Http/RangeSupportedStream.php +++ b/app/Http/RangeSupportedStream.php @@ -3,17 +3,30 @@ namespace BookStack\Http; use BookStack\Util\WebSafeMimeSniffer; -use Symfony\Component\HttpFoundation\HeaderBag; +use Illuminate\Http\Request; +/** + * Helper wrapper for range-based stream response handling. + * Much of this used symfony/http-foundation as a reference during build. + * URL: https://github.com/symfony/http-foundation/blob/v6.0.20/BinaryFileResponse.php + * License: MIT license, Copyright (c) Fabien Potencier. + */ class RangeSupportedStream { protected string $sniffContent; + protected array $responseHeaders; + protected int $responseStatus = 200; + + protected int $responseLength = 0; + protected int $responseOffset = 0; public function __construct( protected $stream, protected int $fileSize, - protected HeaderBag $requestHeaders, + Request $request, ) { + $this->responseLength = $this->fileSize; + $this->parseRequest($request); } /** @@ -40,18 +53,82 @@ class RangeSupportedStream } $outStream = fopen('php://output', 'w'); - $offset = 0; + $sniffOffset = strlen($this->sniffContent); - if (!empty($this->sniffContent)) { - fwrite($outStream, $this->sniffContent); - $offset = strlen($this->sniffContent); + if (!empty($this->sniffContent) && $this->responseOffset < $sniffOffset) { + $sniffOutput = substr($this->sniffContent, $this->responseOffset, min($sniffOffset, $this->responseLength)); + fwrite($outStream, $sniffOutput); + } else if ($this->responseOffset !== 0) { + fseek($this->stream, $this->responseOffset); } - $toWrite = $this->fileSize - $offset; - stream_copy_to_stream($this->stream, $outStream, $toWrite); - fpassthru($this->stream); + stream_copy_to_stream($this->stream, $outStream, $this->responseLength); fclose($this->stream); fclose($outStream); } + + public function getResponseHeaders(): array + { + return $this->responseHeaders; + } + + public function getResponseStatus(): int + { + return $this->responseStatus; + } + + protected function parseRequest(Request $request): void + { + $this->responseHeaders['Accept-Ranges'] = $request->isMethodSafe() ? 'bytes' : 'none'; + + $range = $this->getRangeFromRequest($request); + if ($range) { + [$start, $end] = $range; + if ($start < 0 || $start > $end) { + $this->responseStatus = 416; + $this->responseHeaders['Content-Range'] = sprintf('bytes */%s', $this->fileSize); + } elseif ($end - $start < $this->fileSize - 1) { + $this->responseLength = $end < $this->fileSize ? $end - $start + 1 : -1; + $this->responseOffset = $start; + $this->responseStatus = 206; + $this->responseHeaders['Content-Range'] = sprintf('bytes %s-%s/%s', $start, $end, $this->fileSize); + $this->responseHeaders['Content-Length'] = $end - $start + 1; + } + } + + if ($request->isMethod('HEAD')) { + $this->responseLength = 0; + } + } + + protected function getRangeFromRequest(Request $request): ?array + { + $range = $request->headers->get('Range'); + if (!$range || !$request->isMethod('GET') || !str_starts_with($range, 'bytes=')) { + return null; + } + + if ($request->headers->has('If-Range')) { + return null; + } + + [$start, $end] = explode('-', substr($range, 6), 2) + [0]; + + $end = ('' === $end) ? $this->fileSize - 1 : (int) $end; + + if ('' === $start) { + $start = $this->fileSize - $end; + $end = $this->fileSize - 1; + } else { + $start = (int) $start; + } + + if ($start > $end) { + return null; + } + + $end = min($end, $this->fileSize - 1); + return [$start, $end]; + } } From 91d8d6eaaa5ae42fc9872901d6fcbcae8e19236e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 14 Jan 2024 15:50:00 +0000 Subject: [PATCH 3/5] Range requests: Added test cases to cover functionality Fixed some found issues in the process. --- app/Http/RangeSupportedStream.php | 20 +++--- tests/Uploads/AttachmentTest.php | 101 ++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/app/Http/RangeSupportedStream.php b/app/Http/RangeSupportedStream.php index b51d4a549..300f4488c 100644 --- a/app/Http/RangeSupportedStream.php +++ b/app/Http/RangeSupportedStream.php @@ -13,8 +13,8 @@ use Illuminate\Http\Request; */ class RangeSupportedStream { - protected string $sniffContent; - protected array $responseHeaders; + protected string $sniffContent = ''; + protected array $responseHeaders = []; protected int $responseStatus = 200; protected int $responseLength = 0; @@ -53,16 +53,20 @@ class RangeSupportedStream } $outStream = fopen('php://output', 'w'); - $sniffOffset = strlen($this->sniffContent); + $sniffLength = strlen($this->sniffContent); + $bytesToWrite = $this->responseLength; - if (!empty($this->sniffContent) && $this->responseOffset < $sniffOffset) { - $sniffOutput = substr($this->sniffContent, $this->responseOffset, min($sniffOffset, $this->responseLength)); + if ($sniffLength > 0 && $this->responseOffset < $sniffLength) { + $sniffEnd = min($sniffLength, $bytesToWrite + $this->responseOffset); + $sniffOutLength = $sniffEnd - $this->responseOffset; + $sniffOutput = substr($this->sniffContent, $this->responseOffset, $sniffOutLength); fwrite($outStream, $sniffOutput); + $bytesToWrite -= $sniffOutLength; } else if ($this->responseOffset !== 0) { fseek($this->stream, $this->responseOffset); } - stream_copy_to_stream($this->stream, $outStream, $this->responseLength); + stream_copy_to_stream($this->stream, $outStream, $bytesToWrite); fclose($this->stream); fclose($outStream); @@ -124,10 +128,6 @@ class RangeSupportedStream $start = (int) $start; } - if ($start > $end) { - return null; - } - $end = min($end, $this->fileSize - 1); return [$start, $end]; } diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index bd03c339c..2e1a7b339 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -316,4 +316,105 @@ class AttachmentTest extends TestCase $this->assertFileExists(storage_path($attachment->path)); $this->files->deleteAllAttachmentFiles(); } + + public function test_file_get_range_access() + { + $page = $this->entities->page(); + $this->asAdmin(); + $attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'my_text.txt', 'abc123456', 'text/plain'); + + // Download access + $resp = $this->get($attachment->getUrl(), ['Range' => 'bytes=3-5']); + $resp->assertStatus(206); + $resp->assertStreamedContent('123'); + $resp->assertHeader('Content-Length', '3'); + $resp->assertHeader('Content-Range', 'bytes 3-5/9'); + + // Inline access + $resp = $this->get($attachment->getUrl(true), ['Range' => 'bytes=5-7']); + $resp->assertStatus(206); + $resp->assertStreamedContent('345'); + $resp->assertHeader('Content-Length', '3'); + $resp->assertHeader('Content-Range', 'bytes 5-7/9'); + + $this->files->deleteAllAttachmentFiles(); + } + + public function test_file_head_range_returns_no_content() + { + $page = $this->entities->page(); + $this->asAdmin(); + $attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'my_text.txt', 'abc123456', 'text/plain'); + + $resp = $this->head($attachment->getUrl(), ['Range' => 'bytes=0-9']); + $resp->assertStreamedContent(''); + $resp->assertHeader('Content-Length', '9'); + $resp->assertStatus(200); + + $this->files->deleteAllAttachmentFiles(); + } + + public function test_file_head_range_edge_cases() + { + $page = $this->entities->page(); + $this->asAdmin(); + + // Mime-type "sniffing" happens on first 2k bytes, hence this content (2005 bytes) + $content = '01234' . str_repeat('a', 1990) . '0123456789'; + $attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'my_text.txt', $content, 'text/plain'); + + // Test for both inline and download attachment serving + foreach ([true, false] as $isInline) { + // No end range + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=5-']); + $resp->assertStreamedContent(substr($content, 5)); + $resp->assertHeader('Content-Length', '2000'); + $resp->assertHeader('Content-Range', 'bytes 5-2004/2005'); + $resp->assertStatus(206); + + // End only range + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=-10']); + $resp->assertStreamedContent('0123456789'); + $resp->assertHeader('Content-Length', '10'); + $resp->assertHeader('Content-Range', 'bytes 1995-2004/2005'); + $resp->assertStatus(206); + + // Range across sniff point + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=1997-2002']); + $resp->assertStreamedContent('234567'); + $resp->assertHeader('Content-Length', '6'); + $resp->assertHeader('Content-Range', 'bytes 1997-2002/2005'); + $resp->assertStatus(206); + + // Range up to sniff point + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=0-1997']); + $resp->assertHeader('Content-Length', '1998'); + $resp->assertHeader('Content-Range', 'bytes 0-1997/2005'); + $resp->assertStreamedContent(substr($content, 0, 1998)); + $resp->assertStatus(206); + + // Range beyond sniff point + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=2001-2003']); + $resp->assertStreamedContent('678'); + $resp->assertHeader('Content-Length', '3'); + $resp->assertHeader('Content-Range', 'bytes 2001-2003/2005'); + $resp->assertStatus(206); + + // Range beyond content + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=0-2010']); + $resp->assertStreamedContent($content); + $resp->assertHeader('Content-Length', '2005'); + $resp->assertHeaderMissing('Content-Range'); + $resp->assertStatus(200); + + // Range start before end + $resp = $this->get($attachment->getUrl($isInline), ['Range' => 'bytes=50-10']); + $resp->assertStreamedContent($content); + $resp->assertHeader('Content-Length', '2005'); + $resp->assertHeader('Content-Range', 'bytes */2005'); + $resp->assertStatus(416); + } + + $this->files->deleteAllAttachmentFiles(); + } } From c1552fb799cda7fbb6de27ebcb01aa2580fd5330 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 15 Jan 2024 11:50:05 +0000 Subject: [PATCH 4/5] Attachments: Drag and drop video support Supports dragging and dropping video attahchments to embed them in the editor as HTML video tags. --- app/Uploads/Attachment.php | 19 +++++++++++++++++-- .../views/attachments/manager-list.blade.php | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index 4fd6d4cdc..57d7cb334 100644 --- a/app/Uploads/Attachment.php +++ b/app/Uploads/Attachment.php @@ -77,7 +77,22 @@ class Attachment extends Model } /** - * Generate a HTML link to this attachment. + * Get the representation of this attachment in a format suitable for the page editors. + * Detects and adapts video content to use an inline video embed. + */ + public function editorContent(): array + { + $videoExtensions = ['mp4', 'webm', 'mkv', 'ogg', 'avi']; + if (in_array(strtolower($this->extension), $videoExtensions)) { + $html = ''; + return ['text/html' => $html, 'text/plain' => $html]; + } + + return ['text/html' => $this->htmlLink(), 'text/plain' => $this->markdownLink()]; + } + + /** + * Generate the HTML link to this attachment. */ public function htmlLink(): string { @@ -85,7 +100,7 @@ class Attachment extends Model } /** - * Generate a markdown link to this attachment. + * Generate a MarkDown link to this attachment. */ public function markdownLink(): string { diff --git a/resources/views/attachments/manager-list.blade.php b/resources/views/attachments/manager-list.blade.php index 342b46dca..0e841a042 100644 --- a/resources/views/attachments/manager-list.blade.php +++ b/resources/views/attachments/manager-list.blade.php @@ -4,7 +4,7 @@
@icon('grip')
From 2dc454d206b518804aecd0551a71d338cf889c08 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 15 Jan 2024 13:36:04 +0000 Subject: [PATCH 5/5] Uploads: Explicitly disabled s3 streaming in config This was the default option anyway, just adding here for better visibility of this being set. Can't enable without issues as the app will attempt to seek which does not work for these streams. Also have not tested on non-s3, s3-like systems. --- app/Config/filesystems.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Config/filesystems.php b/app/Config/filesystems.php index e6ae0fed3..1319c8886 100644 --- a/app/Config/filesystems.php +++ b/app/Config/filesystems.php @@ -58,6 +58,7 @@ return [ 'endpoint' => env('STORAGE_S3_ENDPOINT', null), 'use_path_style_endpoint' => env('STORAGE_S3_ENDPOINT', null) !== null, 'throw' => true, + 'stream_reads' => false, ], ],