[bugfix] Allow blocked accounts to show in precise search (#2321)

4 files changed, 177 insertions, 127 deletions

diff --git a/internal/processing/search/accounts.go b/internal/processing/search/accounts.go
index cfcc65b2b..17002a29b 100644
--- a/internal/processing/search/accounts.go
+++ b/internal/processing/search/accounts.go
@@ -29,6 +29,7 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
 // Accounts does a partial search for accounts that
@@ -56,6 +57,11 @@ func (p *Processor) Accounts(
 	// in their search results.
 	const includeInstanceAccounts = false
 
+	// We *might* want to include blocked accounts
+	// in this search, but only if it's a search
+	// for a specific account.
+	includeBlockedAccounts := false
+
 	var (
 		foundAccounts = make([]*gtsmodel.Account, 0, limit)
 		appendAccount = func(foundAccount *gtsmodel.Account) { foundAccounts = append(foundAccounts, foundAccount) }
@@ -95,26 +101,42 @@ func (p *Processor) Accounts(
 			requestingAccount,
 			foundAccounts,
 			includeInstanceAccounts,
+			includeBlockedAccounts,
 		)
 	}
 
-	// Return all accounts we can find that match the
-	// provided query. If it's not a namestring, this
-	// won't return an error, it'll just return 0 results.
-	if _, err := p.accountsByNamestring(
-		ctx,
-		requestingAccount,
-		id.Highest,
-		id.Lowest,
-		limit,
-		offset,
-		query,
-		resolve,
-		following,
-		appendAccount,
-	); err != nil && !errors.Is(err, db.ErrNoEntries) {
-		err = gtserror.Newf("error searching by namestring: %w", err)
-		return nil, gtserror.NewErrorInternalError(err)
+	// See if we have something that looks like a namestring.
+	username, domain, err := util.ExtractNamestringParts(query)
+	if err != nil {
+		log.Warnf(ctx, "couldn't parse '%s' as namestring: %v", query, err)
+	} else {
+		if domain != "" {
+			// Search was an exact namestring;
+			// we can safely assume caller is
+			// searching for a specific account,
+			// and show it to them even if they
+			// have it blocked.
+			includeBlockedAccounts = true
+		}
+
+		// Get all accounts we can find
+		// that match the provided query.
+		if err := p.accountsByNamestring(
+			ctx,
+			requestingAccount,
+			id.Highest,
+			id.Lowest,
+			limit,
+			offset,
+			username,
+			domain,
+			resolve,
+			following,
+			appendAccount,
+		); err != nil && !errors.Is(err, db.ErrNoEntries) {
+			err = gtserror.Newf("error searching by namestring: %w", err)
+			return nil, gtserror.NewErrorInternalError(err)
+		}
 	}
 
 	// Return whatever we got (if anything).
@@ -123,5 +145,6 @@ func (p *Processor) Accounts(
 		requestingAccount,
 		foundAccounts,
 		includeInstanceAccounts,
+		includeBlockedAccounts,
 	)
 }
diff --git a/internal/processing/search/get.go b/internal/processing/search/get.go
index 830e3481b..30a2745af 100644
--- a/internal/processing/search/get.go
+++ b/internal/processing/search/get.go
@@ -77,6 +77,12 @@ func (p *Processor) Get(
 		// search in the database in the latter
 		// parts of this function.
 		includeInstanceAccounts = true
+
+		// Assume caller doesn't want to see
+		// blocked accounts. This will change
+		// to 'true' if caller is searching
+		// for a specific account.
+		includeBlockedAccounts = false
 	)
 
 	// Validate query.
@@ -120,7 +126,9 @@ func (p *Processor) Get(
 			ctx,
 			account,
 			nil, nil, nil, // No results.
-			req.APIv1, includeInstanceAccounts,
+			req.APIv1,
+			includeInstanceAccounts,
+			includeBlockedAccounts,
 		)
 	}
 
@@ -131,7 +139,6 @@ func (p *Processor) Get(
 		appendStatus  = func(s *gtsmodel.Status) { foundStatuses = append(foundStatuses, s) }
 		appendAccount = func(a *gtsmodel.Account) { foundAccounts = append(foundAccounts, a) }
 		appendTag     = func(t *gtsmodel.Tag) { foundTags = append(foundTags, t) }
-		keepLooking   bool
 		err           error
 	)
 
@@ -152,56 +159,83 @@ func (p *Processor) Get(
 			}
 		}
 
-		// Search using what may or may not be a namestring.
-		keepLooking, err = p.accountsByNamestring(
-			ctx,
-			account,
-			maxID,
-			minID,
-			limit,
-			offset,
-			queryC,
-			resolve,
-			following,
-			appendAccount,
-		)
-		if err != nil && !errors.Is(err, db.ErrNoEntries) {
-			err = gtserror.Newf("error searching by namestring: %w", err)
-			return nil, gtserror.NewErrorInternalError(err)
-		}
-
-		if !keepLooking {
-			// Return whatever we have.
-			return p.packageSearchResult(
+		// See if we have something that looks like a namestring.
+		username, domain, err := util.ExtractNamestringParts(queryC)
+		if err == nil {
+			// We managed to parse query as a namestring.
+			// If domain was set, this is a very specific
+			// search for a particular account, so show
+			// that account to the caller even if they
+			// have it blocked. They might be looking
+			// for it to unblock it again!
+			domainSet := (domain != "")
+			includeBlockedAccounts = domainSet
+
+			err = p.accountsByNamestring(
 				ctx,
 				account,
-				foundAccounts,
-				foundStatuses,
-				foundTags,
-				req.APIv1,
-				includeInstanceAccounts,
+				maxID,
+				minID,
+				limit,
+				offset,
+				username,
+				domain,
+				resolve,
+				following,
+				appendAccount,
 			)
+			if err != nil && !errors.Is(err, db.ErrNoEntries) {
+				err = gtserror.Newf("error searching by namestring: %w", err)
+				return nil, gtserror.NewErrorInternalError(err)
+			}
+
+			// If domain was set, we know this is
+			// a full namestring, and not a url or
+			// just a username, so we should stop
+			// looking now and just return what we
+			// have (if anything). Otherwise we'll
+			// let the search keep going.
+			if domainSet {
+				return p.packageSearchResult(
+					ctx,
+					account,
+					foundAccounts,
+					foundStatuses,
+					foundTags,
+					req.APIv1,
+					includeInstanceAccounts,
+					includeBlockedAccounts,
+				)
+			}
 		}
 	}
 
-	// Check if the query is a URI with a recognizable
-	// scheme and use it to look for accounts or statuses.
-	keepLooking, err = p.byURI(
-		ctx,
-		account,
-		query,
-		queryType,
-		resolve,
-		appendAccount,
-		appendStatus,
-	)
-	if err != nil && !errors.Is(err, db.ErrNoEntries) {
-		err = gtserror.Newf("error searching by URI: %w", err)
-		return nil, gtserror.NewErrorInternalError(err)
-	}
+	// Check if we're searching by a known URI scheme.
+	// (This might just be a weirdly-parsed URI,
+	// since Go's url package tends to be a bit
+	// trigger-happy when deciding things are URIs).
+	uri, err := url.Parse(query)
+	if err == nil && (uri.Scheme == "https" || uri.Scheme == "http") {
+		// URI is pretty specific so we can safely assume
+		// caller wants to include blocked accounts too.
+		includeBlockedAccounts = true
 
-	if !keepLooking {
-		// Return whatever we have.
+		if err := p.byURI(
+			ctx,
+			account,
+			uri,
+			queryType,
+			resolve,
+			appendAccount,
+			appendStatus,
+		); err != nil && !errors.Is(err, db.ErrNoEntries) {
+			err = gtserror.Newf("error searching by URI: %w", err)
+			return nil, gtserror.NewErrorInternalError(err)
+		}
+
+		// This was a URI, so at this point just return
+		// whatever we have. You can't search hashtags by
+		// URI, and shouldn't do full-text with a URI either.
 		return p.packageSearchResult(
 			ctx,
 			account,
@@ -210,6 +244,7 @@ func (p *Processor) Get(
 			foundTags,
 			req.APIv1,
 			includeInstanceAccounts,
+			includeBlockedAccounts,
 		)
 	}
 
@@ -226,7 +261,7 @@ func (p *Processor) Get(
 	// We know that none of the subsequent searches
 	// would show any good results either, and those
 	// searches are *much* more expensive.
-	keepLooking, err = p.hashtag(
+	keepLooking, err := p.hashtag(
 		ctx,
 		maxID,
 		minID,
@@ -251,6 +286,7 @@ func (p *Processor) Get(
 			foundTags,
 			req.APIv1,
 			includeInstanceAccounts,
+			includeBlockedAccounts,
 		)
 	}
 
@@ -291,15 +327,15 @@ func (p *Processor) Get(
 		foundTags,
 		req.APIv1,
 		includeInstanceAccounts,
+		includeBlockedAccounts,
 	)
 }
 
 // accountsByNamestring searches for accounts using the
-// provided namestring query. If domain is not set in
-// the namestring, it may return more than one result
-// by doing a text search in the database for accounts
-// matching the query. Otherwise, it tries to return an
-// exact match.
+// provided username and domain. If domain is not set,
+// it may return more than one result by doing a text
+// search in the database for accounts matching the query.
+// Otherwise, it tries to return an exact match.
 func (p *Processor) accountsByNamestring(
 	ctx context.Context,
 	requestingAccount *gtsmodel.Account,
@@ -307,26 +343,18 @@ func (p *Processor) accountsByNamestring(
 	minID string,
 	limit int,
 	offset int,
-	query string,
+	username string,
+	domain string,
 	resolve bool,
 	following bool,
 	appendAccount func(*gtsmodel.Account),
-) (bool, error) {
-	// See if we have something that looks like a namestring.
-	username, domain, err := util.ExtractNamestringParts(query)
-	if err != nil {
-		// No need to return error; just not a namestring
-		// we can search with. Caller should keep looking
-		// with another search method.
-		return true, nil //nolint:nilerr
-	}
-
+) error {
 	if domain == "" {
 		// No error, but no domain set. That means the query
 		// looked like '@someone' which is not an exact search.
 		// Try to search for any accounts that match the query
 		// string, and let the caller know they should stop.
-		return false, p.accountsByText(
+		return p.accountsByText(
 			ctx,
 			requestingAccount.ID,
 			maxID,
@@ -355,18 +383,14 @@ func (p *Processor) accountsByNamestring(
 		// Check for semi-expected error types.
 		// On one of these, we can continue.
 		if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
-			err = gtserror.Newf("error looking up %s as account: %w", query, err)
-			return false, gtserror.NewErrorInternalError(err)
+			err = gtserror.Newf("error looking up @%s@%s as account: %w", username, domain, err)
+			return gtserror.NewErrorInternalError(err)
 		}
 	} else {
 		appendAccount(foundAccount)
 	}
 
-	// Regardless of whether we have a hit at this point,
-	// return false to indicate caller should stop looking;
-	// namestrings are a very specific format so it's unlikely
-	// the caller was looking for something other than an account.
-	return false, nil
+	return nil
 }
 
 // accountByUsernameDomain looks for one account with the given
@@ -443,38 +467,22 @@ func (p *Processor) accountByUsernameDomain(
 func (p *Processor) byURI(
 	ctx context.Context,
 	requestingAccount *gtsmodel.Account,
-	query string,
+	uri *url.URL,
 	queryType string,
 	resolve bool,
 	appendAccount func(*gtsmodel.Account),
 	appendStatus func(*gtsmodel.Status),
-) (bool, error) {
-	uri, err := url.Parse(query)
-	if err != nil {
-		// No need to return error; just not a URI
-		// we can search with. Caller should keep
-		// looking with another search method.
-		return true, nil //nolint:nilerr
-	}
-
-	if !(uri.Scheme == "https" || uri.Scheme == "http") {
-		// This might just be a weirdly-parsed URI,
-		// since Go's url package tends to be a bit
-		// trigger-happy when deciding things are URIs.
-		// Indicate caller should keep looking.
-		return true, nil
-	}
-
+) error {
 	blocked, err := p.state.DB.IsURIBlocked(ctx, uri)
 	if err != nil {
 		err = gtserror.Newf("error checking domain block: %w", err)
-		return false, gtserror.NewErrorInternalError(err)
+		return gtserror.NewErrorInternalError(err)
 	}
 
 	if blocked {
-		// Don't search for blocked domains.
-		// Caller should stop looking.
-		return false, nil
+		// Don't search for
+		// blocked domains.
+		return nil
 	}
 
 	if includeAccounts(queryType) {
@@ -485,14 +493,13 @@ func (p *Processor) byURI(
 			// On one of these, we can continue.
 			if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
 				err = gtserror.Newf("error looking up %s as account: %w", uri, err)
-				return false, gtserror.NewErrorInternalError(err)
+				return gtserror.NewErrorInternalError(err)
 			}
 		} else {
-			// Hit; return false to indicate caller should
-			// stop looking, since it's extremely unlikely
+			// Hit! Return early since it's extremely unlikely
 			// a status and an account will have the same URL.
 			appendAccount(foundAccount)
-			return false, nil
+			return nil
 		}
 	}
 
@@ -504,20 +511,19 @@ func (p *Processor) byURI(
 			// On one of these, we can continue.
 			if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
 				err = gtserror.Newf("error looking up %s as status: %w", uri, err)
-				return false, gtserror.NewErrorInternalError(err)
+				return gtserror.NewErrorInternalError(err)
 			}
 		} else {
-			// Hit; return false to indicate caller should
-			// stop looking, since it's extremely unlikely
+			// Hit! Return early since it's extremely unlikely
 			// a status and an account will have the same URL.
 			appendStatus(foundStatus)
-			return false, nil
+			return nil
 		}
 	}
 
-	// No errors, but no hits either; since this
-	// was a URI, caller should stop looking.
-	return false, nil
+	// No errors, but no hits
+	// either; that's fine.
+	return nil
 }
 
 // accountByURI looks for one account with the given URI.
diff --git a/internal/processing/search/lookup.go b/internal/processing/search/lookup.go
index 8b48d4514..b6f1eef52 100644
--- a/internal/processing/search/lookup.go
+++ b/internal/processing/search/lookup.go
@@ -51,6 +51,11 @@ func (p *Processor) Lookup(
 	// accident.
 	const includeInstanceAccounts = true
 
+	// Since lookup is always for a specific
+	// account, it's fine to include a blocked
+	// account in the results.
+	const includeBlockedAccounts = true
+
 	// Validate query.
 	query = strings.TrimSpace(query)
 	if query == "" {
@@ -108,6 +113,7 @@ func (p *Processor) Lookup(
 		requestingAccount,
 		[]*gtsmodel.Account{account},
 		includeInstanceAccounts,
+		includeBlockedAccounts,
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
diff --git a/internal/processing/search/util.go b/internal/processing/search/util.go
index 9d9175246..de91e5d51 100644
--- a/internal/processing/search/util.go
+++ b/internal/processing/search/util.go
@@ -49,6 +49,7 @@ func (p *Processor) packageAccounts(
 	requestingAccount *gtsmodel.Account,
 	accounts []*gtsmodel.Account,
 	includeInstanceAccounts bool,
+	includeBlockedAccounts bool,
 ) ([]*apimodel.Account, gtserror.WithCode) {
 	apiAccounts := make([]*apimodel.Account, 0, len(accounts))
 
@@ -58,19 +59,26 @@ func (p *Processor) packageAccounts(
 			continue
 		}
 
-		// Ensure requester can see result account.
-		visible, err := p.filter.AccountVisible(ctx, requestingAccount, account)
+		// Check if block exists between searcher and searchee.
+		blocked, err := p.state.DB.IsEitherBlocked(ctx, requestingAccount.ID, account.ID)
 		if err != nil {
-			err = gtserror.Newf("error checking visibility of account %s for account %s: %w", account.ID, requestingAccount.ID, err)
+			err = gtserror.Newf("error checking block between searching account %s and searched account %s: %w", requestingAccount.ID, account.ID, err)
 			return nil, gtserror.NewErrorInternalError(err)
 		}
 
-		if !visible {
-			log.Debugf(ctx, "account %s is not visible to account %s, skipping this result", account.ID, requestingAccount.ID)
+		if blocked && !includeBlockedAccounts {
+			// Don't include
+			// this result.
 			continue
 		}
 
-		apiAccount, err := p.converter.AccountToAPIAccountPublic(ctx, account)
+		var apiAccount *apimodel.Account
+		if blocked {
+			apiAccount, err = p.converter.AccountToAPIAccountBlocked(ctx, account)
+		} else {
+			apiAccount, err = p.converter.AccountToAPIAccountPublic(ctx, account)
+		}
+
 		if err != nil {
 			log.Debugf(ctx, "skipping account %s because it couldn't be converted to its api representation: %s", account.ID, err)
 			continue
@@ -171,8 +179,15 @@ func (p *Processor) packageSearchResult(
 	tags []*gtsmodel.Tag,
 	v1 bool,
 	includeInstanceAccounts bool,
+	includeBlockedAccounts bool,
 ) (*apimodel.SearchResult, gtserror.WithCode) {
-	apiAccounts, errWithCode := p.packageAccounts(ctx, requestingAccount, accounts, includeInstanceAccounts)
+	apiAccounts, errWithCode := p.packageAccounts(
+		ctx,
+		requestingAccount,
+		accounts,
+		includeInstanceAccounts,
+		includeBlockedAccounts,
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}