From b1e4c0931af6a1c937ea713cf70a5dbc742f30db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 13:14:00 +0200 Subject: [PATCH 1/9] Fixes and type-hints: - added more checked type-hints as they prevent bad method invocations - fixed TypeError for HTML::toBBCode() invocations with NULL as first (wrong) argument, thanks to @tobias@social.diekershoff.de pointing this out. --- src/Model/APContact.php | 6 +++++- src/Protocol/ActivityPub/Processor.php | 27 +++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Model/APContact.php b/src/Model/APContact.php index 0255af7a33..e99a692a18 100644 --- a/src/Model/APContact.php +++ b/src/Model/APContact.php @@ -241,9 +241,13 @@ class APContact $apcontact['name'] = $apcontact['nick']; } - $apcontact['about'] = HTML::toBBCode(JsonLD::fetchElement($compacted, 'as:summary', '@value')); + $aboutHtml = JsonLD::fetchElement($compacted, 'as:summary', '@value'); + if ($aboutHtml != "") { + $apcontact['about'] = HTML::toBBCode($aboutHtml); + } $ims = JsonLD::fetchElementArray($compacted, 'vcard:hasInstantMessage'); + if (!empty($ims)) { foreach ($ims as $link) { if (substr($link, 0, 5) == 'xmpp:') { diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 9b6dd4d1a6..f723d287b9 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -71,7 +71,7 @@ class Processor * @param array $languages * @return string language JSON */ - private static function processLanguages(array $languages) + private static function processLanguages(array $languages): string { $codes = array_keys($languages); $lang = []; @@ -88,12 +88,13 @@ class Processor /** * Replaces emojis in the body * - * @param array $emojis + * @param int $uri_id * @param string $body + * @param array $emojis * * @return string with replaced emojis */ - private static function replaceEmojis(int $uri_id, $body, array $emojis) + private static function replaceEmojis(int $uri_id, string $body, array $emojis) { $body = strtr($body, array_combine( @@ -143,7 +144,7 @@ class Processor * @param array $activity * @param array $item */ - private static function storeAttachments($activity, $item) + private static function storeAttachments(array $activity, array $item) { if (empty($activity['attachments'])) { return; @@ -160,7 +161,7 @@ class Processor * @param array $activity * @param array $item */ - private static function storeQuestion($activity, $item) + private static function storeQuestion(array $activity, array $item) { if (empty($activity['question'])) { return; @@ -191,7 +192,7 @@ class Processor * @param array $activity Activity array * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function updateItem($activity) + public static function updateItem(array $activity) { $item = Post::selectFirst(['uri', 'uri-id', 'thr-parent', 'gravity', 'post-type'], ['uri' => $activity['id']]); if (!DBA::isResult($item)) { @@ -262,7 +263,7 @@ class Processor * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function createItem($activity) + public static function createItem(array $activity): array { $item = []; $item['verb'] = Activity::POST; @@ -411,7 +412,7 @@ class Processor * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function deleteItem($activity) + public static function deleteItem(array $activity) { $owner = Contact::getIdForURL($activity['actor']); @@ -426,7 +427,7 @@ class Processor * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function addTag($activity) + public static function addTag(array $activity) { if (empty($activity['object_content']) || empty($activity['object_id'])) { return; @@ -457,7 +458,7 @@ class Processor * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function createActivity($activity, $verb) + public static function createActivity(array $activity, string $verb) { $item = self::createItem($activity); if (empty($item)) { @@ -561,7 +562,7 @@ class Processor * @return int event id * @throws \Exception */ - public static function createEvent($activity, $item) + public static function createEvent(array $activity, array $item): int { $event['summary'] = HTML::toBBCode($activity['name'] ?: $activity['summary']); $event['desc'] = HTML::toBBCode($activity['content']); @@ -605,7 +606,7 @@ class Processor * @return array|bool Returns the item array or false if there was an unexpected occurrence * @throws \Exception */ - private static function processContent($activity, $item) + private static function processContent(array $activity, array $item) { if (!empty($activity['mediatype']) && ($activity['mediatype'] == 'text/markdown')) { $item['title'] = strip_tags($activity['name']); @@ -615,7 +616,7 @@ class Processor $content = $activity['content']; } else { // By default assume "text/html" - $item['title'] = HTML::toBBCode($activity['name']); + $item['title'] = (empty($activity['name']) ? HTML::toBBCode($activity['name']) : ''); $content = HTML::toBBCode($activity['content']); } From b200874f17686cc2b1c8eb4acb760572b79aff0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 13:38:40 +0200 Subject: [PATCH 2/9] Ops: - wrong way around (!empty($foo)) is proper - also needed to be checked on $acitivty['content'] --- src/Protocol/ActivityPub/Processor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index f723d287b9..31a37a36f1 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -616,8 +616,8 @@ class Processor $content = $activity['content']; } else { // By default assume "text/html" - $item['title'] = (empty($activity['name']) ? HTML::toBBCode($activity['name']) : ''); - $content = HTML::toBBCode($activity['content']); + $item['title'] = (!empty($activity['name']) ? HTML::toBBCode($activity['name']) : ''); + $content = (!empty($activity['content']) ? HTML::toBBCode($activity['content']) : ''); } $item['title'] = trim(BBCode::toPlaintext($item['title'])); From 7814ba4fc4be52c3bb387527be1df8fb659d6628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 14:21:11 +0200 Subject: [PATCH 3/9] Fixes for bad invocations of HTML::toBBCode() (1st parameter is now string) --- src/Protocol/ActivityPub/Processor.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 31a37a36f1..077ba3e3fe 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -651,7 +651,7 @@ class Processor $content = self::removeImplicitMentionsFromBody($content, $parent); } - $item['content-warning'] = HTML::toBBCode($activity['summary']); + $item['content-warning'] = (!empty($activity['summary']) ? HTML::toBBCode($activity['summary']) : ''); $item['raw-body'] = $item['body'] = $content; } @@ -1225,7 +1225,8 @@ class Processor $attributed_to = JsonLD::fetchElement($activity['as:object'], 'as:attributedTo', '@id'); $authorid = Contact::getIdForURL($attributed_to); - $body = HTML::toBBCode(JsonLD::fetchElement($activity['as:object'], 'as:content', '@value')); + $value = JsonLD::fetchElement($activity['as:object'], 'as:content', '@value'); + $body = (!empty($value) ? HTML::toBBCode($value) : ''); $messageTags = []; $tags = Receiver::processTags(JsonLD::fetchElementArray($activity['as:object'], 'as:tag') ?? []); From f2b7326650d65728619c3e3545e3ff29b1cd88ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 15:08:04 +0200 Subject: [PATCH 4/9] This will 2 things: 1) The first change prevents "expensive" code in HTML::toBBcode() to be executed just for an empty string which makes no sense. 2) The above change was maybe flawed as $apcontact['about'] would have never been created, not even empty which could have side effects Thanks to @annando to make me rethink this part of code. --- src/Model/APContact.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Model/APContact.php b/src/Model/APContact.php index e99a692a18..69f9be5135 100644 --- a/src/Model/APContact.php +++ b/src/Model/APContact.php @@ -242,9 +242,7 @@ class APContact } $aboutHtml = JsonLD::fetchElement($compacted, 'as:summary', '@value'); - if ($aboutHtml != "") { - $apcontact['about'] = HTML::toBBCode($aboutHtml); - } + $apcontact['about'] = (!empty($aboutHtml) ? HTML::toBBCode($aboutHtml) : ''); $ims = JsonLD::fetchElementArray($compacted, 'vcard:hasInstantMessage'); From 962b06bf41e773ace6c898df510043f3cda206f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 15:45:22 +0200 Subject: [PATCH 5/9] Added check as suggested by @MrPetovan for empty $message. --- src/Content/Text/HTML.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Content/Text/HTML.php b/src/Content/Text/HTML.php index 9a5b909256..9121087296 100644 --- a/src/Content/Text/HTML.php +++ b/src/Content/Text/HTML.php @@ -143,6 +143,14 @@ class HTML */ public static function toBBCode(string $message, string $basepath = ''): string { + /* + * Check if message is empty to prevent a lot code below being executed + * for just an empty message. + */ + if (empty($message)) { + return ''; + } + DI::profiler()->startRecording('rendering'); $message = str_replace("\r", "", $message); From 624e4c192c7f837ac0587a50da6e1409081eb519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 16:59:54 +0200 Subject: [PATCH 6/9] Changed to null-coalscing style (??) as sugguested by @MrPetovan --- src/Protocol/ActivityPub/Processor.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 077ba3e3fe..311806f8c0 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -616,8 +616,8 @@ class Processor $content = $activity['content']; } else { // By default assume "text/html" - $item['title'] = (!empty($activity['name']) ? HTML::toBBCode($activity['name']) : ''); - $content = (!empty($activity['content']) ? HTML::toBBCode($activity['content']) : ''); + $item['title'] = HTML::toBBCode($activity['name'] ??: ''); + $content = HTML::toBBCode($activity['content'] ?? : ''); } $item['title'] = trim(BBCode::toPlaintext($item['title'])); @@ -651,7 +651,7 @@ class Processor $content = self::removeImplicitMentionsFromBody($content, $parent); } - $item['content-warning'] = (!empty($activity['summary']) ? HTML::toBBCode($activity['summary']) : ''); + $item['content-warning'] = HTML::toBBCode($activity['summary'] ?? ''); $item['raw-body'] = $item['body'] = $content; } From e8fee5644b5e746f8c09e604caac3d678f376989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 17:03:25 +0200 Subject: [PATCH 7/9] Ops, syntax errors get unnoticed with a simple editor. :-( --- src/Protocol/ActivityPub/Processor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 311806f8c0..1c448cd575 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -616,8 +616,8 @@ class Processor $content = $activity['content']; } else { // By default assume "text/html" - $item['title'] = HTML::toBBCode($activity['name'] ??: ''); - $content = HTML::toBBCode($activity['content'] ?? : ''); + $item['title'] = HTML::toBBCode($activity['name'] ?? ''); + $content = HTML::toBBCode($activity['content'] ?? ''); } $item['title'] = trim(BBCode::toPlaintext($item['title'])); From 0e1f734b038849d3d213adf0ddc6b02c5c8c1691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 17:34:43 +0200 Subject: [PATCH 8/9] Also make this null-coalscing --- src/Model/APContact.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Model/APContact.php b/src/Model/APContact.php index 69f9be5135..9c972efaf2 100644 --- a/src/Model/APContact.php +++ b/src/Model/APContact.php @@ -241,8 +241,7 @@ class APContact $apcontact['name'] = $apcontact['nick']; } - $aboutHtml = JsonLD::fetchElement($compacted, 'as:summary', '@value'); - $apcontact['about'] = (!empty($aboutHtml) ? HTML::toBBCode($aboutHtml) : ''); + $apcontact['about'] = HTML::toBBCode(JsonLD::fetchElement($compacted, 'as:summary', '@value') ?? ''); $ims = JsonLD::fetchElementArray($compacted, 'vcard:hasInstantMessage'); From 51a7b5c5846c320dd273f5b1005d856416dd1252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Thu, 16 Jun 2022 17:36:32 +0200 Subject: [PATCH 9/9] Made also this one sweeter (null-coalscing) --- src/Protocol/ActivityPub/Processor.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 1c448cd575..2288427b47 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -1225,8 +1225,7 @@ class Processor $attributed_to = JsonLD::fetchElement($activity['as:object'], 'as:attributedTo', '@id'); $authorid = Contact::getIdForURL($attributed_to); - $value = JsonLD::fetchElement($activity['as:object'], 'as:content', '@value'); - $body = (!empty($value) ? HTML::toBBCode($value) : ''); + $body = HTML::toBBCode(JsonLD::fetchElement($activity['as:object'], 'as:content', '@value') ?? ''); $messageTags = []; $tags = Receiver::processTags(JsonLD::fetchElementArray($activity['as:object'], 'as:tag') ?? []);