From 8e43331d4719ee23f0f923f0cd66dc7b59e636ab Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 29 Jan 2021 09:46:14 +0100 Subject: [PATCH 1/4] Improve mention matches The previous regex didn't correctly match users with . ' or - Signed-off-by: Joas Schilling --- apps/comments/lib/Activity/Provider.php | 12 +++--------- apps/comments/lib/Notification/Notifier.php | 6 +++++- lib/private/Comments/Comment.php | 1 + 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/apps/comments/lib/Activity/Provider.php b/apps/comments/lib/Activity/Provider.php index 527c9c4ca690f..fca3b9e75d53e 100644 --- a/apps/comments/lib/Activity/Provider.php +++ b/apps/comments/lib/Activity/Provider.php @@ -213,17 +213,11 @@ protected function parseMessage(IEvent $event) { continue; } - $pattern = '/(^|\s)(' . '@' . $mention['id'] . ')(\b)/'; - if (strpos($mention['id'], ' ') !== false) { - $pattern = '/(^|\s)(' . '@"' . $mention['id'] . '"' . ')(\b)?/'; + $message = str_replace('@"' . $mention['id'] . '"', '{mention' . $mentionCount . '}', $message); + if (strpos($mention['id'], ' ') === false && strpos($mention['id'], 'guest/') !== 0) { + $message = str_replace('@' . $mention['id'], '{mention' . $mentionCount . '}', $message); } - $message = preg_replace( - $pattern, - //'${1}' . $this->regexSafeUser($mention['id'], $displayName) . '${3}', - '${1}' . '{mention' . $mentionCount . '}' . '${3}', - $message - ); $mentions['mention' . $mentionCount] = $this->generateUserParameter($mention['id']); $mentionCount++; } diff --git a/apps/comments/lib/Notification/Notifier.php b/apps/comments/lib/Notification/Notifier.php index 9f69e5599f8f2..20e0667d143c2 100644 --- a/apps/comments/lib/Notification/Notifier.php +++ b/apps/comments/lib/Notification/Notifier.php @@ -195,7 +195,11 @@ public function commentToRichMessage(IComment $comment): array { // could contain characters like '@' for user IDs) but a one-based // index of the mentions of that type. $mentionParameterId = 'mention-' . $mention['type'] . $mentionTypeCount[$mention['type']]; - $message = str_replace('@' . $mention['id'], '{' . $mentionParameterId . '}', $message); + $message = str_replace('@"' . $mention['id'] . '"', '{' . $mentionParameterId . '}', $message); + if (strpos($mention['id'], ' ') === false && strpos($mention['id'], 'guest/') !== 0) { + $message = str_replace('@' . $mention['id'], '{' . $mentionParameterId . '}', $message); + } + try { $displayName = $this->commentsManager->resolveDisplayName($mention['type'], $mention['id']); } catch (\OutOfBoundsException $e) { diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index 7368425174a84..c838e9571c6b7 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -233,6 +233,7 @@ public function getMentions() { return []; } $uids = array_unique($mentions[0]); + usort($uids, 'mb_strlen'); $result = []; foreach ($uids as $uid) { $cleanUid = trim(substr($uid, 1), '"'); From cf11f914a72489321bd2065782af3c3ea4900b10 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 29 Jan 2021 10:13:14 +0100 Subject: [PATCH 2/4] Make Psalm happy Signed-off-by: Joas Schilling --- lib/private/Comments/Comment.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index c838e9571c6b7..e2d32e7485047 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -233,7 +233,9 @@ public function getMentions() { return []; } $uids = array_unique($mentions[0]); - usort($uids, 'mb_strlen'); + usort($uids, static function($uid1, $uid2) { + return mb_strlen($uid2) <=> mb_strlen($uid1); + }); $result = []; foreach ($uids as $uid) { $cleanUid = trim(substr($uid, 1), '"'); From 814a8accccde0eb2fa7699821443d922ff0b89a6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 29 Jan 2021 13:09:37 +0100 Subject: [PATCH 3/4] Fix unit tests Signed-off-by: Joas Schilling --- tests/lib/Comments/CommentTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/Comments/CommentTest.php b/tests/lib/Comments/CommentTest.php index 245ca9693ea12..7b3254ac1419f 100644 --- a/tests/lib/Comments/CommentTest.php +++ b/tests/lib/Comments/CommentTest.php @@ -51,7 +51,7 @@ public function testSettersValidInput() { $this->assertSame($object['id'], $comment->getObjectId()); } - + public function testSetIdIllegalInput() { $this->expectException(\OCP\Comments\IllegalIDChangeException::class); @@ -120,7 +120,7 @@ public function testSetRoleInvalidInput($role, $type, $id) { $comment->$setter($type, $id); } - + public function testSetUberlongMessage() { $this->expectException(\OCP\Comments\MessageTooLongException::class); @@ -149,7 +149,7 @@ public function mentionsProvider() { ' cc @23452-4333-54353-2342 @yolo!' . ' however the most important thing to know is that www.croissant.com/@oil is not valid' . ' and won\'t match anything at all', - ['foobar', 'barfoo', 'foo@bar.com', 'bar@foo.org@foobar.io', '23452-4333-54353-2342', 'yolo'] + ['bar@foo.org@foobar.io', '23452-4333-54353-2342', 'foo@bar.com', 'foobar', 'barfoo', 'yolo'] ], [ '@@chef is also a valid mention, no matter how strange it looks', ['@chef'] From 5164f43bd36645fc274d6ab18ead37a906631fd4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 2 Feb 2021 13:39:29 +0100 Subject: [PATCH 4/4] Fix linter Signed-off-by: Joas Schilling --- lib/private/Comments/Comment.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index e2d32e7485047..8517bef58936a 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -233,7 +233,7 @@ public function getMentions() { return []; } $uids = array_unique($mentions[0]); - usort($uids, static function($uid1, $uid2) { + usort($uids, static function ($uid1, $uid2) { return mb_strlen($uid2) <=> mb_strlen($uid1); }); $result = [];