diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php index 04e89ac5d..74eae641b 100644 --- a/app/Http/Controllers/AttachmentController.php +++ b/app/Http/Controllers/AttachmentController.php @@ -14,16 +14,14 @@ use Illuminate\Validation\ValidationException; class AttachmentController extends Controller { protected $attachmentService; - protected $attachment; protected $pageRepo; /** * AttachmentController constructor. */ - public function __construct(AttachmentService $attachmentService, Attachment $attachment, PageRepo $pageRepo) + public function __construct(AttachmentService $attachmentService, PageRepo $pageRepo) { $this->attachmentService = $attachmentService; - $this->attachment = $attachment; $this->pageRepo = $pageRepo; } @@ -67,7 +65,7 @@ class AttachmentController extends Controller 'file' => 'required|file' ]); - $attachment = $this->attachment->newQuery()->findOrFail($attachmentId); + $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('view', $attachment->page); $this->checkOwnablePermission('page-update', $attachment->page); $this->checkOwnablePermission('attachment-create', $attachment); @@ -89,7 +87,7 @@ class AttachmentController extends Controller */ public function getUpdateForm(string $attachmentId) { - $attachment = $this->attachment->findOrFail($attachmentId); + $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('page-update', $attachment->page); $this->checkOwnablePermission('attachment-create', $attachment); @@ -202,9 +200,10 @@ class AttachmentController extends Controller * @throws FileNotFoundException * @throws NotFoundException */ - public function get(string $attachmentId) + public function get(Request $request, string $attachmentId) { - $attachment = $this->attachment->findOrFail($attachmentId); + /** @var Attachment $attachment */ + $attachment = Attachment::query()->findOrFail($attachmentId); try { $page = $this->pageRepo->getById($attachment->uploaded_to); } catch (NotFoundException $exception) { @@ -217,8 +216,13 @@ class AttachmentController extends Controller return redirect($attachment->path); } + $fileName = $attachment->getFileName(); $attachmentContents = $this->attachmentService->getAttachmentFromStorage($attachment); - return $this->downloadResponse($attachmentContents, $attachment->getFileName()); + + if ($request->get('open') === 'true') { + return $this->inlineDownloadResponse($attachmentContents, $fileName); + } + return $this->downloadResponse($attachmentContents, $fileName); } /** @@ -227,7 +231,7 @@ class AttachmentController extends Controller */ public function delete(string $attachmentId) { - $attachment = $this->attachment->findOrFail($attachmentId); + $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('attachment-delete', $attachment); $this->attachmentService->deleteFile($attachment); return response()->json(['message' => trans('entities.attachments_deleted')]); diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 034dfa524..47b03b28d 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -6,6 +6,7 @@ use BookStack\Facades\Activity; use BookStack\Interfaces\Loggable; use BookStack\HasCreatorAndUpdater; use BookStack\Model; +use finfo; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; use Illuminate\Http\Exceptions\HttpResponseException; @@ -121,6 +122,20 @@ abstract class Controller extends BaseController ]); } + /** + * 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 + { + $finfo = new finfo(FILEINFO_MIME_TYPE); + $mime = $finfo->buffer($content) ?: 'application/octet-stream'; + return response()->make($content, 200, [ + 'Content-Type' => $mime, + 'Content-Disposition' => 'inline; filename="' . $fileName . '"' + ]); + } + /** * Show a positive, successful notification to the user on next view load. */ diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index d1060477d..474d68998 100644 --- a/app/Uploads/Attachment.php +++ b/app/Uploads/Attachment.php @@ -41,12 +41,12 @@ class Attachment extends Model /** * Get the url of this file. */ - public function getUrl(): string + public function getUrl($openInline = false): string { if ($this->external && strpos($this->path, 'http') !== 0) { return $this->path; } - return url('/attachments/' . $this->id); + return url('/attachments/' . $this->id . ($openInline ? '?open=true' : '')); } /** diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index 4437897c7..37adb4f83 100644 --- a/app/Uploads/AttachmentService.php +++ b/app/Uploads/AttachmentService.php @@ -3,8 +3,10 @@ use BookStack\Exceptions\FileUploadException; use Exception; use Illuminate\Contracts\Filesystem\Factory as FileSystem; +use Illuminate\Contracts\Filesystem\FileNotFoundException; use Illuminate\Contracts\Filesystem\Filesystem as FileSystemInstance; use Illuminate\Support\Str; +use Log; use Symfony\Component\HttpFoundation\File\UploadedFile; class AttachmentService @@ -38,11 +40,9 @@ class AttachmentService /** * Get an attachment from storage. - * @param Attachment $attachment - * @return string - * @throws \Illuminate\Contracts\Filesystem\FileNotFoundException + * @throws FileNotFoundException */ - public function getAttachmentFromStorage(Attachment $attachment) + public function getAttachmentFromStorage(Attachment $attachment): string { return $this->getStorage()->get($attachment->path); } @@ -202,7 +202,7 @@ class AttachmentService try { $storage->put($attachmentPath, $attachmentData); } catch (Exception $e) { - \Log::error('Error when attempting file upload:' . $e->getMessage()); + Log::error('Error when attempting file upload:' . $e->getMessage()); throw new FileUploadException(trans('errors.path_not_writable', ['filePath' => $attachmentPath])); } diff --git a/composer.json b/composer.json index 3e604b8fd..8450a2f92 100644 --- a/composer.json +++ b/composer.json @@ -8,6 +8,7 @@ "php": "^7.3|^8.0", "ext-curl": "*", "ext-dom": "*", + "ext-fileinfo": "*", "ext-gd": "*", "ext-json": "*", "ext-mbstring": "*", diff --git a/resources/js/components/attachments-list.js b/resources/js/components/attachments-list.js new file mode 100644 index 000000000..34979c2e7 --- /dev/null +++ b/resources/js/components/attachments-list.js @@ -0,0 +1,47 @@ +/** + * Attachments List + * Adds '?open=true' query to file attachment links + * when ctrl/cmd is pressed down. + * @extends {Component} + */ +class AttachmentsList { + + setup() { + this.container = this.$el; + this.setupListeners(); + } + + setupListeners() { + const isExpectedKey = (event) => event.key === 'Control' || event.key === 'Meta'; + window.addEventListener('keydown', event => { + if (isExpectedKey(event)) { + this.addOpenQueryToLinks(); + } + }, {passive: true}); + window.addEventListener('keyup', event => { + if (isExpectedKey(event)) { + this.removeOpenQueryFromLinks(); + } + }, {passive: true}); + } + + addOpenQueryToLinks() { + const links = this.container.querySelectorAll('a.attachment-file'); + for (const link of links) { + if (link.href.split('?')[1] !== 'open=true') { + link.href = link.href + '?open=true'; + link.setAttribute('target', '_blank'); + } + } + } + + removeOpenQueryFromLinks() { + const links = this.container.querySelectorAll('a.attachment-file'); + for (const link of links) { + link.href = link.href.split('?')[0]; + link.removeAttribute('target'); + } + } +} + +export default AttachmentsList; \ No newline at end of file diff --git a/resources/js/components/index.js b/resources/js/components/index.js index 91ccdaf3a..010ee04ba 100644 --- a/resources/js/components/index.js +++ b/resources/js/components/index.js @@ -2,6 +2,7 @@ import addRemoveRows from "./add-remove-rows.js" import ajaxDeleteRow from "./ajax-delete-row.js" import ajaxForm from "./ajax-form.js" import attachments from "./attachments.js" +import attachmentsList from "./attachments-list.js" import autoSuggest from "./auto-suggest.js" import backToTop from "./back-to-top.js" import bookSort from "./book-sort.js" @@ -56,6 +57,7 @@ const componentMapping = { "ajax-delete-row": ajaxDeleteRow, "ajax-form": ajaxForm, "attachments": attachments, + "attachments-list": attachmentsList, "auto-suggest": autoSuggest, "back-to-top": backToTop, "book-sort": bookSort, diff --git a/resources/views/attachments/list.blade.php b/resources/views/attachments/list.blade.php index 8c9be8290..f0a1354ea 100644 --- a/resources/views/attachments/list.blade.php +++ b/resources/views/attachments/list.blade.php @@ -1,8 +1,10 @@ -@foreach($attachments as $attachment) -
- external) target="_blank" @endif> - @icon($attachment->external ? 'export' : 'file') - {{ $attachment->name }} - -
-@endforeach \ No newline at end of file +
+ @foreach($attachments as $attachment) +
+ external) target="_blank" @endif> + @icon($attachment->external ? 'export' : 'file') + {{ $attachment->name }} + +
+ @endforeach +
\ No newline at end of file diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 1ca9ea23b..55a5aa84f 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -4,11 +4,9 @@ use BookStack\Entities\Tools\TrashCan; use BookStack\Entities\Repos\PageRepo; use BookStack\Uploads\Attachment; use BookStack\Entities\Models\Page; -use BookStack\Auth\Permissions\PermissionService; use BookStack\Uploads\AttachmentService; use Illuminate\Http\UploadedFile; use Tests\TestCase; -use Tests\TestResponse; class AttachmentTest extends TestCase { @@ -57,7 +55,7 @@ class AttachmentTest extends TestCase public function test_file_upload() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $admin = $this->getAdmin(); $fileName = 'upload_test_file.txt'; @@ -85,7 +83,7 @@ class AttachmentTest extends TestCase public function test_file_upload_does_not_use_filename() { - $page = Page::first(); + $page = Page::query()->first(); $fileName = 'upload_test_file.txt'; @@ -99,7 +97,7 @@ class AttachmentTest extends TestCase public function test_file_display_and_access() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $fileName = 'upload_test_file.txt'; @@ -119,7 +117,7 @@ class AttachmentTest extends TestCase public function test_attaching_link_to_page() { - $page = Page::first(); + $page = Page::query()->first(); $admin = $this->getAdmin(); $this->asAdmin(); @@ -156,7 +154,7 @@ class AttachmentTest extends TestCase public function test_attachment_updating() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $attachment = $this->createAttachment($page); @@ -180,7 +178,7 @@ class AttachmentTest extends TestCase public function test_file_deletion() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $fileName = 'deletion_test.txt'; $this->uploadFile($fileName, $page->id); @@ -202,7 +200,7 @@ class AttachmentTest extends TestCase public function test_attachment_deletion_on_page_deletion() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $fileName = 'deletion_test.txt'; $this->uploadFile($fileName, $page->id); @@ -230,7 +228,7 @@ class AttachmentTest extends TestCase { $admin = $this->getAdmin(); $viewer = $this->getViewer(); - $page = Page::first(); /** @var Page $page */ + $page = Page::query()->first(); /** @var Page $page */ $this->actingAs($admin); $fileName = 'permission_test.txt'; @@ -253,7 +251,7 @@ class AttachmentTest extends TestCase public function test_data_and_js_links_cannot_be_attached_to_a_page() { - $page = Page::first(); + $page = Page::query()->first(); $this->asAdmin(); $badLinks = [ @@ -291,4 +289,22 @@ class AttachmentTest extends TestCase ]); } } + + public function test_file_access_with_open_query_param_provides_inline_response_with_correct_content_type() + { + $page = Page::query()->first(); + $this->asAdmin(); + $fileName = 'upload_test_file.txt'; + + $upload = $this->uploadFile($fileName, $page->id); + $upload->assertStatus(200); + $attachment = Attachment::query()->orderBy('id', 'desc')->take(1)->first(); + + $attachmentGet = $this->get($attachment->getUrl(true)); + // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. + $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8'); + $attachmentGet->assertHeader('Content-Disposition', "inline; filename=\"upload_test_file.txt\""); + + $this->deleteUploads(); + } }