From 40f9eefc2153ee8811abb750e07064c4f98f549c Mon Sep 17 00:00:00 2001
From: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Sun, 18 Feb 2024 10:49:40 +0100
Subject: [PATCH] [bugfix] Fix dereferencing ancestors on new status create
 (#2652)

* [bugfix] Pass `latest` to dereferenceThread instead of barebones status

* only mark status orphaned if visibility suggests parent is really deleted

* tone down "not deref'd" warnings, since they represent a legit visibility situation

* remove FAQ entry for "status not deref'd yet"
---
 docs/faq.md                                 |  4 ---
 internal/federation/dereferencing/status.go |  2 +-
 internal/federation/dereferencing/thread.go | 31 +++++++++++++++++++--
 internal/visibility/home_timeline.go        |  2 +-
 internal/visibility/public_timeline.go      |  2 +-
 5 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/docs/faq.md b/docs/faq.md
index 219932078..23403d363 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -50,10 +50,6 @@ Take a look at the [list of open bugs](https://github.com/superseriousbusiness/g
 
 These warnings are caused by Mastodon forgetting to take query parameters into account when signing HTTP requests. Fixes on both Mastodon and GoToSocial sides are in the works, and other instance software is expected to follow.
 
-## Warnings “status not yet deref'd” in the logs
-
-The warning is mostly to say that the derived visibility of the given status may not be entirely accurate due to a parent not being dereferenced yet, so do not worry about it.
-
 ## Will you support tables in Markdown?
 
 Not at the moment, as most clients handle them terribly.
diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go
index 6d3dd5691..e3f97553d 100644
--- a/internal/federation/dereferencing/status.go
+++ b/internal/federation/dereferencing/status.go
@@ -222,7 +222,7 @@ func (d *Dereferencer) RefreshStatus(
 		d.dereferenceThread(ctx,
 			requestUser,
 			uri,
-			status,
+			latest,
 			statusable,
 			isNew,
 		)
diff --git a/internal/federation/dereferencing/thread.go b/internal/federation/dereferencing/thread.go
index 2814c0e7d..28f7ffa8a 100644
--- a/internal/federation/dereferencing/thread.go
+++ b/internal/federation/dereferencing/thread.go
@@ -124,9 +124,34 @@ func (d *Dereferencer) DereferenceStatusAncestors(ctx context.Context, username
 		// Check for a returned HTTP code via error.
 		switch code := gtserror.StatusCode(err); {
 
-		// Status codes 404 and 410 incicate the status does not exist anymore.
-		// Gone (410) is the preferred for deletion, but we accept NotFound too.
-		case code == http.StatusNotFound || code == http.StatusGone:
+		// 404 may indicate deletion, but can also
+		// indicate that we don't have permission to
+		// view the status (it's followers-only and
+		// we don't follow, for example).
+		case code == http.StatusNotFound:
+			// If this reply is followers-only or stricter,
+			// we can safely assume the status it replies
+			// to is also followers only or stricter.
+			//
+			// In this case we should leave the inReplyTo
+			// URI in place for visibility filtering,
+			// and just return since we can go no further.
+			if status.Visibility == gtsmodel.VisibilityFollowersOnly ||
+				status.Visibility == gtsmodel.VisibilityMutualsOnly ||
+				status.Visibility == gtsmodel.VisibilityDirect {
+				return nil
+			}
+
+			// If the reply is public or unlisted then
+			// likely the replied-to status is/was public
+			// or unlisted and has indeed been deleted,
+			// fall through to the Gone case to clean up.
+			fallthrough
+
+		// Gone (410) definitely indicates deletion.
+		// Update the status to remove references to
+		// the now-gone parent.
+		case code == http.StatusGone:
 			l.Trace("status orphaned")
 			current.InReplyToID = ""
 			current.InReplyToURI = ""
diff --git a/internal/visibility/home_timeline.go b/internal/visibility/home_timeline.go
index 3cecbb5de..0a3fbde4e 100644
--- a/internal/visibility/home_timeline.go
+++ b/internal/visibility/home_timeline.go
@@ -156,7 +156,7 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A
 
 		// Check parent is deref'd.
 		if next.InReplyToID == "" {
-			log.Warnf(ctx, "status not yet deref'd: %s", next.InReplyToURI)
+			log.Debugf(ctx, "status not (yet) deref'd: %s", next.InReplyToURI)
 			return false, cache.SentinelError
 		}
 
diff --git a/internal/visibility/public_timeline.go b/internal/visibility/public_timeline.go
index b2c05d51f..bad7cf991 100644
--- a/internal/visibility/public_timeline.go
+++ b/internal/visibility/public_timeline.go
@@ -95,7 +95,7 @@ func (f *Filter) isStatusPublicTimelineable(ctx context.Context, requester *gtsm
 		// Fetch next parent to lookup.
 		parentID := parent.InReplyToID
 		if parentID == "" {
-			log.Warnf(ctx, "status not yet deref'd: %s", parent.InReplyToURI)
+			log.Debugf(ctx, "status not (yet) deref'd: %s", parent.InReplyToURI)
 			return false, cache.SentinelError
 		}