Found the cause of the delete looping, and I can prevent it going forward. The issue remains what to do about comments which are already in the DB and have ITEM_ORIGIN incorrectly set. We can't exactly reset them because the "original" context has been lost. (Sorry but couldn't resist an insider pun that none of you will be able to follow anyway). Read the comments.

3 files changed, 54 insertions, 15 deletions

diff --git a/include/items.php b/include/items.php
index b1e17b171..b7bdad838 100755
--- a/include/items.php
+++ b/include/items.php
@@ -3073,7 +3073,16 @@ function start_delivery_chain($channel,$item,$item_id,$parent) {
 	if((! $private) && $new_public_policy)
 		$private = 1;
 
-	$flag_bits = $item['item_flags'] | ITEM_WALL|ITEM_ORIGIN;
+	$flag_bits = $item['item_flags'] | ITEM_WALL;
+
+	// The message didn't necessarily originate on this site, (we'll honour it if it did),
+	// but the parent post of this thread will be reset as a local post, as it is the top of 
+	// this delivery chain and is coming from this site, regardless of where the original
+	// originated.
+
+	if(! $parent)
+		$flag_bits = $flag_bits | ITEM_ORIGIN;
+
 
 	// unset the nocomment bit if it's there. 
 
diff --git a/include/notifier.php b/include/notifier.php
index c77857087..f64732884 100644
--- a/include/notifier.php
+++ b/include/notifier.php
@@ -366,6 +366,11 @@ function notifier_run($argv, $argc){
 
 		$encoded_item = encode_item($target_item);
 		
+		// Send comments to the owner to re-deliver to everybody in the conversation
+		// We only do this if the item in question originated on this site. This prevents looping.
+		// To clarify, a site accepting a new comment is responsible for sending it to the owner for relay.
+		// Relaying should never be initiated on a post that arrived from elsewhere.  
+
 		$relay_to_owner = (((! $top_level_post) && ($target_item['item_flags'] & ITEM_ORIGIN)) ? true : false);
 
 		$uplink = false;
@@ -379,7 +384,7 @@ function notifier_run($argv, $argc){
 
 		// tag_deliver'd post which needs to be sent back to the original author
 
-		if(($cmd === 'uplink') && ($parent_item['item_flags'] & ITEM_UPLINK) && (! $top_level_post) && (! $deleted_item)) {
+		if(($cmd === 'uplink') && ($parent_item['item_flags'] & ITEM_UPLINK) && (! $top_level_post)) {
 			logger('notifier: uplink');			
 			$uplink = true;
 		} 
@@ -400,7 +405,7 @@ function notifier_run($argv, $argc){
 			// if our parent is a tag_delivery recipient, uplink to the original author causing
 			// a delivery fork. 
 
-			if(($parent_item['item_flags'] & ITEM_UPLINK) && (! $top_level_post) && ($cmd !== 'uplink') && (! $deleted_item)) {
+			if(($parent_item['item_flags'] & ITEM_UPLINK) && (! $top_level_post) && ($cmd !== 'uplink')) {
 				logger('notifier: uplinking this item');
 				proc_run('php','include/notifier.php','uplink',$item_id);
 			}
diff --git a/include/zot.php b/include/zot.php
index 59d158a0b..455702b06 100644
--- a/include/zot.php
+++ b/include/zot.php
@@ -1297,7 +1297,9 @@ function public_recips($msg) {
 		else {
 
 			// This doesn't look like it works so I have to explain what happened. These are my
-			// notes (below) from when I got this section of code working. 
+			// notes (below) from when I got this section of code working. You would think that
+			// we only have to find those with the requisite stream or comment permissions,
+			// depending on whether this is a top-level post or a comment - but you would be wrong.
  
 			// ... so public_recips and allowed_public_recips is working so much better
 			// than before, but was still not quite right. We seem to be getting all the right 
@@ -1403,9 +1405,12 @@ function public_recips($msg) {
 		}
 	}
 	else {
-		// this is a comment. Find any parent with ITEM_UPLINK set.
+		// This is a comment. We need to find any parent with ITEM_UPLINK set. But in fact, let's just return
+		// everybody that stored a copy of the parent. This way we know we're covered. We'll check the 
+		// comment permissions when we deliver them.
+
 		if($msg['message']['message_top']) {
-			$z = q("select owner_xchan as hash from item where parent_mid = '%s' and ( item_flags & %d ) > 0 ",
+			$z = q("select owner_xchan as hash from item where parent_mid = '%s' ",
 				dbesc($msg['message']['message_top']),
 				intval(ITEM_UPLINK)
 			);
@@ -1414,10 +1419,22 @@ function public_recips($msg) {
 		}
 	}
 
-	// FIXME
-	// There are probably a lot of duplicates in $r at this point. We really need to filter those out.
+	// There are probably a lot of duplicates in $r at this point. We need to filter those out.
 	// It's a bit of work since it's a multi-dimensional array
 
+	if($r) {
+		$uniq = array();
+		
+		foreach($r as $rr) {
+			if(! in_array($rr['hash'],$uniq))
+				$uniq[] = $rr['hash'];
+		}
+		$r = array();
+		foreach($uniq as $rr) {
+			$r[] = array('hash' => $rr);
+		}
+	}
+
 	logger('public_recips: ' . print_r($r,true), LOGGER_DATA);
 	return $r;
 }
@@ -1427,9 +1444,16 @@ function public_recips($msg) {
 
 function allowed_public_recips($msg) {
 
-
 	logger('allowed_public_recips: ' . print_r($msg,true),LOGGER_DATA);
 
+	if(array_key_exists('public_scope',$msg['message']))
+		$scope = $msg['message']['public_scope'];
+
+	// Mail won't have a public scope.
+	// in fact, it's doubtful mail will ever get here since it almost universally
+	// has a recipient, but in fact we don't require this, so it's technically 
+	// possible to send mail to anybody that's listening.  
+
 	$recips = public_recips($msg);
 
 	if(! $recips)
@@ -1438,11 +1462,6 @@ function allowed_public_recips($msg) {
 	if($msg['message']['type'] === 'mail')
 		return $recips;
 
-	if(array_key_exists('public_scope',$msg['message']))
-		$scope = $msg['message']['public_scope'];
-
-	$hash = make_xchan_hash($msg['notify']['sender']['guid'],$msg['notify']['sender']['guid_sig']);
-
 	if($scope === 'public' || $scope === 'network: red' || $scope === 'authenticated')
 		return $recips;
 
@@ -1454,12 +1473,17 @@ function allowed_public_recips($msg) {
 	}
 
 	if($scope === 'self') {
+
+		$hash = make_xchan_hash($msg['notify']['sender']['guid'],$msg['notify']['sender']['guid_sig']);
+
 		foreach($recips as $r)
 			if($r['hash'] === $hash)
 				return array('hash' => $hash);
 	}
 
-	if($scope === 'contacts') {
+	// note: we shouldn't ever see $scope === 'specific' in this function, but handle it anyway
+
+	if($scope === 'contacts' || $scope === 'any connections' || $scope === 'specific') {
 		$condensed_recips = array();
 		foreach($recips as $rr)
 			$condensed_recips[] = $rr['hash'];
@@ -1477,6 +1501,7 @@ function allowed_public_recips($msg) {
 		return $results;
 	}
 
+
 	return array();
 }