Skip to content

Commit 8744eb2

Browse files
ssddanbrownssddanbrown
committed
Merge branch 'v23-10' into release
2 parents d8383cf + 15d7161 commit 8744eb2

File tree

9 files changed

+116
-42
lines changed

9 files changed

+116
-42
lines changed

‎app/Config/app.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@
141141
// Third party service providers
142142
Barryvdh\DomPDF\ServiceProvider::class,
143143
Barryvdh\Snappy\ServiceProvider::class,
144-
Intervention\Image\ImageServiceProvider::class,
145144
SocialiteProviders\Manager\ServiceProvider::class,
146145

147146
// BookStack custom service providers
@@ -161,9 +160,6 @@
161160
// Laravel Packages
162161
'Socialite' => Laravel\Socialite\Facades\Socialite::class,
163162

164-
// Third Party
165-
'ImageTool' => Intervention\Image\Facades\Image::class,
166-
167163
// Custom BookStack
168164
'Activity' => BookStack\Facades\Activity::class,
169165
'Theme' => BookStack\Facades\Theme::class,

‎app/Entities/Repos/PageRepo.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ protected function updateTemplateStatusAndContentFromInput(Page $page, array $in
211211
$inputEmpty = empty($input['markdown']) && empty($input['html']);
212212

213213
if ($haveInput && $inputEmpty) {
214-
$pageContent->setNewHTML('');
214+
$pageContent->setNewHTML('', user());
215215
} elseif (!empty($input['markdown']) && is_string($input['markdown'])) {
216216
$newEditor = 'markdown';
217-
$pageContent->setNewMarkdown($input['markdown']);
217+
$pageContent->setNewMarkdown($input['markdown'], user());
218218
} elseif (isset($input['html'])) {
219219
$newEditor = 'wysiwyg';
220-
$pageContent->setNewHTML($input['html']);
220+
$pageContent->setNewHTML($input['html'], user());
221221
}
222222

223223
if ($newEditor !== $currentEditor && userCan('editor-change')) {
@@ -284,9 +284,9 @@ public function restoreRevision(Page $page, int $revisionId): Page
284284
$content = new PageContent($page);
285285

286286
if (!empty($revision->markdown)) {
287-
$content->setNewMarkdown($revision->markdown);
287+
$content->setNewMarkdown($revision->markdown, user());
288288
} else {
289-
$content->setNewHTML($revision->html);
289+
$content->setNewHTML($revision->html, user());
290290
}
291291

292292
$page->updated_by = user()->id;

‎app/Entities/Tools/PageContent.php‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
use BookStack\Theming\ThemeEvents;
1010
use BookStack\Uploads\ImageRepo;
1111
use BookStack\Uploads\ImageService;
12+
use BookStack\Users\Models\User;
1213
use BookStack\Util\HtmlContentFilter;
14+
use BookStack\Util\WebSafeMimeSniffer;
1315
use DOMDocument;
1416
use DOMElement;
1517
use DOMNode;
@@ -27,9 +29,9 @@ public function __construct(
2729
/**
2830
* Update the content of the page with new provided HTML.
2931
*/
30-
public function setNewHTML(string $html): void
32+
public function setNewHTML(string $html, User $updater): void
3133
{
32-
$html = $this->extractBase64ImagesFromHtml($html);
34+
$html = $this->extractBase64ImagesFromHtml($html, $updater);
3335
$this->page->html = $this->formatHtml($html);
3436
$this->page->text = $this->toPlainText();
3537
$this->page->markdown = '';
@@ -38,9 +40,9 @@ public function setNewHTML(string $html): void
3840
/**
3941
* Update the content of the page with new provided Markdown content.
4042
*/
41-
public function setNewMarkdown(string $markdown): void
43+
public function setNewMarkdown(string $markdown, User $updater): void
4244
{
43-
$markdown = $this->extractBase64ImagesFromMarkdown($markdown);
45+
$markdown = $this->extractBase64ImagesFromMarkdown($markdown, $updater);
4446
$this->page->markdown = $markdown;
4547
$html = (new MarkdownToHtml($markdown))->convert();
4648
$this->page->html = $this->formatHtml($html);
@@ -50,7 +52,7 @@ public function setNewMarkdown(string $markdown): void
5052
/**
5153
* Convert all base64 image data to saved images.
5254
*/
53-
protected function extractBase64ImagesFromHtml(string $htmlText): string
55+
protected function extractBase64ImagesFromHtml(string $htmlText, User $updater): string
5456
{
5557
if (empty($htmlText) || !str_contains($htmlText, 'data:image')) {
5658
return $htmlText;
@@ -66,7 +68,7 @@ protected function extractBase64ImagesFromHtml(string $htmlText): string
6668
$imageNodes = $xPath->query('//img[contains(@src, \'data:image\')]');
6769
foreach ($imageNodes as $imageNode) {
6870
$imageSrc = $imageNode->getAttribute('src');
69-
$newUrl = $this->base64ImageUriToUploadedImageUrl($imageSrc);
71+
$newUrl = $this->base64ImageUriToUploadedImageUrl($imageSrc, $updater);
7072
$imageNode->setAttribute('src', $newUrl);
7173
}
7274

@@ -86,7 +88,7 @@ protected function extractBase64ImagesFromHtml(string $htmlText): string
8688
* Attempting to capture the whole data uri using regex can cause PHP
8789
* PCRE limits to be hit with larger, multi-MB, files.
8890
*/
89-
protected function extractBase64ImagesFromMarkdown(string $markdown): string
91+
protected function extractBase64ImagesFromMarkdown(string $markdown, User $updater): string
9092
{
9193
$matches = [];
9294
$contentLength = strlen($markdown);
@@ -104,7 +106,7 @@ protected function extractBase64ImagesFromMarkdown(string $markdown): string
104106
$dataUri .= $char;
105107
}
106108

107-
$newUrl = $this->base64ImageUriToUploadedImageUrl($dataUri);
109+
$newUrl = $this->base64ImageUriToUploadedImageUrl($dataUri, $updater);
108110
$replacements[] = [$dataUri, $newUrl];
109111
}
110112

@@ -119,16 +121,28 @@ protected function extractBase64ImagesFromMarkdown(string $markdown): string
119121
* Parse the given base64 image URI and return the URL to the created image instance.
120122
* Returns an empty string if the parsed URI is invalid or causes an error upon upload.
121123
*/
122-
protected function base64ImageUriToUploadedImageUrl(string $uri): string
124+
protected function base64ImageUriToUploadedImageUrl(string $uri, User $updater): string
123125
{
124126
$imageRepo = app()->make(ImageRepo::class);
125127
$imageInfo = $this->parseBase64ImageUri($uri);
126128

129+
// Validate user has permission to create images
130+
if (!$updater->can('image-create-all')) {
131+
return '';
132+
}
133+
127134
// Validate extension and content
128135
if (empty($imageInfo['data']) || !ImageService::isExtensionSupported($imageInfo['extension'])) {
129136
return '';
130137
}
131138

139+
// Validate content looks like an image via sniffing mime type
140+
$mimeSniffer = new WebSafeMimeSniffer();
141+
$mime = $mimeSniffer->sniff($imageInfo['data']);
142+
if (!str_starts_with($mime, 'image/')) {
143+
return '';
144+
}
145+
132146
// Validate that the content is not over our upload limit
133147
$uploadLimitBytes = (config('app.upload_limit') * 1000000);
134148
if (strlen($imageInfo['data']) > $uploadLimitBytes) {

‎app/Uploads/FaviconHandler.php‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@
33
namespace BookStack\Uploads;
44

55
use Illuminate\Http\UploadedFile;
6-
use Intervention\Image\ImageManager;
76

87
class FaviconHandler
98
{
109
protected string $path;
1110

1211
public function __construct(
13-
protected ImageManager $imageTool
12+
protected ImageResizer $imageResizer,
1413
) {
1514
$this->path = public_path('favicon.ico');
1615
}
@@ -25,10 +24,8 @@ public function saveForUploadedImage(UploadedFile $file): void
2524
}
2625

2726
$imageData = file_get_contents($file->getRealPath());
28-
$image = $this->imageTool->make($imageData);
29-
$image->resize(32, 32);
30-
$bmpData = $image->encode('png');
31-
$icoData = $this->pngToIco($bmpData, 32, 32);
27+
$pngData = $this->imageResizer->resizeImageData($imageData, 32, 32, false, 'png');
28+
$icoData = $this->pngToIco($pngData, 32, 32);
3229

3330
file_put_contents($this->path, $icoData);
3431
}
@@ -81,7 +78,7 @@ public function getOriginalPath(): string
8178
* Built following the file format info from Wikipedia:
8279
* https://en.wikipedia.org/wiki/ICO_(file_format)
8380
*/
84-
protected function pngToIco(string $bmpData, int $width, int $height): string
81+
protected function pngToIco(string $pngData, int $width, int $height): string
8582
{
8683
// ICO header
8784
$header = pack('v', 0x00); // Reserved. Must always be 0
@@ -100,11 +97,11 @@ protected function pngToIco(string $bmpData, int $width, int $height): string
10097
// via intervention from png typically provides this as 24.
10198
$entry .= pack('v', 0x00);
10299
// Size of the image data in bytes
103-
$entry .= pack('V', strlen($bmpData));
100+
$entry .= pack('V', strlen($pngData));
104101
// Offset of the bmp data from file start
105102
$entry .= pack('V', strlen($header) + strlen($entry) + 4);
106103

107104
// Join & return the combined parts of the ICO image data
108-
return $header . $entry . $bmpData;
105+
return $header . $entry . $pngData;
109106
}
110107
}

‎app/Uploads/ImageResizer.php‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@
66
use Exception;
77
use GuzzleHttp\Psr7\Utils;
88
use Illuminate\Support\Facades\Cache;
9+
use Intervention\Image\Gd\Driver;
910
use Intervention\Image\Image as InterventionImage;
10-
use Intervention\Image\ImageManager;
1111

1212
class ImageResizer
1313
{
1414
protected const THUMBNAIL_CACHE_TIME = 604_800; // 1 week
1515

1616
public function __construct(
17-
protected ImageManager $intervention,
1817
protected ImageStorage $storage,
1918
) {
2019
}
@@ -105,13 +104,19 @@ public function resizeToThumbnailUrl(
105104

106105
/**
107106
* Resize the image of given data to the specified size, and return the new image data.
107+
* Format will remain the same as the input format, unless specified.
108108
*
109109
* @throws ImageUploadException
110110
*/
111-
public function resizeImageData(string $imageData, ?int $width, ?int $height, bool $keepRatio): string
112-
{
111+
public function resizeImageData(
112+
string $imageData,
113+
?int $width,
114+
?int $height,
115+
bool $keepRatio,
116+
?string $format = null,
117+
): string {
113118
try {
114-
$thumb = $this->intervention->make($imageData);
119+
$thumb = $this->interventionFromImageData($imageData);
115120
} catch (Exception $e) {
116121
throw new ImageUploadException(trans('errors.cannot_create_thumbs'));
117122
}
@@ -127,7 +132,7 @@ public function resizeImageData(string $imageData, ?int $width, ?int $height, bo
127132
$thumb->fit($width, $height);
128133
}
129134

130-
$thumbData = (string) $thumb->encode();
135+
$thumbData = (string) $thumb->encode($format);
131136

132137
// Use original image data if we're keeping the ratio
133138
// and the resizing does not save any space.
@@ -138,6 +143,17 @@ public function resizeImageData(string $imageData, ?int $width, ?int $height, bo
138143
return $thumbData;
139144
}
140145

146+
/**
147+
* Create an intervention image instance from the given image data.
148+
* Performs some manual library usage to ensure image is specifically loaded
149+
* from given binary data instead of data being misinterpreted.
150+
*/
151+
protected function interventionFromImageData(string $imageData): InterventionImage
152+
{
153+
$driver = new Driver();
154+
return $driver->decoder->initFromBinary($imageData);
155+
}
156+
141157
/**
142158
* Orientate the given intervention image based upon the given original image data.
143159
* Intervention does have an `orientate` method but the exif data it needs is lost before it

‎app/Users/Models/User.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,6 @@ public function attachDefaultRole(): void
160160
*/
161161
public function can(string $permissionName): bool
162162
{
163-
if ($this->email === 'guest') {
164-
return false;
165-
}
166-
167163
return $this->permissions()->contains($permissionName);
168164
}
169165

‎resources/js/services/util.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import {elem} from './dom';
2-
31
/**
42
* Returns a function, that, as long as it continues to be invoked, will not
53
* be triggered. The function will be called after it stops being called for

‎tests/Entity/PageContentTest.php‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
class PageContentTest extends TestCase
1010
{
11-
protected $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=';
11+
protected string $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=';
1212

1313
public function test_page_includes()
1414
{
@@ -649,6 +649,35 @@ public function test_base64_images_within_html_blanked_if_not_supported_extensio
649649
}
650650
}
651651

652+
public function test_base64_images_within_html_blanked_if_no_image_create_permission()
653+
{
654+
$editor = $this->users->editor();
655+
$page = $this->entities->page();
656+
$this->permissions->removeUserRolePermissions($editor, ['image-create-all']);
657+
658+
$this->actingAs($editor)->put($page->getUrl(), [
659+
'name' => $page->name,
660+
'html' => '<p>test<img src="data:image/jpeg;base64,' . $this->base64Jpeg . '"/></p>',
661+
]);
662+
663+
$page->refresh();
664+
$this->assertStringMatchesFormat('%A<p%A>test<img src="">%A</p>%A', $page->html);
665+
}
666+
667+
public function test_base64_images_within_html_blanked_if_content_does_not_appear_like_an_image()
668+
{
669+
$page = $this->entities->page();
670+
671+
$imgContent = base64_encode('file://test/a/b/c');
672+
$this->asEditor()->put($page->getUrl(), [
673+
'name' => $page->name,
674+
'html' => '<p>test<img src="data:image/jpeg;base64,' . $imgContent . '"/></p>',
675+
]);
676+
677+
$page->refresh();
678+
$this->assertStringMatchesFormat('%A<p%A>test<img src="">%A</p>%A', $page->html);
679+
}
680+
652681
public function test_base64_images_get_extracted_from_markdown_page_content()
653682
{
654683
$this->asEditor();
@@ -682,7 +711,7 @@ public function test_markdown_base64_extract_not_limited_by_pcre_limits()
682711
ini_set('pcre.backtrack_limit', '500');
683712
ini_set('pcre.recursion_limit', '500');
684713

685-
$content = str_repeat('a', 5000);
714+
$content = str_repeat(base64_decode($this->base64Jpeg), 50);
686715
$base64Content = base64_encode($content);
687716

688717
$this->put($page->getUrl(), [
@@ -716,6 +745,34 @@ public function test_base64_images_within_markdown_blanked_if_not_supported_exte
716745
$this->assertStringContainsString('<img src=""', $page->refresh()->html);
717746
}
718747

748+
public function test_base64_images_within_markdown_blanked_if_no_image_create_permission()
749+
{
750+
$editor = $this->users->editor();
751+
$page = $this->entities->page();
752+
$this->permissions->removeUserRolePermissions($editor, ['image-create-all']);
753+
754+
$this->actingAs($editor)->put($page->getUrl(), [
755+
'name' => $page->name,
756+
'markdown' => 'test ![test](data:image/jpeg;base64,' . $this->base64Jpeg . ')',
757+
]);
758+
759+
$this->assertStringContainsString('<img src=""', $page->refresh()->html);
760+
}
761+
762+
public function test_base64_images_within_markdown_blanked_if_content_does_not_appear_like_an_image()
763+
{
764+
$page = $this->entities->page();
765+
766+
$imgContent = base64_encode('file://test/a/b/c');
767+
$this->asEditor()->put($page->getUrl(), [
768+
'name' => $page->name,
769+
'markdown' => 'test ![test](data:image/jpeg;base64,' . $imgContent . ')',
770+
]);
771+
772+
$page->refresh();
773+
$this->assertStringContainsString('<img src=""', $page->refresh()->html);
774+
}
775+
719776
public function test_nested_headers_gets_assigned_an_id()
720777
{
721778
$page = $this->entities->page();

‎tests/ThemeTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function test_event_commonmark_environment_configure()
7878

7979
$page = $this->entities->page();
8080
$content = new PageContent($page);
81-
$content->setNewMarkdown('# test');
81+
$content->setNewMarkdown('# test', $this->users->editor());
8282

8383
$this->assertTrue($callbackCalled);
8484
}

0 commit comments

Comments
 (0)