diff --git a/app/Http/Controllers/Api/AttachmentApiController.php b/app/Http/Controllers/Api/AttachmentApiController.php
index fc5008e3c..2476cb951 100644
--- a/app/Http/Controllers/Api/AttachmentApiController.php
+++ b/app/Http/Controllers/Api/AttachmentApiController.php
@@ -87,14 +87,32 @@ class AttachmentApiController extends ApiController
'markdown' => $attachment->markdownLink(),
]);
- if (!$attachment->external) {
- $attachmentContents = $this->attachmentService->getAttachmentFromStorage($attachment);
- $attachment->setAttribute('content', base64_encode($attachmentContents));
- } else {
+ // Simply return a JSON response of the attachment for link-based attachments
+ if ($attachment->external) {
$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']);
}
/**
diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php
index 084f6f96a..0a092b63a 100644
--- a/app/Http/Controllers/AttachmentController.php
+++ b/app/Http/Controllers/AttachmentController.php
@@ -15,8 +15,8 @@ use Illuminate\Validation\ValidationException;
class AttachmentController extends Controller
{
- protected $attachmentService;
- protected $pageRepo;
+ protected AttachmentService $attachmentService;
+ 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);
}
/**
diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php
index d616974c6..6ca2239cc 100644
--- a/app/Http/Controllers/Controller.php
+++ b/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',
]);
}
diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index 7974d7ae9..ec02182bb 100644
--- a/app/Uploads/AttachmentService.php
+++ b/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());
diff --git a/resources/views/common/export-styles.blade.php b/resources/views/common/export-styles.blade.php
index ee10637dd..1dfa6bb45 100644
--- a/resources/views/common/export-styles.blade.php
+++ b/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>
diff --git a/tests/Api/AttachmentsApiTest.php b/tests/Api/AttachmentsApiTest.php
index d7625c938..a34337016 100644
--- a/tests/Api/AttachmentsApiTest.php
+++ b/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,
diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php
index 5545edf13..27a23bcae 100644
--- a/tests/Uploads/AttachmentTest.php
+++ b/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();
}