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.
This commit is contained in:
Dan Brown 2022-04-03 16:22:31 +01:00
parent 08a8c0070e
commit 59d1fb2d10
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
4 changed files with 18 additions and 9 deletions

View File

@ -116,7 +116,7 @@ abstract class Controller extends BaseController
{ {
return response()->make($content, 200, [ return response()->make($content, 200, [
'Content-Type' => 'application/octet-stream', 'Content-Type' => 'application/octet-stream',
'Content-Disposition' => 'attachment; filename="' . $fileName . '"', 'Content-Disposition' => 'attachment; filename="' . str_replace('"', '', $fileName) . '"',
'X-Content-Type-Options' => 'nosniff', 'X-Content-Type-Options' => 'nosniff',
]); ]);
} }
@ -127,12 +127,17 @@ abstract class Controller extends BaseController
protected function streamedDownloadResponse($stream, string $fileName): StreamedResponse protected function streamedDownloadResponse($stream, string $fileName): StreamedResponse
{ {
return response()->stream(function() use ($stream) { 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); fpassthru($stream);
fclose($stream); fclose($stream);
}, 200, [ }, 200, [
'Content-Type' => 'application/octet-stream', 'Content-Type' => 'application/octet-stream',
'Content-Disposition' => 'attachment; filename="' . $fileName . '"', 'Content-Disposition' => 'attachment; filename="' . str_replace('"', '', $fileName) . '"',
'X-Content-Type-Options' => 'nosniff', 'X-Content-Type-Options' => 'nosniff',
]); ]);
} }
@ -147,7 +152,7 @@ abstract class Controller extends BaseController
return response()->make($content, 200, [ return response()->make($content, 200, [
'Content-Type' => $mime, 'Content-Type' => $mime,
'Content-Disposition' => 'inline; filename="' . $fileName . '"', 'Content-Disposition' => 'inline; filename="' . str_replace('"', '', $fileName) . '"',
'X-Content-Type-Options' => 'nosniff', 'X-Content-Type-Options' => 'nosniff',
]); ]);
} }
@ -168,7 +173,7 @@ abstract class Controller extends BaseController
fclose($stream); fclose($stream);
}, 200, [ }, 200, [
'Content-Type' => $mime, 'Content-Type' => $mime,
'Content-Disposition' => 'inline; filename="' . $fileName . '"', 'Content-Disposition' => 'inline; filename="' . str_replace('"', '', $fileName) . '"',
'X-Content-Type-Options' => 'nosniff', 'X-Content-Type-Options' => 'nosniff',
]); ]);
} }

View File

@ -1,5 +1,5 @@
<style> <style>
@if (!app()->environment('testing')) @if (!app()->runningUnitTests())
{!! file_get_contents(public_path('/dist/export-styles.css')) !!} {!! file_get_contents(public_path('/dist/export-styles.css')) !!}
@endif @endif
</style> </style>

View File

@ -5,6 +5,7 @@ namespace Tests\Api;
use BookStack\Entities\Models\Page; use BookStack\Entities\Models\Page;
use BookStack\Uploads\Attachment; use BookStack\Uploads\Attachment;
use Illuminate\Http\UploadedFile; use Illuminate\Http\UploadedFile;
use Illuminate\Testing\AssertableJsonString;
use Tests\TestCase; use Tests\TestCase;
class AttachmentsApiTest extends TestCase class AttachmentsApiTest extends TestCase
@ -228,9 +229,11 @@ class AttachmentsApiTest extends TestCase
$attachment = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->firstOrFail(); $attachment = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->firstOrFail();
$resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}"); $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
$resp->assertStatus(200); $resp->assertStatus(200);
$resp->assertJson([ $resp->assertHeader('Content-Type', 'application/json');
$json = new AssertableJsonString($resp->streamedContent());
$json->assertSubset([
'id' => $attachment->id, 'id' => $attachment->id,
'content' => base64_encode(file_get_contents(storage_path($attachment->path))), 'content' => base64_encode(file_get_contents(storage_path($attachment->path))),
'external' => false, 'external' => false,

View File

@ -128,7 +128,8 @@ class AttachmentTest extends TestCase
$pageGet->assertSee($attachment->getUrl()); $pageGet->assertSee($attachment->getUrl());
$attachmentGet = $this->get($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(); $this->deleteUploads();
} }