From d7e1ce47bbc51f3f0b2fac6ce17b83378f5393b1 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 02:44:43 -0500 Subject: [PATCH 01/18] Use item.thr-parent as expected in Model\Item::insert() - Rework Model\Item::getTopLevelParent - Backward compatibility with item.parent-uri is ensured --- src/Model/Item.php | 175 ++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 90 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index d5f57cff7..c03722372 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1379,7 +1379,7 @@ class Item public static function isValid(array $item) { // When there is no content then we don't post it - if ($item['body'].$item['title'] == '') { + if ($item['body'] . $item['title'] == '') { Logger::notice('No body, no title.'); return false; } @@ -1497,88 +1497,43 @@ class Item } /** - * Fetch parent data for the given item array + * Fetch top-level parent data for the given item array * * @param array $item * @return array item array with parent data + * @throws \Exception */ - private static function getParentData(array $item) + private static function getTopLevelParent(array $item) { - // find the parent and snarf the item id and ACLs - // and anything else we need to inherit - - $fields = ['uri', 'parent-uri', 'id', 'deleted', + $fields = ['uid', 'uri', 'parent-uri', 'id', 'deleted', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'wall', 'private', 'forum_mode', 'origin', 'author-id']; - $condition = ['uri' => $item['parent-uri'], 'uid' => $item['uid']]; + $condition = ['uri' => $item['thr-parent'], 'uid' => $item['uid']]; $params = ['order' => ['id' => false]]; $parent = self::selectFirst($fields, $condition, $params); if (!DBA::isResult($parent)) { - Logger::info('item parent was not found - ignoring item', ['parent-uri' => $item['parent-uri'], 'uid' => $item['uid']]); + Logger::info('item parent was not found - ignoring item', ['thr-parent' => $item['thr-parent'], 'uid' => $item['uid']]); return []; - } else { - // is the new message multi-level threaded? - // even though we don't support it now, preserve the info - // and re-attach to the conversation parent. - if ($parent['uri'] != $parent['parent-uri']) { - $item['parent-uri'] = $parent['parent-uri']; - - $condition = ['uri' => $item['parent-uri'], - 'parent-uri' => $item['parent-uri'], - 'uid' => $item['uid']]; - $params = ['order' => ['id' => false]]; - $toplevel_parent = self::selectFirst($fields, $condition, $params); - - if (DBA::isResult($toplevel_parent)) { - $parent = $toplevel_parent; - } - } - - $item['parent'] = $parent['id']; - $item["deleted"] = $parent['deleted']; - $item["allow_cid"] = $parent['allow_cid']; - $item['allow_gid'] = $parent['allow_gid']; - $item['deny_cid'] = $parent['deny_cid']; - $item['deny_gid'] = $parent['deny_gid']; - $item['parent_origin'] = $parent['origin']; - - // Don't federate received participation messages - if ($item['verb'] != Activity::FOLLOW) { - $item['wall'] = $parent['wall']; - } else { - $item['wall'] = false; - } - - /* - * If the parent is private, force privacy for the entire conversation - * This differs from the above settings as it subtly allows comments from - * email correspondents to be private even if the overall thread is not. - */ - if ($parent['private']) { - $item['private'] = $parent['private']; - } - - /* - * Edge case. We host a public forum that was originally posted to privately. - * The original author commented, but as this is a comment, the permissions - * weren't fixed up so it will still show the comment as private unless we fix it here. - */ - if ((intval($parent['forum_mode']) == 1) && ($parent['private'] != self::PUBLIC)) { - $item['private'] = self::PUBLIC; - } - - // If its a post that originated here then tag the thread as "mention" - if ($item['origin'] && $item['uid']) { - DBA::update('thread', ['mention' => true], ['iid' => $item['parent']]); - Logger::info('tagged thread as mention', ['parent' => $item['parent'], 'uid' => $item['uid']]); - } - - // Update the contact relations - Contact\Relation::store($parent['author-id'], $item['author-id'], $item['created']); } - return $item; + if ($parent['uri'] == $parent['parent-uri']) { + return $parent; + } + + $condition = ['uri' => $item['parent-uri'], + 'parent-uri' => $item['parent-uri'], + 'uid' => $item['uid']]; + // We select wall = 1 in priority for top level permission checks + $params = ['order' => ['wall' => true]]; + $toplevel_parent = self::selectFirst($fields, $condition, $params); + + if (!DBA::isResult($toplevel_parent)) { + Logger::info('item parent was not found - ignoring item', ['parent-uri' => $item['parent-uri'], 'uid' => $item['uid']]); + return []; + } + + return $toplevel_parent; } /** @@ -1636,13 +1591,14 @@ class Item // Store URI data $item['uri-id'] = ItemURI::insert(['uri' => $item['uri'], 'guid' => $item['guid']]); + // Backward compatibility: parent-uri used to be the direct parent uri. + // If it is provided without a thr-parent, it probably is the old behavior. + $item['thr-parent'] = trim($item['thr-parent'] ?? $item['parent-uri'] ?? $item['uri']); + $item['parent-uri'] = $item['thr-parent']; + // Store conversation data $item = Conversation::insert($item); - if (!empty($item['thr-parent'])) { - $item['parent-uri'] = $item['thr-parent']; - } - /* * Do we already have this item? * We have to check several networks since Friendica posts could be repeated @@ -1740,6 +1696,62 @@ class Item return 0; } + if ($item['thr-parent'] != $item['uri']) { + $toplevel_parent = self::getTopLevelParent($item); + if (empty($toplevel_parent)) { + return 0; + } + + $parent_id = $toplevel_parent['id']; + $item['parent-uri'] = $toplevel_parent['uri']; + $item['deleted'] = $toplevel_parent['deleted']; + $item['allow_cid'] = $toplevel_parent['allow_cid']; + $item['allow_gid'] = $toplevel_parent['allow_gid']; + $item['deny_cid'] = $toplevel_parent['deny_cid']; + $item['deny_gid'] = $toplevel_parent['deny_gid']; + $parent_origin = $toplevel_parent['origin']; + + // Don't federate received participation messages + if ($item['verb'] != Activity::FOLLOW) { + $item['wall'] = $toplevel_parent['wall']; + } else { + $item['wall'] = false; + } + + /* + * If the parent is private, force privacy for the entire conversation + * This differs from the above settings as it subtly allows comments from + * email correspondents to be private even if the overall thread is not. + */ + if ($toplevel_parent['private']) { + $item['private'] = $toplevel_parent['private']; + } + + /* + * Edge case. We host a public forum that was originally posted to privately. + * The original author commented, but as this is a comment, the permissions + * weren't fixed up so it will still show the comment as private unless we fix it here. + */ + if ((intval($toplevel_parent['forum_mode']) == 1) && ($toplevel_parent['private'] != self::PUBLIC)) { + $item['private'] = self::PUBLIC; + } + + // If its a post that originated here then tag the thread as "mention" + if ($item['origin'] && $item['uid']) { + DBA::update('thread', ['mention' => true], ['iid' => $parent_id]); + Logger::info('tagged thread as mention', ['parent' => $parent_id, 'uid' => $item['uid']]); + } + + // Update the contact relations + Contact\Relation::store($toplevel_parent['author-id'], $item['author-id'], $item['created']); + + unset($item['parent']); + unset($item['parent_origin']); + } else { + $parent_id = 0; + $parent_origin = $item['origin']; + } + // We don't store the causer link, only the id unset($item['causer-link']); @@ -1753,23 +1765,6 @@ class Item unset($item['owner-name']); unset($item['owner-avatar']); - $item['thr-parent'] = $item['parent-uri']; - - if ($item['parent-uri'] != $item['uri']) { - $item = self::getParentData($item); - if (empty($item)) { - return 0; - } - - $parent_id = $item['parent']; - unset($item['parent']); - $parent_origin = $item['parent_origin']; - unset($item['parent_origin']); - } else { - $parent_id = 0; - $parent_origin = $item['origin']; - } - $item['parent-uri-id'] = ItemURI::getIdByURI($item['parent-uri']); $item['thr-parent-id'] = ItemURI::getIdByURI($item['thr-parent']); From 0c3a5c815e7067aa587ac8f0bb8e32bb5892082e Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 9 Nov 2020 11:13:18 -0500 Subject: [PATCH 02/18] Remove obsolete references to item.parent-uri --- include/api.php | 1 - mod/photos.php | 3 --- src/Model/Event.php | 1 - src/Model/Item.php | 7 +++---- src/Module/Contact/Poke.php | 1 - src/Worker/OnePoll.php | 1 + 6 files changed, 4 insertions(+), 10 deletions(-) diff --git a/include/api.php b/include/api.php index 87ce982b0..5969ccd5f 100644 --- a/include/api.php +++ b/include/api.php @@ -4766,7 +4766,6 @@ function post_photo_item($hash, $allow_cid, $deny_cid, $allow_gid, $deny_gid, $f $arr['guid'] = System::createUUID(); $arr['uid'] = intval(api_user()); $arr['uri'] = $uri; - $arr['parent-uri'] = $uri; $arr['type'] = 'photo'; $arr['wall'] = 1; $arr['resource-id'] = $hash; diff --git a/mod/photos.php b/mod/photos.php index bba12aace..e94a3ed8d 100644 --- a/mod/photos.php +++ b/mod/photos.php @@ -395,7 +395,6 @@ function photos_post(App $a) $arr['guid'] = System::createUUID(); $arr['uid'] = $page_owner_uid; $arr['uri'] = $uri; - $arr['parent-uri'] = $uri; $arr['post-type'] = Item::PT_IMAGE; $arr['wall'] = 1; $arr['resource-id'] = $photo['resource-id']; @@ -560,7 +559,6 @@ function photos_post(App $a) $arr['guid'] = System::createUUID(); $arr['uid'] = $page_owner_uid; $arr['uri'] = $uri; - $arr['parent-uri'] = $uri; $arr['wall'] = 1; $arr['contact-id'] = $owner_record['id']; $arr['owner-name'] = $owner_record['name']; @@ -791,7 +789,6 @@ function photos_post(App $a) $arr['guid'] = System::createUUID(); $arr['uid'] = $page_owner_uid; $arr['uri'] = $uri; - $arr['parent-uri'] = $uri; $arr['type'] = 'photo'; $arr['wall'] = 1; $arr['resource-id'] = $resource_id; diff --git a/src/Model/Event.php b/src/Model/Event.php index 6f8ed123c..5139abc43 100644 --- a/src/Model/Event.php +++ b/src/Model/Event.php @@ -347,7 +347,6 @@ class Event $item_arr['uid'] = $event['uid']; $item_arr['contact-id'] = $event['cid']; $item_arr['uri'] = $event['uri']; - $item_arr['parent-uri'] = $event['uri']; $item_arr['guid'] = $event['guid']; $item_arr['plink'] = $arr['plink'] ?? ''; $item_arr['post-type'] = Item::PT_EVENT; diff --git a/src/Model/Item.php b/src/Model/Item.php index c03722372..e67b86234 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1633,7 +1633,6 @@ class Item $item['coord'] = trim($item['coord'] ?? ''); $item['visible'] = (isset($item['visible']) ? intval($item['visible']) : 1); $item['deleted'] = 0; - $item['parent-uri'] = trim(($item['parent-uri'] ?? '') ?: $item['uri']); $item['post-type'] = ($item['post-type'] ?? '') ?: self::PT_ARTICLE; $item['verb'] = trim($item['verb'] ?? ''); $item['object-type'] = trim($item['object-type'] ?? ''); @@ -2858,14 +2857,15 @@ class Item unset($datarray["plink"]); $datarray["uri"] = self::newURI($contact['uid'], $datarray["guid"]); $datarray["uri-id"] = ItemURI::getIdByURI($datarray["uri"]); - $datarray["parent-uri"] = $datarray["uri"]; - $datarray["thr-parent"] = $datarray["uri"]; $datarray["extid"] = Protocol::DFRN; $urlpart = parse_url($datarray2['author-link']); $datarray["app"] = $urlpart["host"]; if (!empty($old_uri_id)) { Post\Media::copy($old_uri_id, $datarray["uri-id"]); } + + unset($datarray["parent-uri"]); + unset($datarray["thr-parent"]); } else { $datarray['private'] = self::PUBLIC; } @@ -3297,7 +3297,6 @@ class Item 'network' => Protocol::DFRN, 'gravity' => GRAVITY_ACTIVITY, 'parent' => $item['id'], - 'parent-uri' => $item['uri'], 'thr-parent' => $item['uri'], 'owner-id' => $author_id, 'author-id' => $author_id, diff --git a/src/Module/Contact/Poke.php b/src/Module/Contact/Poke.php index 9f2ae7bde..d9fab36c0 100644 --- a/src/Module/Contact/Poke.php +++ b/src/Module/Contact/Poke.php @@ -67,7 +67,6 @@ class Poke extends BaseModule $arr['guid'] = System::createUUID(); $arr['uid'] = $uid; $arr['uri'] = $uri; - $arr['parent-uri'] = $uri; $arr['wall'] = 1; $arr['contact-id'] = $actor['id']; $arr['owner-name'] = $actor['name']; diff --git a/src/Worker/OnePoll.php b/src/Worker/OnePoll.php index 93dff93ae..8f1e94580 100644 --- a/src/Worker/OnePoll.php +++ b/src/Worker/OnePoll.php @@ -625,6 +625,7 @@ class OnePoll if (empty($datarray['parent-uri'])) { $datarray['parent-uri'] = $datarray['uri']; } + unset($datarray['parent-uri']); $headers = imap_headerinfo($mbox, $meta->msgno); From 0f2a5daf09b67c85b8c61b3db346697c612edf61 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 02:47:48 -0500 Subject: [PATCH 03/18] Replace confusing uses of item.parent-uri with expected item.thr-parent --- mod/item.php | 5 ++- mod/tagger.php | 2 +- src/Protocol/DFRN.php | 39 ++++++++++++------------ src/Protocol/Diaspora.php | 64 ++++++++++++++++++--------------------- src/Protocol/Feed.php | 8 ++--- src/Protocol/OStatus.php | 31 +++++++++---------- src/Worker/OnePoll.php | 6 ++-- 7 files changed, 74 insertions(+), 81 deletions(-) diff --git a/mod/item.php b/mod/item.php index 9828d1acb..3869ea910 100644 --- a/mod/item.php +++ b/mod/item.php @@ -605,8 +605,7 @@ function item_post(App $a) { $datarray['pubmail'] = $pubmail_enabled; $datarray['attach'] = $attachments; - // This is not a bug. The item store function changes 'parent-uri' to 'thr-parent' and fetches 'parent-uri' new. (We should change this) - $datarray['parent-uri'] = $thr_parent_uri; + $datarray['thr-parent'] = $thr_parent_uri; $datarray['postopts'] = $postopts; $datarray['origin'] = $origin; @@ -627,7 +626,7 @@ function item_post(App $a) { // This field is for storing the raw conversation data $datarray['protocol'] = Conversation::PARCEL_DFRN; - $conversation = DBA::selectFirst('conversation', ['conversation-uri', 'conversation-href'], ['item-uri' => $datarray['parent-uri']]); + $conversation = DBA::selectFirst('conversation', ['conversation-uri', 'conversation-href'], ['item-uri' => $datarray['thr-parent']]); if (DBA::isResult($conversation)) { if ($conversation['conversation-uri'] != '') { $datarray['conversation-uri'] = $conversation['conversation-uri']; diff --git a/mod/tagger.php b/mod/tagger.php index 86a6ff69f..63e7f2ca8 100644 --- a/mod/tagger.php +++ b/mod/tagger.php @@ -136,7 +136,7 @@ EOT; $arr['wall'] = $item['wall']; $arr['gravity'] = GRAVITY_COMMENT; $arr['parent'] = $item['id']; - $arr['parent-uri'] = $item['uri']; + $arr['thr-parent'] = $item['uri']; $arr['owner-name'] = $item['author-name']; $arr['owner-link'] = $item['author-link']; $arr['owner-avatar'] = $item['author-avatar']; diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index 11c4e3a14..165a63c7c 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -438,7 +438,7 @@ class DFRN $mail->appendChild($sender); XML::addElement($doc, $mail, "dfrn:id", $item['uri']); - XML::addElement($doc, $mail, "dfrn:in-reply-to", $item['parent-uri']); + XML::addElement($doc, $mail, "dfrn:in-reply-to", $item['thr-parent']); XML::addElement($doc, $mail, "dfrn:sentdate", DateTimeFormat::utc($item['created'] . '+00:00', DateTimeFormat::ATOM)); XML::addElement($doc, $mail, "dfrn:subject", $item['title']); XML::addElement($doc, $mail, "dfrn:content", $item['body']); @@ -956,10 +956,9 @@ class DFRN $entry->appendChild($dfrnowner); if ($item['gravity'] != GRAVITY_PARENT) { - $parent_item = (($item['thr-parent']) ? $item['thr-parent'] : $item['parent-uri']); - $parent = Item::selectFirst(['guid', 'plink'], ['uri' => $parent_item, 'uid' => $item['uid']]); + $parent = Item::selectFirst(['guid', 'plink'], ['uri' => $item['thr-parent'], 'uid' => $item['uid']]); if (DBA::isResult($parent)) { - $attributes = ["ref" => $parent_item, "type" => "text/html", + $attributes = ["ref" => $item['thr-parent'], "type" => "text/html", "href" => $parent['plink'], "dfrn:diaspora_guid" => $parent['guid']]; XML::addElement($doc, $entry, "thr:in-reply-to", "", $attributes); @@ -971,7 +970,7 @@ class DFRN $conversation_uri = $conversation_href; if (isset($parent_item)) { - $conversation = DBA::selectFirst('conversation', ['conversation-uri', 'conversation-href'], ['item-uri' => $item['parent-uri']]); + $conversation = DBA::selectFirst('conversation', ['conversation-uri', 'conversation-href'], ['item-uri' => $item['thr-parent']]); if (DBA::isResult($conversation)) { if ($conversation['conversation-uri'] != '') { $conversation_uri = $conversation['conversation-uri']; @@ -1768,7 +1767,7 @@ class DFRN $msg["from-photo"] = XML::getFirstValue($xpath, "dfrn:sender/dfrn:avatar/text()", $mail); $msg["contact-id"] = $importer["id"]; $msg["uri"] = XML::getFirstValue($xpath, "dfrn:id/text()", $mail); - $msg["parent-uri"] = XML::getFirstValue($xpath, "dfrn:in-reply-to/text()", $mail); + $msg["thr-parent"] = XML::getFirstValue($xpath, "dfrn:in-reply-to/text()", $mail); $msg["created"] = DateTimeFormat::utc(XML::getFirstValue($xpath, "dfrn:sentdate/text()", $mail)); $msg["title"] = XML::getFirstValue($xpath, "dfrn:subject/text()", $mail); $msg["body"] = XML::getFirstValue($xpath, "dfrn:content/text()", $mail); @@ -1918,7 +1917,7 @@ class DFRN */ private static function getEntryType($importer, $item) { - if ($item["parent-uri"] != $item["uri"]) { + if ($item["thr-parent"] != $item["uri"]) { $community = false; if ($importer["page-flags"] == User::PAGE_FLAGS_COMMUNITY || $importer["page-flags"] == User::PAGE_FLAGS_PRVGROUP) { @@ -1934,18 +1933,18 @@ class DFRN $is_a_remote_action = false; - $parent = Item::selectFirst(['parent-uri'], ['uri' => $item["parent-uri"]]); + $parent = Item::selectFirst(['thr-parent'], ['uri' => $item["thr-parent"]]); if (DBA::isResult($parent)) { $r = q( "SELECT `item`.`forum_mode`, `item`.`wall` FROM `item` INNER JOIN `contact` ON `contact`.`id` = `item`.`contact-id` - WHERE `item`.`uri` = '%s' AND (`item`.`parent-uri` = '%s' OR `item`.`thr-parent` = '%s') + WHERE `item`.`uri` = '%s' AND (`item`.`thr-parent` = '%s' OR `item`.`thr-parent` = '%s') AND `item`.`uid` = %d $sql_extra LIMIT 1", - DBA::escape($parent["parent-uri"]), - DBA::escape($parent["parent-uri"]), - DBA::escape($parent["parent-uri"]), + DBA::escape($parent["thr-parent"]), + DBA::escape($parent["thr-parent"]), + DBA::escape($parent["thr-parent"]), intval($importer["importer_uid"]) ); if (DBA::isResult($r)) { @@ -2008,7 +2007,7 @@ class DFRN if ($Blink && Strings::compareLink($Blink, DI::baseUrl() . "/profile/" . $importer["nickname"])) { $author = DBA::selectFirst('contact', ['name', 'thumb', 'url'], ['id' => $item['author-id']]); - $parent = Item::selectFirst(['id'], ['uri' => $item['parent-uri'], 'uid' => $importer["importer_uid"]]); + $parent = Item::selectFirst(['id'], ['uri' => $item['thr-parent'], 'uid' => $importer["importer_uid"]]); $item['parent'] = $parent['id']; // send a notification @@ -2090,15 +2089,15 @@ class DFRN $is_like = true; $item["gravity"] = GRAVITY_ACTIVITY; // only one like or dislike per person - // splitted into two queries for performance issues + // split into two queries for performance issues $condition = ['uid' => $item["uid"], 'author-id' => $item["author-id"], 'gravity' => GRAVITY_ACTIVITY, - 'verb' => $item["verb"], 'parent-uri' => $item["parent-uri"]]; + 'verb' => $item['verb'], 'parent-uri' => $item['thr-parent']]; if (Item::exists($condition)) { return false; } $condition = ['uid' => $item["uid"], 'author-id' => $item["author-id"], 'gravity' => GRAVITY_ACTIVITY, - 'verb' => $item["verb"], 'thr-parent' => $item["parent-uri"]]; + 'verb' => $item['verb'], 'thr-parent' => $item['thr-parent']]; if (Item::exists($condition)) { return false; } @@ -2370,19 +2369,19 @@ class DFRN } // Is it a reply or a top level posting? - $item["parent-uri"] = $item["uri"]; + $item['thr-parent'] = $item['uri']; $inreplyto = $xpath->query("thr:in-reply-to", $entry); if (is_object($inreplyto->item(0))) { foreach ($inreplyto->item(0)->attributes as $attributes) { if ($attributes->name == "ref") { - $item["parent-uri"] = $attributes->textContent; + $item['thr-parent'] = $attributes->textContent; } } } // Check if the message is wanted - if (($importer["importer_uid"] == 0) && ($item['uri'] == $item['parent-uri'])) { + if (($importer['importer_uid'] == 0) && ($item['uri'] == $item['thr-parent'])) { if (!self::isSolicitedMessage($item)) { DBA::delete('item-uri', ['uri' => $item['uri']]); return 403; @@ -2760,7 +2759,7 @@ class DFRN // check that the message originated elsewhere and is a top-level post - if ($item['wall'] || $item['origin'] || ($item['uri'] != $item['parent-uri'])) { + if ($item['wall'] || $item['origin'] || ($item['uri'] != $item['thr-parent'])) { return false; } diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index 8c93d7c0f..04d076c03 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -1697,9 +1697,9 @@ class Diaspora if (isset($data->thread_parent_guid)) { $thread_parent_guid = Strings::escapeTags(XML::unescape($data->thread_parent_guid)); - $thr_uri = self::getUriFromGuid("", $thread_parent_guid, true); + $thr_parent = self::getUriFromGuid("", $thread_parent_guid, true); } else { - $thr_uri = ""; + $thr_parent = ""; } $contact = self::allowedContactByHandle($importer, $sender, true); @@ -1712,8 +1712,8 @@ class Diaspora return true; } - $parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); - if (!$parent_item) { + $toplevel_parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); + if (!$toplevel_parent_item) { return false; } @@ -1754,11 +1754,8 @@ class Diaspora $datarray["verb"] = Activity::POST; $datarray["gravity"] = GRAVITY_COMMENT; - if ($thr_uri != "") { - $datarray["parent-uri"] = $thr_uri; - } else { - $datarray["parent-uri"] = $parent_item["uri"]; - } + $datarray['thr-parent'] = $thr_parent ?: $toplevel_parent_item['uri']; + $datarray['parent-uri'] = $toplevel_parent_item['uri']; $datarray["object-type"] = Activity\ObjectType::COMMENT; @@ -1767,7 +1764,7 @@ class Diaspora $datarray["changed"] = $datarray["created"] = $datarray["edited"] = $created_at; - $datarray["plink"] = self::plink($author, $guid, $parent_item['guid']); + $datarray["plink"] = self::plink($author, $guid, $toplevel_parent_item['guid']); $body = Markdown::toBBCode($text); $datarray["body"] = self::replacePeopleGuid($body, $person["url"]); @@ -1779,7 +1776,7 @@ class Diaspora // If we are the origin of the parent we store the original data. // We notify our followers during the item storage. - if ($parent_item["origin"]) { + if ($toplevel_parent_item["origin"]) { $datarray['diaspora_signed_text'] = json_encode($data); } @@ -1953,8 +1950,8 @@ class Diaspora return true; } - $parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); - if (!$parent_item) { + $toplevel_parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); + if (!$toplevel_parent_item) { return false; } @@ -1991,7 +1988,7 @@ class Diaspora $datarray["verb"] = $verb; $datarray["gravity"] = GRAVITY_ACTIVITY; - $datarray["parent-uri"] = $parent_item["uri"]; + $datarray['thr-parent'] = $toplevel_parent_item['uri']; $datarray["object-type"] = Activity\ObjectType::NOTE; @@ -2001,11 +1998,11 @@ class Diaspora $datarray["changed"] = $datarray["created"] = $datarray["edited"] = DateTimeFormat::utcNow(); // like on comments have the comment as parent. So we need to fetch the toplevel parent - if ($parent_item['gravity'] != GRAVITY_PARENT) { - $toplevel = Item::selectFirst(['origin'], ['id' => $parent_item['parent']]); + if ($toplevel_parent_item['gravity'] != GRAVITY_PARENT) { + $toplevel = Item::selectFirst(['origin'], ['id' => $toplevel_parent_item['parent']]); $origin = $toplevel["origin"]; } else { - $origin = $parent_item["origin"]; + $origin = $toplevel_parent_item["origin"]; } // If we are the origin of the parent we store the original data. @@ -2116,16 +2113,16 @@ class Diaspora return true; } - $parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); - if (!$parent_item) { + $toplevel_parent_item = self::parentItem($importer["uid"], $parent_guid, $author, $contact); + if (!$toplevel_parent_item) { return false; } - if (!$parent_item['origin']) { + if (!$toplevel_parent_item['origin']) { Logger::info('Not our origin. Participation is ignored', ['parent_guid' => $parent_guid, 'guid' => $guid, 'author' => $author]); } - if (!in_array($parent_item['private'], [Item::PUBLIC, Item::UNLISTED])) { + if (!in_array($toplevel_parent_item['private'], [Item::PUBLIC, Item::UNLISTED])) { Logger::info('Item is not public, participation is ignored', ['parent_guid' => $parent_guid, 'guid' => $guid, 'author' => $author]); return false; } @@ -2155,7 +2152,8 @@ class Diaspora $datarray["verb"] = Activity::FOLLOW; $datarray["gravity"] = GRAVITY_ACTIVITY; - $datarray["parent-uri"] = $parent_item["uri"]; + $datarray['thr-parent'] = $toplevel_parent_item['uri']; + $datarray['parent-uri'] = $toplevel_parent_item['parent-uri']; $datarray["object-type"] = Activity\ObjectType::NOTE; @@ -2170,7 +2168,7 @@ class Diaspora // Send all existing comments and likes to the requesting server $comments = Item::select(['id', 'uri-id', 'parent-author-network', 'author-network', 'verb'], - ['parent' => $parent_item['id'], 'gravity' => [GRAVITY_COMMENT, GRAVITY_ACTIVITY]]); + ['parent' => $toplevel_parent_item['id'], 'gravity' => [GRAVITY_COMMENT, GRAVITY_ACTIVITY]]); while ($comment = Item::fetch($comments)) { if (in_array($comment['verb'], [Activity::FOLLOW, Activity::TAG])) { Logger::info('participation messages are not relayed', ['item' => $comment['id']]); @@ -2547,7 +2545,8 @@ class Diaspora $datarray['guid'] = $parent['guid'] . '-' . $guid; $datarray['uri'] = self::getUriFromGuid($author, $datarray['guid']); - $datarray['parent-uri'] = $parent['uri']; + $datarray['thr-parent'] = $parent['uri']; + $datarray['parent-uri'] = $parent['parent-uri']; $datarray['verb'] = $datarray['body'] = Activity::ANNOUNCE; $datarray['gravity'] = GRAVITY_ACTIVITY; @@ -2618,7 +2617,7 @@ class Diaspora $datarray["owner-id"] = $datarray["author-id"]; $datarray["guid"] = $guid; - $datarray["uri"] = $datarray["parent-uri"] = self::getUriFromGuid($author, $guid); + $datarray["uri"] = $datarray["thr-parent"] = self::getUriFromGuid($author, $guid); $datarray['uri-id'] = ItemURI::insert(['uri' => $datarray['uri'], 'guid' => $datarray['guid']]); $datarray["verb"] = Activity::POST; @@ -2703,7 +2702,7 @@ class Diaspora } // Fetch items that are about to be deleted - $fields = ['uid', 'id', 'parent', 'parent-uri', 'author-link', 'file']; + $fields = ['uid', 'id', 'parent', 'author-link', 'file']; // When we receive a public retraction, we delete every item that we find. if ($importer['uid'] == 0) { @@ -2872,7 +2871,7 @@ class Diaspora $datarray = []; $datarray["guid"] = $guid; - $datarray["uri"] = $datarray["parent-uri"] = self::getUriFromGuid($author, $guid); + $datarray["uri"] = $datarray["thr-parent"] = self::getUriFromGuid($author, $guid); $datarray['uri-id'] = ItemURI::insert(['uri' => $datarray['uri'], 'guid' => $datarray['guid']]); // Attach embedded pictures to the body @@ -3681,12 +3680,12 @@ class Diaspora */ private static function constructLike(array $item, array $owner) { - $parent = Item::selectFirst(['guid', 'uri', 'parent-uri'], ['uri' => $item["thr-parent"]]); + $parent = Item::selectFirst(['guid', 'uri', 'thr-parent'], ['uri' => $item["thr-parent"]]); if (!DBA::isResult($parent)) { return false; } - $target_type = ($parent["uri"] === $parent["parent-uri"] ? "Post" : "Comment"); + $target_type = ($parent["uri"] === $parent["thr-parent"] ? "Post" : "Comment"); $positive = null; if ($item['verb'] === Activity::LIKE) { $positive = "true"; @@ -3713,7 +3712,7 @@ class Diaspora */ private static function constructAttend(array $item, array $owner) { - $parent = Item::selectFirst(['guid', 'uri', 'parent-uri'], ['uri' => $item["thr-parent"]]); + $parent = Item::selectFirst(['guid'], ['uri' => $item['thr-parent']]); if (!DBA::isResult($parent)) { return false; } @@ -4242,10 +4241,7 @@ class Diaspora return false; } - // This is a workaround for the behaviour of the "insert" function, see mod/item.php - $item['thr-parent'] = $item['parent-uri']; - - $parent = Item::selectFirst(['parent-uri'], ['uri' => $item['parent-uri']]); + $parent = Item::selectFirst(['parent-uri'], ['uri' => $item['thr-parent']]); if (!DBA::isResult($parent)) { return; } diff --git a/src/Protocol/Feed.php b/src/Protocol/Feed.php index fee765f25..c7d19a996 100644 --- a/src/Protocol/Feed.php +++ b/src/Protocol/Feed.php @@ -354,7 +354,7 @@ class Feed $item["plink"] = DI::httpRequest()->finalUrl($item["plink"]); - $item["parent-uri"] = $item["uri"]; + $item["thr-parent"] = $item["uri"]; $item["title"] = XML::getFirstNodeValue($xpath, 'atom:title/text()', $entry); @@ -602,6 +602,7 @@ class Feed if ($notify) { $item['guid'] = Item::guidFromUri($orig_plink, DI::baseUrl()->getHostname()); unset($item['uri']); + unset($item['thr-parent']); unset($item['parent-uri']); // Set the delivery priority for "remote self" to "medium" @@ -1108,9 +1109,8 @@ class Feed if ($item['gravity'] != GRAVITY_PARENT) { $parent = Item::selectFirst(['guid', 'author-link', 'owner-link'], ['id' => $item['parent']]); - $parent_item = (($item['thr-parent']) ? $item['thr-parent'] : $item['parent-uri']); - $thrparent = Item::selectFirst(['guid', 'author-link', 'owner-link', 'plink'], ['uid' => $owner["uid"], 'uri' => $parent_item]); + $thrparent = Item::selectFirst(['guid', 'author-link', 'owner-link', 'plink'], ['uid' => $owner["uid"], 'uri' => $item['thr-parent']]); if (DBA::isResult($thrparent)) { $mentioned[$thrparent["author-link"]] = $thrparent["author-link"]; @@ -1123,7 +1123,7 @@ class Feed } $attributes = [ - "ref" => $parent_item, + "ref" => $item['thr-parent'], "href" => $parent_plink]; XML::addElement($doc, $entry, "thr:in-reply-to", "", $attributes); diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index e394877b3..9218f2665 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -482,7 +482,7 @@ class OStatus Logger::log("Favorite ".$orig_uri." ".print_r($item, true)); $item["verb"] = Activity::LIKE; - $item["parent-uri"] = $orig_uri; + $item["thr-parent"] = $orig_uri; $item["gravity"] = GRAVITY_ACTIVITY; $item["object-type"] = Activity\ObjectType::NOTE; } @@ -495,7 +495,7 @@ class OStatus self::processPost($xpath, $entry, $item, $importer); if ($initialize && (count(self::$itemlist) > 0)) { - if (self::$itemlist[0]['uri'] == self::$itemlist[0]['parent-uri']) { + if (self::$itemlist[0]['uri'] == self::$itemlist[0]['thr-parent']) { // We will import it everytime, when it is started by our contacts $valid = Contact::isSharingByURL(self::$itemlist[0]['author-link'], self::$itemlist[0]['uid']); @@ -523,9 +523,9 @@ class OStatus } } else { // But we will only import complete threads - $valid = Item::exists(['uid' => $importer["uid"], 'uri' => self::$itemlist[0]['parent-uri']]); + $valid = Item::exists(['uid' => $importer["uid"], 'uri' => self::$itemlist[0]['thr-parent']]); if ($valid) { - Logger::log("Item with uri ".self::$itemlist[0]["uri"]." belongs to parent ".self::$itemlist[0]['parent-uri']." of user ".$importer["uid"].". It will be imported.", Logger::DEBUG); + Logger::log("Item with uri ".self::$itemlist[0]["uri"]." belongs to parent ".self::$itemlist[0]['thr-parent']." of user ".$importer["uid"].". It will be imported.", Logger::DEBUG); } } @@ -621,7 +621,7 @@ class OStatus if (is_object($inreplyto->item(0))) { foreach ($inreplyto->item(0)->attributes as $attributes) { if ($attributes->name == "ref") { - $item["parent-uri"] = $attributes->textContent; + $item["thr-parent"] = $attributes->textContent; } if ($attributes->name == "href") { $related = $attributes->textContent; @@ -702,16 +702,16 @@ class OStatus self::fetchConversation($item['conversation-href'], $item['conversation-uri']); } - if (isset($item["parent-uri"])) { - if (!Item::exists(['uid' => $importer["uid"], 'uri' => $item['parent-uri']])) { + if (isset($item["thr-parent"])) { + if (!Item::exists(['uid' => $importer["uid"], 'uri' => $item['thr-parent']])) { if ($related != '') { - self::fetchRelated($related, $item["parent-uri"], $importer); + self::fetchRelated($related, $item["thr-parent"], $importer); } } else { Logger::log('Reply with URI '.$item["uri"].' already existed for user '.$importer["uid"].'.', Logger::DEBUG); } } else { - $item["parent-uri"] = $item["uri"]; + $item["thr-parent"] = $item["uri"]; $item["gravity"] = GRAVITY_PARENT; } @@ -1074,7 +1074,7 @@ class OStatus if (is_object($inreplyto->item(0))) { foreach ($inreplyto->item(0)->attributes as $attributes) { if ($attributes->name == "ref") { - $item["parent-uri"] = $attributes->textContent; + $item["thr-parent"] = $attributes->textContent; } } } @@ -1126,8 +1126,8 @@ class OStatus break; case "related": if ($item["object-type"] != Activity\ObjectType::BOOKMARK) { - if (!isset($item["parent-uri"])) { - $item["parent-uri"] = $attribute['href']; + if (!isset($item["thr-parent"])) { + $item["thr-parent"] = $attribute['href']; } $link_data['related'] = $attribute['href']; } else { @@ -1934,9 +1934,8 @@ class OStatus if ($item['gravity'] != GRAVITY_PARENT) { $parent = Item::selectFirst(['guid', 'author-link', 'owner-link'], ['id' => $item['parent']]); - $parent_item = (($item['thr-parent']) ? $item['thr-parent'] : $item['parent-uri']); - $thrparent = Item::selectFirst(['guid', 'author-link', 'owner-link', 'plink'], ['uid' => $owner["uid"], 'uri' => $parent_item]); + $thrparent = Item::selectFirst(['guid', 'author-link', 'owner-link', 'plink'], ['uid' => $owner["uid"], 'uri' => $item['thr-parent']]); if (DBA::isResult($thrparent)) { $mentioned[$thrparent["author-link"]] = $thrparent["author-link"]; @@ -1949,7 +1948,7 @@ class OStatus } $attributes = [ - "ref" => $parent_item, + "ref" => $item['thr-parent'], "href" => $parent_plink]; XML::addElement($doc, $entry, "thr:in-reply-to", "", $attributes); @@ -1960,7 +1959,7 @@ class OStatus } if (intval($item['parent']) > 0) { - $conversation_href = $conversation_uri = str_replace('/objects/', '/context/', $item['parent-uri']); + $conversation_href = $conversation_uri = str_replace('/objects/', '/context/', $item['thr-parent']); if (isset($parent_item)) { $conversation = DBA::selectFirst('conversation', ['conversation-uri', 'conversation-href'], ['item-uri' => $parent_item]); diff --git a/src/Worker/OnePoll.php b/src/Worker/OnePoll.php index 8f1e94580..bdef1ac4d 100644 --- a/src/Worker/OnePoll.php +++ b/src/Worker/OnePoll.php @@ -622,8 +622,8 @@ class OnePoll } } - if (empty($datarray['parent-uri'])) { - $datarray['parent-uri'] = $datarray['uri']; + if (!empty($datarray['parent-uri'])) { + $datarray['thr-parent'] = $datarray['parent-uri']; } unset($datarray['parent-uri']); @@ -664,7 +664,7 @@ class OnePoll $datarray['owner-link'] = "mailto:".$contact['addr']; $datarray['owner-avatar'] = $contact['photo']; - if ($datarray['parent-uri'] === $datarray['uri']) { + if ($datarray['thr-parent'] === $datarray['uri']) { $datarray['private'] = Item::PRIVATE; } From 5e76def1ff1269b4e21297f5ce18e58f60996b64 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 02:48:52 -0500 Subject: [PATCH 04/18] Clarify item.parent-uri use in database field comment --- static/dbstructure.config.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index 9fb0c9911..d1da02412 100644 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -709,8 +709,8 @@ return [ "uri-id" => ["type" => "int unsigned", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table entry that contains the item uri"], "uri-hash" => ["type" => "varchar(80)", "not null" => "1", "default" => "", "comment" => "RIPEMD-128 hash from uri"], "parent" => ["type" => "int unsigned", "not null" => "1", "default" => "0", "relation" => ["item" => "id"], "comment" => "item.id of the parent to this item if it is a reply of some form; otherwise this must be set to the id of this item"], - "parent-uri" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "uri of the parent to this item"], - "parent-uri-id" => ["type" => "int unsigned", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table that contains the parent uri"], + "parent-uri" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "uri of the top-level parent to this item"], + "parent-uri-id" => ["type" => "int unsigned", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table that contains the top-level parent uri"], "thr-parent" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "If the parent of this item is not the top-level item in the conversation, the uri of the immediate parent; otherwise set to parent-uri"], "thr-parent-id" => ["type" => "int unsigned", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table that contains the thread parent uri"], "created" => ["type" => "datetime", "not null" => "1", "default" => DBA::NULL_DATETIME, "comment" => "Creation timestamp."], From ffc364f2a4c8ed14c28fa01061d11c4f1a28af06 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 02:50:22 -0500 Subject: [PATCH 05/18] Reject replies when author is blocked by thread owner in Model\Item::insert - Move user-level item permission to Model\Item::isAllowedByUser - Add user-level check for comments on top-level item --- src/Model/Contact/User.php | 4 +-- src/Model/Item.php | 63 +++++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/Model/Contact/User.php b/src/Model/Contact/User.php index 34a3d6f34..be60c119b 100644 --- a/src/Model/Contact/User.php +++ b/src/Model/Contact/User.php @@ -64,7 +64,7 @@ class User { $cdata = Contact::getPublicAndUserContacID($cid, $uid); if (empty($cdata)) { - return; + return false; } $public_blocked = false; @@ -127,7 +127,7 @@ class User { $cdata = Contact::getPublicAndUserContacID($cid, $uid); if (empty($cdata)) { - return; + return false; } $public_ignored = false; diff --git a/src/Model/Item.php b/src/Model/Item.php index e67b86234..3239c4a8f 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1415,11 +1415,6 @@ class Item return false; } - if (!empty($item['uid']) && !empty($item['author-id']) && Contact\User::isBlocked($item['author-id'], $item['uid'])) { - Logger::notice('Author is blocked by user', ['author-link' => $item['author-link'], 'uid' => $item['uid'], 'item-uri' => $item['uri']]); - return false; - } - if (!empty($item['owner-id']) && Contact::isBlocked($item['owner-id'])) { Logger::notice('Owner is blocked node-wide', ['owner-link' => $item['owner-link'], 'item-uri' => $item['uri']]); return false; @@ -1430,22 +1425,10 @@ class Item return false; } - if (!empty($item['uid']) && !empty($item['owner-id']) && Contact\User::isBlocked($item['owner-id'], $item['uid'])) { - Logger::notice('Owner is blocked by user', ['owner-link' => $item['owner-link'], 'uid' => $item['uid'], 'item-uri' => $item['uri']]); + if (!empty($item['uid']) && !self::isAllowedByUser($item, $item['uid'])) { return false; } - // The causer is set during a thread completion, for example because of a reshare. It countains the responsible actor. - if (!empty($item['uid']) && !empty($item['causer-id']) && Contact\User::isBlocked($item['causer-id'], $item['uid'])) { - Logger::notice('Causer is blocked by user', ['causer-link' => $item['causer-link'], 'uid' => $item['uid'], 'item-uri' => $item['uri']]); - return false; - } - - if (!empty($item['uid']) && !empty($item['causer-id']) && ($item['parent-uri'] == $item['uri']) && Contact\User::isIgnored($item['causer-id'], $item['uid'])) { - Logger::notice('Causer is ignored by user', ['causer-link' => $item['causer-link'], 'uid' => $item['uid'], 'item-uri' => $item['uri']]); - return false; - } - if ($item['verb'] == Activity::FOLLOW) { if (!$item['origin'] && ($item['author-id'] == Contact::getPublicIdByUserId($item['uid']))) { // Our own follow request can be relayed to us. We don't store it to avoid notification chaos. @@ -1533,6 +1516,13 @@ class Item return []; } + if ($toplevel_parent['wall'] + && $toplevel_parent['uid'] && + !self::isAllowedByUser($item, $toplevel_parent['uid']) + ) { + return []; + } + return $toplevel_parent; } @@ -3955,4 +3945,41 @@ class Item return array_merge($item, $shared_item); } + + /** + * Check a prospective item array against user-level permissions + * + * @param array $item Expected keys: uri, gravity, and + * author-link if is author-id is set, + * owner-link if is owner-id is set, + * causer-link if is causer-id is set. + * @param int $user_id Local user ID + * @return bool + * @throws \Exception + */ + protected static function isAllowedByUser(array $item, int $user_id) + { + if (!empty($item['author-id']) && Contact\User::isBlocked($item['author-id'], $user_id)) { + Logger::notice('Author is blocked by user', ['author-link' => $item['author-link'], 'uid' => $user_id, 'item-uri' => $item['uri']]); + return false; + } + + if (!empty($item['owner-id']) && Contact\User::isBlocked($item['owner-id'], $user_id)) { + Logger::notice('Owner is blocked by user', ['owner-link' => $item['owner-link'], 'uid' => $user_id, 'item-uri' => $item['uri']]); + return false; + } + + // The causer is set during a thread completion, for example because of a reshare. It countains the responsible actor. + if (!empty($item['causer-id']) && Contact\User::isBlocked($item['causer-id'], $user_id)) { + Logger::notice('Causer is blocked by user', ['causer-link' => $item['causer-link'], 'uid' => $user_id, 'item-uri' => $item['uri']]); + return false; + } + + if (!empty($item['causer-id']) && ($item['gravity'] === GRAVITY_PARENT) && Contact\User::isIgnored($item['causer-id'], $user_id)) { + Logger::notice('Causer is ignored by user', ['causer-link' => $item['causer-link'], 'uid' => $user_id, 'item-uri' => $item['uri']]); + return false; + } + + return true; + } } From 355cd401ae077bad6f3e37cf4e41004ff7570e72 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 10:07:39 -0500 Subject: [PATCH 06/18] Replace uri fields conditions by gravity condition in Model\Item::insert --- src/Model/Item.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 3239c4a8f..a8ac82ede 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1576,7 +1576,7 @@ class Item $uid = intval($item['uid']); $item['guid'] = self::guid($item, $notify); - $item['uri'] = substr(Strings::escapeTags(trim(($item['uri'] ?? '') ?: self::newURI($item['uid'], $item['guid']))), 0, 255); + $item['uri'] = substr(trim($item['uri'] ?? '') ?: self::newURI($item['uid'], $item['guid']), 0, 255); // Store URI data $item['uri-id'] = ItemURI::insert(['uri' => $item['uri'], 'guid' => $item['guid']]); @@ -1685,7 +1685,7 @@ class Item return 0; } - if ($item['thr-parent'] != $item['uri']) { + if ($item['gravity'] !== GRAVITY_PARENT) { $toplevel_parent = self::getTopLevelParent($item); if (empty($toplevel_parent)) { return 0; @@ -1917,7 +1917,7 @@ class Item Logger::notice('created item', ['id' => $current_post, 'uid' => $item['uid'], 'network' => $item['network'], 'uri-id' => $item['uri-id'], 'guid' => $item['guid']]); - if (!$parent_id || ($item['parent-uri'] === $item['uri'])) { + if (!$parent_id || ($item['gravity'] === GRAVITY_PARENT)) { $parent_id = $current_post; } @@ -1942,7 +1942,7 @@ class Item DBA::update('item', ['changed' => DateTimeFormat::utcNow()], ['id' => $parent_id]); } - if ($item['parent-uri'] === $item['uri']) { + if ($item['gravity'] === GRAVITY_PARENT) { self::addThread($current_post); } else { self::updateThread($parent_id); @@ -1970,7 +1970,7 @@ class Item } } - if ($item['parent-uri'] === $item['uri']) { + if ($item['gravity'] === GRAVITY_PARENT) { self::addShadow($current_post); } else { self::addShadowPost($current_post); From eebcf1ae86f16bf3022f4c9a7d91d878ed2d2298 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 22:58:34 -0500 Subject: [PATCH 07/18] Separate $parent_item and $toplevel_item in mod/item --- mod/item.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/mod/item.php b/mod/item.php index 3869ea910..d70353d9b 100644 --- a/mod/item.php +++ b/mod/item.php @@ -100,29 +100,32 @@ function item_post(App $a) { } // Is this a reply to something? - $toplevel_item_id = intval($_REQUEST['parent'] ?? 0); + $parent_item_id = intval($_REQUEST['parent'] ?? 0); $thr_parent_uri = trim($_REQUEST['parent_uri'] ?? ''); + $parent_item = null; $toplevel_item = null; - $parent_user = null; + $toplevel_item_id = null; + $toplevel_user_id = null; $objecttype = null; $profile_uid = ($_REQUEST['profile_uid'] ?? 0) ?: local_user(); $posttype = ($_REQUEST['post_type'] ?? '') ?: Item::PT_ARTICLE; - if ($toplevel_item_id || $thr_parent_uri) { - if ($toplevel_item_id) { - $toplevel_item = Item::selectFirst([], ['id' => $toplevel_item_id]); + if ($parent_item_id || $thr_parent_uri) { + if ($parent_item_id) { + $parent_item = Item::selectFirst([], ['id' => $parent_item_id]); } elseif ($thr_parent_uri) { - $toplevel_item = Item::selectFirst([], ['uri' => $thr_parent_uri, 'uid' => $profile_uid]); + $parent_item = Item::selectFirst([], ['uri' => $thr_parent_uri, 'uid' => $profile_uid]); } // if this isn't the top-level parent of the conversation, find it - if (DBA::isResult($toplevel_item)) { + if (DBA::isResult($parent_item)) { // The URI and the contact is taken from the direct parent which needn't to be the top parent - $thr_parent_uri = $toplevel_item['uri']; + $thr_parent_uri = $parent_item['uri']; + $toplevel_item = $parent_item; - if ($toplevel_item['gravity'] != GRAVITY_PARENT) { + if ($parent_item['gravity'] != GRAVITY_PARENT) { $toplevel_item = Item::selectFirst([], ['id' => $toplevel_item['parent']]); } } @@ -146,7 +149,7 @@ function item_post(App $a) { } $toplevel_item_id = $toplevel_item['id']; - $parent_user = $toplevel_item['uid']; + $toplevel_user_id = $toplevel_item['uid']; $objecttype = Activity\ObjectType::COMMENT; } @@ -168,8 +171,8 @@ function item_post(App $a) { } // Ensure that the user id in a thread always stay the same - if (!is_null($parent_user) && in_array($parent_user, [local_user(), 0])) { - $profile_uid = $parent_user; + if (!is_null($toplevel_user_id) && in_array($toplevel_user_id, [local_user(), 0])) { + $profile_uid = $toplevel_user_id; } // Check for multiple posts with the same message id (when the post was created via API) From d3708cf1c2c819bb3a1711884cbd1b7328fcc4ac Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 11 Nov 2020 22:59:57 -0500 Subject: [PATCH 08/18] Fix wrong variable use in Model\Item::getTopLevelParent - It was preventing items at levels 3 and beyond to be inserted - Logging for missing top level parent has been bumped to notice --- src/Model/Item.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index a8ac82ede..1ccd85c64 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1496,7 +1496,7 @@ class Item $parent = self::selectFirst($fields, $condition, $params); if (!DBA::isResult($parent)) { - Logger::info('item parent was not found - ignoring item', ['thr-parent' => $item['thr-parent'], 'uid' => $item['uid']]); + Logger::notice('item parent was not found - ignoring item', ['thr-parent' => $item['thr-parent'], 'uid' => $item['uid']]); return []; } @@ -1504,21 +1504,21 @@ class Item return $parent; } - $condition = ['uri' => $item['parent-uri'], - 'parent-uri' => $item['parent-uri'], - 'uid' => $item['uid']]; + $condition = ['uri' => $parent['parent-uri'], + 'parent-uri' => $parent['parent-uri'], + 'uid' => $parent['uid']]; // We select wall = 1 in priority for top level permission checks $params = ['order' => ['wall' => true]]; $toplevel_parent = self::selectFirst($fields, $condition, $params); if (!DBA::isResult($toplevel_parent)) { - Logger::info('item parent was not found - ignoring item', ['parent-uri' => $item['parent-uri'], 'uid' => $item['uid']]); + Logger::notice('item top level parent was not found - ignoring item', ['parent-uri' => $parent['parent-uri'], 'uid' => $parent['uid']]); return []; } if ($toplevel_parent['wall'] - && $toplevel_parent['uid'] && - !self::isAllowedByUser($item, $toplevel_parent['uid']) + && $toplevel_parent['uid'] + && !self::isAllowedByUser($item, $toplevel_parent['uid']) ) { return []; } From c36ca3cffea2f168683e1c5d2119b23de8028165 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 12 Nov 2020 09:29:37 -0500 Subject: [PATCH 09/18] Fix null value for item.parent column --- mod/item.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod/item.php b/mod/item.php index d70353d9b..2e5560f5c 100644 --- a/mod/item.php +++ b/mod/item.php @@ -105,7 +105,7 @@ function item_post(App $a) { $parent_item = null; $toplevel_item = null; - $toplevel_item_id = null; + $toplevel_item_id = 0; $toplevel_user_id = null; $objecttype = null; From a9d114316d9ba423cb7440a78e0190e81d2f9625 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 12 Nov 2020 10:23:50 -0500 Subject: [PATCH 10/18] Ensure the parent field isn't set during Item insertion - Avoid a database error if a null value is provided --- src/Model/Item.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 1ccd85c64..4645f553a 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1734,7 +1734,6 @@ class Item // Update the contact relations Contact\Relation::store($toplevel_parent['author-id'], $item['author-id'], $item['created']); - unset($item['parent']); unset($item['parent_origin']); } else { $parent_id = 0; @@ -1777,11 +1776,13 @@ class Item $item['parent'] = $parent_id; Hook::callAll('post_local', $item); unset($item['edit']); - unset($item['parent']); } else { Hook::callAll('post_remote', $item); } + // Set after the insert because top-level posts are self-referencing + unset($item['parent']); + if (!empty($item['cancel'])) { Logger::log('post cancelled by addon.'); return 0; From ff66633a44dbe7d714008e3836a91ae19e63ff3b Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 04:54:01 -0500 Subject: [PATCH 11/18] Remove references to item.parent-uri in Worker\OnePoll --- src/Worker/OnePoll.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Worker/OnePoll.php b/src/Worker/OnePoll.php index bdef1ac4d..c5dfb70ab 100644 --- a/src/Worker/OnePoll.php +++ b/src/Worker/OnePoll.php @@ -583,9 +583,9 @@ class OnePoll } } $condition = ['uri' => $refs_arr, 'uid' => $importer_uid]; - $parent = Item::selectFirst(['parent-uri'], $condition); + $parent = Item::selectFirst(['uri'], $condition); if (DBA::isResult($parent)) { - $datarray['parent-uri'] = $parent['parent-uri']; // Set the parent as the top-level item + $datarray['thr-parent'] = $parent['uri']; } } @@ -613,20 +613,15 @@ class OnePoll $datarray['title'] = self::RemoveReply($datarray['title']); // If it seems to be a reply but a header couldn't be found take the last message with matching subject - if (empty($datarray['parent-uri']) && $reply) { + if (empty($datarray['thr-parent']) && $reply) { $condition = ['title' => $datarray['title'], 'uid' => $importer_uid, 'network' => Protocol::MAIL]; $params = ['order' => ['created' => true]]; - $parent = Item::selectFirst(['parent-uri'], $condition, $params); + $parent = Item::selectFirst(['uri'], $condition, $params); if (DBA::isResult($parent)) { - $datarray['parent-uri'] = $parent['parent-uri']; + $datarray['thr-parent'] = $parent['uri']; } } - if (!empty($datarray['parent-uri'])) { - $datarray['thr-parent'] = $datarray['parent-uri']; - } - unset($datarray['parent-uri']); - $headers = imap_headerinfo($mbox, $meta->msgno); $object = []; From 5ce8cc24de5dc184ba5f4250c05d69244454b9fd Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 05:00:31 -0500 Subject: [PATCH 12/18] Clarify parameter type in DFRN::mail --- src/Protocol/DFRN.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index 165a63c7c..7189588a8 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -414,36 +414,36 @@ class DFRN /** * Create XML text for DFRN mails * - * @param array $item message elements + * @param array $mail Mail record * @param array $owner Owner record * * @return string DFRN mail * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @todo Find proper type-hints */ - public static function mail($item, $owner) + public static function mail(array $mail, array $owner) { $doc = new DOMDocument('1.0', 'utf-8'); $doc->formatOutput = true; $root = self::addHeader($doc, $owner, "dfrn:owner", "", false); - $mail = $doc->createElement("dfrn:mail"); - $sender = $doc->createElement("dfrn:sender"); + $mailElement = $doc->createElement("dfrn:mail"); + $senderElement = $doc->createElement("dfrn:sender"); - XML::addElement($doc, $sender, "dfrn:name", $owner['name']); - XML::addElement($doc, $sender, "dfrn:uri", $owner['url']); - XML::addElement($doc, $sender, "dfrn:avatar", $owner['thumb']); + XML::addElement($doc, $senderElement, "dfrn:name", $owner['name']); + XML::addElement($doc, $senderElement, "dfrn:uri", $owner['url']); + XML::addElement($doc, $senderElement, "dfrn:avatar", $owner['thumb']); - $mail->appendChild($sender); + $mailElement->appendChild($senderElement); - XML::addElement($doc, $mail, "dfrn:id", $item['uri']); - XML::addElement($doc, $mail, "dfrn:in-reply-to", $item['thr-parent']); - XML::addElement($doc, $mail, "dfrn:sentdate", DateTimeFormat::utc($item['created'] . '+00:00', DateTimeFormat::ATOM)); - XML::addElement($doc, $mail, "dfrn:subject", $item['title']); - XML::addElement($doc, $mail, "dfrn:content", $item['body']); + XML::addElement($doc, $mailElement, "dfrn:id", $mail['uri']); + XML::addElement($doc, $mailElement, "dfrn:in-reply-to", $mail['parent-uri']); + XML::addElement($doc, $mailElement, "dfrn:sentdate", DateTimeFormat::utc($mail['created'] . '+00:00', DateTimeFormat::ATOM)); + XML::addElement($doc, $mailElement, "dfrn:subject", $mail['title']); + XML::addElement($doc, $mailElement, "dfrn:content", $mail['body']); - $root->appendChild($mail); + $root->appendChild($mailElement); return trim($doc->saveXML()); } From 042f6b98ac1299f63b9c8d58423f2b252e20ed53 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 05:00:52 -0500 Subject: [PATCH 13/18] Remove unnecessary data array assignment in Protocol\Feed --- src/Protocol/Feed.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Protocol/Feed.php b/src/Protocol/Feed.php index c7d19a996..93e68241c 100644 --- a/src/Protocol/Feed.php +++ b/src/Protocol/Feed.php @@ -354,8 +354,6 @@ class Feed $item["plink"] = DI::httpRequest()->finalUrl($item["plink"]); - $item["thr-parent"] = $item["uri"]; - $item["title"] = XML::getFirstNodeValue($xpath, 'atom:title/text()', $entry); if (empty($item["title"])) { From 2e7c505ac079b0d3b11076e1ad5fb378b03d5453 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 05:01:29 -0500 Subject: [PATCH 14/18] Revert wrong item.thr-parent field usage in Protocol\OStatus --- src/Protocol/OStatus.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 9218f2665..9d9e10f7a 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -523,7 +523,7 @@ class OStatus } } else { // But we will only import complete threads - $valid = Item::exists(['uid' => $importer["uid"], 'uri' => self::$itemlist[0]['thr-parent']]); + $valid = Item::exists(['uid' => $importer["uid"], 'uri' => self::$itemlist[0]['parent-uri']]); if ($valid) { Logger::log("Item with uri ".self::$itemlist[0]["uri"]." belongs to parent ".self::$itemlist[0]['thr-parent']." of user ".$importer["uid"].". It will be imported.", Logger::DEBUG); } From c98da630413b5c420d624f1895de57b5c15a1be4 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 05:05:52 -0500 Subject: [PATCH 15/18] [Database version 1375] Add update method to populate missing item.thr-parent values --- static/dbstructure.config.php | 2 +- update.php | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index d1da02412..290550174 100644 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -54,7 +54,7 @@ use Friendica\Database\DBA; if (!defined('DB_UPDATE_VERSION')) { - define('DB_UPDATE_VERSION', 1374); + define('DB_UPDATE_VERSION', 1375); } return [ diff --git a/update.php b/update.php index 37cccd768..460fad885 100644 --- a/update.php +++ b/update.php @@ -749,3 +749,12 @@ function pre_update_1365() return Update::SUCCESS; } + +function update_1375() +{ + if (!DBA::e("UPDATE `item` SET `thr-parent` = `parent-uri`, `thr-parent-id` = `parent-uri-id` WHERE `thr-parent` = ''")) { + return Update::FAILED; + } + + return Update::SUCCESS; +} From cb963a3259f6cbe20af3eed06223cb4f85169f0d Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 13 Nov 2020 23:56:19 -0500 Subject: [PATCH 16/18] Retrieve local top level parent item separately to check permissions in Model\Item::getTopLevelParentData --- src/Model/Item.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 4645f553a..508cb691c 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1507,19 +1507,17 @@ class Item $condition = ['uri' => $parent['parent-uri'], 'parent-uri' => $parent['parent-uri'], 'uid' => $parent['uid']]; - // We select wall = 1 in priority for top level permission checks - $params = ['order' => ['wall' => true]]; + $params = ['order' => ['id' => false]]; $toplevel_parent = self::selectFirst($fields, $condition, $params); - if (!DBA::isResult($toplevel_parent)) { Logger::notice('item top level parent was not found - ignoring item', ['parent-uri' => $parent['parent-uri'], 'uid' => $parent['uid']]); return []; } - if ($toplevel_parent['wall'] - && $toplevel_parent['uid'] - && !self::isAllowedByUser($item, $toplevel_parent['uid']) - ) { + // If the thread originated from this node, we check the permission against the thread starter + $condition = ['uri' => $toplevel_parent['uri'], 'wall' => true]; + $localTopLevelParent = self::selectFirst(['uid'], $condition); + if (!empty($localTopLevelParent['uid']) && !self::isAllowedByUser($item, $localTopLevelParent['uid'])) { return []; } From b5d3fcb8d4ec29d072779480c5afcca1201e5300 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sat, 14 Nov 2020 07:41:01 -0500 Subject: [PATCH 17/18] Move top-level permission check outside of Model\Item::getTopLevelParentData - It wasn't checked when the direct parent was also the top-level parent --- src/Model/Item.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 508cb691c..fc4d81e43 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1514,13 +1514,6 @@ class Item return []; } - // If the thread originated from this node, we check the permission against the thread starter - $condition = ['uri' => $toplevel_parent['uri'], 'wall' => true]; - $localTopLevelParent = self::selectFirst(['uid'], $condition); - if (!empty($localTopLevelParent['uid']) && !self::isAllowedByUser($item, $localTopLevelParent['uid'])) { - return []; - } - return $toplevel_parent; } @@ -1689,6 +1682,13 @@ class Item return 0; } + // If the thread originated from this node, we check the permission against the thread starter + $condition = ['uri' => $toplevel_parent['uri'], 'wall' => true]; + $localTopLevelParent = self::selectFirst(['uid'], $condition); + if (!empty($localTopLevelParent['uid']) && !self::isAllowedByUser($item, $localTopLevelParent['uid'])) { + return 0; + } + $parent_id = $toplevel_parent['id']; $item['parent-uri'] = $toplevel_parent['uri']; $item['deleted'] = $toplevel_parent['deleted']; From 682b9c24f8e494dd2caaffe72184ab89040b087f Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 15 Nov 2020 11:42:46 -0500 Subject: [PATCH 18/18] Update database.sql with the latest structure changes --- database.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/database.sql b/database.sql index daf65d7c8..e2a0f243a 100644 --- a/database.sql +++ b/database.sql @@ -641,8 +641,8 @@ CREATE TABLE IF NOT EXISTS `item` ( `uri-id` int unsigned COMMENT 'Id of the item-uri table entry that contains the item uri', `uri-hash` varchar(80) NOT NULL DEFAULT '' COMMENT 'RIPEMD-128 hash from uri', `parent` int unsigned NOT NULL DEFAULT 0 COMMENT 'item.id of the parent to this item if it is a reply of some form; otherwise this must be set to the id of this item', - `parent-uri` varchar(255) NOT NULL DEFAULT '' COMMENT 'uri of the parent to this item', - `parent-uri-id` int unsigned COMMENT 'Id of the item-uri table that contains the parent uri', + `parent-uri` varchar(255) NOT NULL DEFAULT '' COMMENT 'uri of the top-level parent to this item', + `parent-uri-id` int unsigned COMMENT 'Id of the item-uri table that contains the top-level parent uri', `thr-parent` varchar(255) NOT NULL DEFAULT '' COMMENT 'If the parent of this item is not the top-level item in the conversation, the uri of the immediate parent; otherwise set to parent-uri', `thr-parent-id` int unsigned COMMENT 'Id of the item-uri table that contains the thread parent uri', `created` datetime NOT NULL DEFAULT '0001-01-01 00:00:00' COMMENT 'Creation timestamp.',