plugins-nextcloud-mirror/spreed
1
0
Fork 0
mirror of https://github.com/nextcloud/spreed.git synced 2025-12-17 21:12:20 +01:00
Code Projects Releases Activity

fix(performance): Parse file shares inaccurately to prevent setupFS

Browse source 
In some cases we only use the "plain text" version of a chat message.
We can render the conversation list subline without knowing the exact
path of a file. All other information is still available on the cache
entry. The path is only needed in some special cases when afterwards
the viewer is proposed to the user.

Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2025-04-02 08:39:44 +02:00
parent b21659365b
commit fa7d240896
No known key found for this signature in database
GPG key ID: F72FA5B49FFA96B0
12 changed files with 56 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
lib/Chat/MessageParser.php
View file

@ -66,7 +66,12 @@ class MessageParser {
return $message;
}
public function parseMessage(Message $message): void {
/**
* @param bool $allowInaccurate File share messages will not have fully correct data for the file object
* E.g. path is only the file name and preview generation is estimated by
* mimetype only. This is done to prevent a filesystem setup.
*/
public function parseMessage(Message $message, bool $allowInaccurate = false): void {
$message->setMessage($message->getComment()->getMessage(), []);
$verb = $message->getComment()->getVerb();
@ -77,7 +82,7 @@ class MessageParser {
$this->setMessageActor($message);
$this->setLastEditInfo($message);
$event = new MessageParseEvent($message->getRoom(), $message);
$event = new MessageParseEvent($message->getRoom(), $message, $allowInaccurate);
$this->dispatcher->dispatchTyped($event);
}

36
lib/Chat/Parser/SystemMessage.php
View file

@ -28,6 +28,7 @@ use OCP\Comments\ICommentsManager;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Federation\ICloudIdManager;
use OCP\Files\FileInfo;
use OCP\Files\InvalidPathException;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
@ -92,7 +93,7 @@ class SystemMessage implements IEventListener {
if ($event->getMessage()->getMessageType() === ChatManager::VERB_SYSTEM) {
try {
$this->parseMessage($event->getMessage());
$this->parseMessage($event->getMessage(), $event->allowInaccurate());
// Disabled so we can parse mentions in captions: $event->stopPropagation();
} catch (\OutOfBoundsException $e) {
// Unknown message, ignore
@ -111,7 +112,7 @@ class SystemMessage implements IEventListener {
* @param Message $chatMessage
* @throws \OutOfBoundsException
*/
protected function parseMessage(Message $chatMessage): void {
protected function parseMessage(Message $chatMessage, $allowInaccurate): void {
$this->l = $chatMessage->getL10n();
$comment = $chatMessage->getComment();
$room = $chatMessage->getRoom();
@ -515,7 +516,7 @@ class SystemMessage implements IEventListener {
}
} elseif ($message === 'file_shared') {
try {
$parsedParameters['file'] = $this->getFileFromShare($room, $participant, $parameters['share']);
$parsedParameters['file'] = $this->getFileFromShare($room, $participant, $parameters['share'], $allowInaccurate);
$parsedMessage = '{file}';
$metaData = $parameters['metaData'] ?? [];
if (isset($metaData['messageType'])) {
@ -759,18 +760,24 @@ class SystemMessage implements IEventListener {
}
/**
* @param ?Participant $participant
* @param string $shareId
* @return array
* @throws InvalidPathException
* @throws NotFoundException
* @throws ShareNotFound
*/
protected function getFileFromShare(Room $room, ?Participant $participant, string $shareId): array {
protected function getFileFromShare(Room $room, ?Participant $participant, string $shareId, bool $allowInaccurate): array {
$share = $this->shareProvider->getShareById((int)$shareId);
if ($participant && $participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) {
if ($share->getShareOwner() !== $participant->getAttendee()->getActorId()) {
if ($allowInaccurate) {
$node = $share->getNodeCacheEntry();
if ($node === null) {
throw new ShareNotFound();
}
$name = $node->getName();
$size = $node->getSize();
$path = $name;
} elseif ($share->getShareOwner() !== $participant->getAttendee()->getActorId()) {
$userFolder = $this->rootFolder->getUserFolder($participant->getAttendee()->getActorId());
if (!$userFolder instanceof Node) {
throw new ShareNotFound();
@ -814,7 +821,12 @@ class SystemMessage implements IEventListener {
} elseif ($participant && $room->getType() !== Room::TYPE_PUBLIC && $participant->getAttendee()->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
throw new ShareNotFound();
} else {
$node = $share->getNode();
if ($allowInaccurate) {
$node = $share->getNodeCacheEntry();
} else {
$node = $share->getNode();
}
$name = $node->getName();
$size = $node->getSize();
$path = $name;
@ -825,7 +837,11 @@ class SystemMessage implements IEventListener {
}
$fileId = $node->getId();
$isPreviewAvailable = $this->previewManager->isAvailable($node);
if ($node instanceof FileInfo) {
$isPreviewAvailable = $this->previewManager->isAvailable($node);
} else {
$isPreviewAvailable = $size > 0 && $this->previewManager->isMimeSupported($node->getMimeType());
}
$data = [
'type' => 'file',

2
lib/Chat/SystemMessage/Listener.php
View file

@ -469,7 +469,7 @@ class Listener implements IEventListener {
try {
$parentComment = $this->chatManager->getParentComment($room, (string)$replyTo);
$parentMessage = $this->messageParser->createMessage($room, $participant, $parentComment, $this->l);
$this->messageParser->parseMessage($parentMessage);
$this->messageParser->parseMessage($parentMessage, true);
if ($parentMessage->isReplyable()) {
$parent = $parentComment;
}

2
lib/Collaboration/Reference/TalkReferenceProvider.php
View file

@ -173,7 +173,7 @@ class TalkReferenceProvider extends ADiscoverableReferenceProvider implements IS
throw new RoomNotFoundException();
}
$message = $this->messageParser->createMessage($room, $participant, $comment, $this->l);
$this->messageParser->parseMessage($message);
$this->messageParser->parseMessage($message, true);
} else {
try {
$proxy = $this->proxyCacheMessageMapper->findById($room, (int)$messageId);

2
lib/Controller/ChatController.php
View file

@ -553,7 +553,7 @@ class ChatController extends AEnvironmentAwareOCSController {
foreach ($comments as $comment) {
$nextOffset = (int)$comment->getId();
$message = $this->messageParser->createMessage($this->room, $this->participant, $comment, $this->l);
$this->messageParser->parseMessage($message);
$this->messageParser->parseMessage($message, true);
if (!$message->getVisibility()) {
continue;

2
lib/Dashboard/TalkWidget.php
View file

@ -281,7 +281,7 @@ class TalkWidget implements IAPIWidget, IIconWidget, IButtonWidget, IOptionWidge
}
} elseif ($room->getLastMessageId() && $room->getLastMessage() && !$room->isFederatedConversation()) {
$message = $this->messageParser->createMessage($room, $participant, $room->getLastMessage(), $this->l10n);
$this->messageParser->parseMessage($message);
$this->messageParser->parseMessage($message, true);
$subtitle = $this->getSubtitleFromMessage($message);
}

5
lib/Events/MessageParseEvent.php
View file

@ -15,6 +15,7 @@ class MessageParseEvent extends ARoomEvent {
public function __construct(
Room $room,
protected Message $message,
protected bool $allowInaccurate,
) {
parent::__construct($room);
}
@ -22,4 +23,8 @@ class MessageParseEvent extends ARoomEvent {
public function getMessage(): Message {
return $this->message;
}
public function allowInaccurate(): bool {
return $this->allowInaccurate;
}
}

2
lib/Notification/Notifier.php
View file

@ -525,7 +525,7 @@ class Notifier implements INotifier {
}
$message = $this->messageParser->createMessage($room, $participant, $comment, $l);
$this->messageParser->parseMessage($message);
$this->messageParser->parseMessage($message, true);
if (!$message->getVisibility()) {
throw new AlreadyProcessedException();

4
lib/Service/BotService.php
View file

@ -106,7 +106,7 @@ class BotService {
$parent,
$this->l10nFactory->get('spreed', 'en', 'en')
);
$messageParser->parseMessage($parentMessage);
$messageParser->parseMessage($parentMessage, true);
$parentMessageData = [
'message' => $parentMessage->getMessage(),
'parameters' => $parentMessage->getMessageParameters(),
@ -125,7 +125,7 @@ class BotService {
$event->getComment(),
$this->l10nFactory->get('spreed', 'en', 'en')
);
$messageParser->parseMessage($message);
$messageParser->parseMessage($message, true);
$messageData = [
'message' => $message->getMessage(),
'parameters' => $message->getMessageParameters(),

2
lib/Service/RoomFormatter.php
View file

@ -435,7 +435,7 @@ class RoomFormatter {
IComment $lastMessage,
): ?array {
$message = $this->messageParser->createMessage($room, $participant, $lastMessage, $this->l10n);
$this->messageParser->parseMessage($message);
$this->messageParser->parseMessage($message, true);
if (!$message->getVisibility()) {
return null;

16
tests/php/Chat/Parser/SystemMessageTest.php
View file

@ -632,7 +632,7 @@ class SystemMessageTest extends TestCase {
'parameters' => $parameters,
]), [], $message);
self::invokePrivate($parser, 'parseMessage', [$chatMessage]);
self::invokePrivate($parser, 'parseMessage', [$chatMessage, false]);
$this->assertSame($expectedMessage, $chatMessage->getMessage());
$this->assertSame($expectedParameters, $chatMessage->getMessageParameters());
@ -672,7 +672,7 @@ class SystemMessageTest extends TestCase {
$chatMessage->setMessage($return, []);
$this->expectException(\OutOfBoundsException::class);
self::invokePrivate($parser, 'parseMessage', [$chatMessage]);
self::invokePrivate($parser, 'parseMessage', [$chatMessage, false]);
}
public function testGetFileFromShareForGuest(): void {
@ -743,7 +743,7 @@ class SystemMessageTest extends TestCase {
'preview-available' => 'yes',
'width' => '1234',
'height' => '4567',
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']));
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]));
}
public function testGetFileFromShareForGuestWithBlurhash(): void {
@ -820,7 +820,7 @@ class SystemMessageTest extends TestCase {
'width' => '1234',
'height' => '4567',
'blurhash' => 'LEHV9uae2yk8pyo0adR*.7kCMdnj',
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']));
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]));
}
public function testGetFileFromShareForOwner(): void {
@ -897,7 +897,7 @@ class SystemMessageTest extends TestCase {
'permissions' => '27',
'mimetype' => 'httpd/unix-directory',
'preview-available' => 'no',
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']));
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]));
}
public function testGetFileFromShareForRecipient(): void {
@ -982,7 +982,7 @@ class SystemMessageTest extends TestCase {
'permissions' => '27',
'mimetype' => 'application/octet-stream',
'preview-available' => 'no',
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']));
], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]));
}
public function testGetFileFromShareForRecipientThrows(): void {
@ -1026,7 +1026,7 @@ class SystemMessageTest extends TestCase {
$parser = $this->getParser();
$this->expectException(NotFoundException::class);
self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']);
self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]);
}
public function testGetFileFromShareThrows(): void {
@ -1046,7 +1046,7 @@ class SystemMessageTest extends TestCase {
->willReturn($attendee);
$parser = $this->getParser();
$this->expectException(ShareNotFound::class);
self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']);
self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23', false]);
}
public static function dataGetActor(): array {

4
tests/php/Controller/ChatControllerTest.php
View file

@ -421,8 +421,8 @@ class ChatControllerTest extends TestCase {
$i = 0;
$expectedCalls = [
[$parentMessage],
[$chatMessage],
[$parentMessage, false],
[$chatMessage, false],
];
$this->messageParser->expects($this->exactly(2))
->method('parseMessage')