From 59d1fb2d1033616ac7edc143f4876485bb997c6a Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 3 Apr 2022 16:22:31 +0100
Subject: [PATCH] Fixed tests from streaming changes

- Added testing check to buffer stop/clear on streaming output due to
  interference during tests.
- Made content-disposition header a little safer in download responses.
- Also aligned how we check for testing environment.
---
 app/Http/Controllers/Controller.php            | 15 ++++++++++-----
 resources/views/common/export-styles.blade.php |  2 +-
 tests/Api/AttachmentsApiTest.php               |  7 +++++--
 tests/Uploads/AttachmentTest.php               |  3 ++-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php
index 4c979c26a..6ca2239cc 100644
--- a/app/Http/Controllers/Controller.php
+++ b/app/Http/Controllers/Controller.php
@@ -116,7 +116,7 @@ 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',
         ]);
     }
@@ -127,12 +127,17 @@ abstract class Controller extends BaseController
     protected function streamedDownloadResponse($stream, string $fileName): StreamedResponse
     {
         return response()->stream(function() use ($stream) {
-            ob_end_clean();
+            // 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="' . $fileName . '"',
+            'Content-Disposition'    => 'attachment; filename="' . str_replace('"', '', $fileName) . '"',
             'X-Content-Type-Options' => 'nosniff',
         ]);
     }
@@ -147,7 +152,7 @@ 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',
         ]);
     }
@@ -168,7 +173,7 @@ abstract class Controller extends BaseController
            fclose($stream);
         }, 200, [
             'Content-Type'           => $mime,
-            'Content-Disposition'    => 'inline; filename="' . $fileName . '"',
+            'Content-Disposition'    => 'inline; filename="' . str_replace('"', '', $fileName) . '"',
             'X-Content-Type-Options' => 'nosniff',
         ]);
     }
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();
     }