[bugfix] Relax Mention parsing, allowing either href or name (#2320)

This commit is contained in:
tobi 2023-10-31 12:05:17 +01:00 committed by GitHub
parent dd4b0241ea
commit 51d0a0bba5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 317 additions and 40 deletions

View file

@ -427,3 +427,58 @@ With just one tag, the `tag` property will be an object rather than an array, wh
The `href` URL provided by GoToSocial in outgoing tags points to a web URL that serves `text/html`. The `href` URL provided by GoToSocial in outgoing tags points to a web URL that serves `text/html`.
GoToSocial makes no guarantees whatsoever about what the content of the given `text/html` will be, and remote servers should not interpret the URL as a canonical ActivityPub ID/URI property. The `href` URL is provided merely as an endpoint which *might* contain more information about the given hashtag. GoToSocial makes no guarantees whatsoever about what the content of the given `text/html` will be, and remote servers should not interpret the URL as a canonical ActivityPub ID/URI property. The `href` URL is provided merely as an endpoint which *might* contain more information about the given hashtag.
## Mentions
GoToSocial users can Mention other users in their posts, using the common `@[username]@[domain]` format. For example, if a GoToSocial user wanted to mention user `someone` on instance `example.org`, they could do this by including `@someone@example.org` in their post somewhere.
!!! info "Mentions and activity addressing"
Mentions are not just aesthetic, they affect addressing of Activities as well.
If a GoToSocial user explicitly mentions another user in a Note, the URI of that user will always be included in the `To` or `Cc` property of the Note's Create activity.
If the Note is direct (ie., not `To` public or followers), each mention target URI will be in the `To` property of the wrapping Activity
In all other cases, mentions will be included in the `Cc` property of the wrapping Activity.
### Outgoing
When a GoToSocial user Mentions another account, the Mention is included in outgoing federated messages as an entry in the `tag` property.
For example, say a user on a GoToSocial instance Mentions `@someone@example.org`, the `tag` property of the outgoing Note might look like the following:
```json
"tag": {
"href": "http://example.org/users/someone",
"name": "@someone@example.org",
"type": "Mention"
}
```
If a user Mentions a local user they share an instance with, the full `name` of the local user will still be included.
For example, a GoToSocial user on domain `some.gotosocial.instance` mentions another user on the same instance called `user2`. They also mention `@someone@example.org` as above. The `tag` property of the outgoing Note would look like the following:
```json
"tag": [
{
"href": "http://example.org/users/someone",
"name": "@someone@example.org",
"type": "Mention"
},
{
"href": "http://some.gotosocial.instance/users/user2",
"name": "@user2@some.gotosocial.instance",
"type": "Mention"
}
]
```
For the convenience of remote servers, GoToSocial will always provide both the `href` and the `name` properties on outgoing Mentions. The `href` property used by GoToSocial will always be the ActivityPub ID/URI of the target account, not the web URL.
### Incoming
GoToSocial tries to parse incoming Mentions in the same way it sends them out: as a `Mention` type entry in the `tag` property. However, when parsing incoming Mentions it's a bit more relaxed with regards to which properties must be set.
GoToSocial will prefer the `href` property, which can be either the ActivityPub ID/URI or the web URL of the target; if `href` is not present, it will fall back to using the `name` property. If neither property is present, the mention will be considered invalid and discarded.

View file

@ -32,7 +32,6 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/text" "github.com/superseriousbusiness/gotosocial/internal/text"
"github.com/superseriousbusiness/gotosocial/internal/util"
) )
// ExtractObjects will extract object vocab.Types from given implementing interface. // ExtractObjects will extract object vocab.Types from given implementing interface.
@ -841,27 +840,27 @@ func ExtractMentions(i WithTag) ([]*gtsmodel.Mention, error) {
// ExtractMention extracts a minimal gtsmodel.Mention from a Mentionable. // ExtractMention extracts a minimal gtsmodel.Mention from a Mentionable.
func ExtractMention(i Mentionable) (*gtsmodel.Mention, error) { func ExtractMention(i Mentionable) (*gtsmodel.Mention, error) {
// See if a name has been set in the
// format `@someone@example.org`.
nameString := ExtractName(i) nameString := ExtractName(i)
if nameString == "" {
return nil, gtserror.New("name prop empty")
}
// Ensure namestring is valid so we
// can handle it properly later on.
if _, _, err := util.ExtractNamestringParts(nameString); err != nil {
return nil, err
}
// The href prop should be the AP URI // The href prop should be the AP URI
// of the target account. // of the target account; it could also
// be the URL, but we'll check this later.
var href string
hrefProp := i.GetActivityStreamsHref() hrefProp := i.GetActivityStreamsHref()
if hrefProp == nil || !hrefProp.IsIRI() { if hrefProp != nil && hrefProp.IsIRI() {
return nil, gtserror.New("no href prop") href = hrefProp.GetIRI().String()
}
// One of nameString and hrefProp must be set.
if nameString == "" && href == "" {
return nil, gtserror.Newf("neither Name nor Href were set")
} }
return &gtsmodel.Mention{ return &gtsmodel.Mention{
NameString: nameString, NameString: nameString,
TargetAccountURI: hrefProp.GetIRI().String(), TargetAccountURI: href,
}, nil }, nil
} }

View file

@ -21,14 +21,16 @@ import (
"testing" "testing"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
"github.com/superseriousbusiness/activity/streams"
"github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/testrig"
) )
type ExtractMentionsTestSuite struct { type ExtractMentionsTestSuite struct {
APTestSuite APTestSuite
} }
func (suite *ExtractMentionsTestSuite) TestExtractMentions() { func (suite *ExtractMentionsTestSuite) TestExtractMentionsFromNote() {
note := suite.noteWithMentions1 note := suite.noteWithMentions1
mentions, err := ap.ExtractMentions(note) mentions, err := ap.ExtractMentions(note)
@ -44,6 +46,101 @@ func (suite *ExtractMentionsTestSuite) TestExtractMentions() {
suite.Equal("https://gts.superseriousbusiness.org/users/f0x", m2.TargetAccountURI) suite.Equal("https://gts.superseriousbusiness.org/users/f0x", m2.TargetAccountURI)
} }
func (suite *ExtractMentionsTestSuite) TestExtractMentions() {
newMention := func(nameString string, href string) ap.Mentionable {
mention := streams.NewActivityStreamsMention()
if nameString != "" {
nameProp := streams.NewActivityStreamsNameProperty()
nameProp.AppendXMLSchemaString(nameString)
mention.SetActivityStreamsName(nameProp)
}
if href != "" {
hrefProp := streams.NewActivityStreamsHrefProperty()
hrefProp.SetIRI(testrig.URLMustParse(href))
mention.SetActivityStreamsHref(hrefProp)
}
return mention
}
type test struct {
nameString string
href string
expectedNameString string
expectedHref string
expectedErr string
}
for i, t := range []test{
{
// Mention with both Name and Href set, should be fine.
nameString: "@someone@example.org",
href: "https://example.org/@someone",
expectedNameString: "@someone@example.org",
expectedHref: "https://example.org/@someone",
expectedErr: "",
},
{
// Mention with just Href set, should be fine.
nameString: "",
href: "https://example.org/@someone",
expectedNameString: "",
expectedHref: "https://example.org/@someone",
expectedErr: "",
},
{
// Mention with just Name set, should be fine.
nameString: "@someone@example.org",
href: "",
expectedNameString: "@someone@example.org",
expectedHref: "",
expectedErr: "",
},
{
// Mention with nothing set, not fine!
nameString: "",
href: "",
expectedNameString: "",
expectedHref: "",
expectedErr: "ExtractMention: neither Name nor Href were set",
},
} {
apMention := newMention(t.nameString, t.href)
mention, err := ap.ExtractMention(apMention)
if err != nil {
if errString := err.Error(); errString != t.expectedErr {
suite.Fail("",
"test %d expected error %s, got %s",
i+1, t.expectedErr, errString,
)
}
continue
} else if t.expectedErr != "" {
suite.Fail("",
"test %d expected error %s, got no error",
i+1, t.expectedErr,
)
}
if mention.NameString != t.expectedNameString {
suite.Fail("",
"test %d expected nameString %s, got %s",
i+1, t.expectedNameString, mention.NameString,
)
}
if mention.TargetAccountURI != t.expectedHref {
suite.Fail("",
"test %d expected href %s, got %s",
i+1, t.expectedHref, mention.TargetAccountURI,
)
}
}
}
func TestExtractMentionsTestSuite(t *testing.T) { func TestExtractMentionsTestSuite(t *testing.T) {
suite.Run(t, &ExtractMentionsTestSuite{}) suite.Run(t, &ExtractMentionsTestSuite{})
} }

View file

@ -163,6 +163,37 @@ func (d *Dereferencer) getAccountByURI(ctx context.Context, requestUser string,
// or a remote model whose last_fetched date is beyond a certain interval, the account will be dereferenced. In the case of dereferencing, some low-priority // or a remote model whose last_fetched date is beyond a certain interval, the account will be dereferenced. In the case of dereferencing, some low-priority
// account information may be enqueued for asynchronous fetching, e.g. featured account statuses (pins). An ActivityPub object indicates the account was dereferenced. // account information may be enqueued for asynchronous fetching, e.g. featured account statuses (pins). An ActivityPub object indicates the account was dereferenced.
func (d *Dereferencer) GetAccountByUsernameDomain(ctx context.Context, requestUser string, username string, domain string) (*gtsmodel.Account, ap.Accountable, error) { func (d *Dereferencer) GetAccountByUsernameDomain(ctx context.Context, requestUser string, username string, domain string) (*gtsmodel.Account, ap.Accountable, error) {
account, apubAcc, err := d.getAccountByUsernameDomain(
ctx,
requestUser,
username,
domain,
)
if err != nil {
return nil, nil, err
}
if apubAcc != nil {
// This account was updated, enqueue re-dereference featured posts.
d.state.Workers.Federator.MustEnqueueCtx(ctx, func(ctx context.Context) {
if err := d.dereferenceAccountFeatured(ctx, requestUser, account); err != nil {
log.Errorf(ctx, "error fetching account featured collection: %v", err)
}
})
}
return account, apubAcc, nil
}
// getAccountByUsernameDomain is a package internal form
// of .GetAccountByUsernameDomain() that doesn't bother
// dereferencing featured posts.
func (d *Dereferencer) getAccountByUsernameDomain(
ctx context.Context,
requestUser string,
username string,
domain string,
) (*gtsmodel.Account, ap.Accountable, error) {
if domain == config.GetHost() || domain == config.GetAccountDomain() { if domain == config.GetHost() || domain == config.GetAccountDomain() {
// We do local lookups using an empty domain, // We do local lookups using an empty domain,
// else it will fail the db search below. // else it will fail the db search below.
@ -196,13 +227,6 @@ func (d *Dereferencer) GetAccountByUsernameDomain(ctx context.Context, requestUs
return nil, nil, err return nil, nil, err
} }
// This account was updated, enqueue dereference featured posts.
d.state.Workers.Federator.MustEnqueueCtx(ctx, func(ctx context.Context) {
if err := d.dereferenceAccountFeatured(ctx, requestUser, account); err != nil {
log.Errorf(ctx, "error fetching account featured collection: %v", err)
}
})
return account, apubAcc, nil return account, apubAcc, nil
} }

View file

@ -36,6 +36,7 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/media" "github.com/superseriousbusiness/gotosocial/internal/media"
"github.com/superseriousbusiness/gotosocial/internal/transport" "github.com/superseriousbusiness/gotosocial/internal/transport"
"github.com/superseriousbusiness/gotosocial/internal/util"
) )
// statusUpToDate returns whether the given status model is both updateable // statusUpToDate returns whether the given status model is both updateable
@ -342,35 +343,110 @@ func (d *Dereferencer) enrichStatus(
return latestStatus, apubStatus, nil return latestStatus, apubStatus, nil
} }
func (d *Dereferencer) fetchStatusMentions(ctx context.Context, requestUser string, existing, status *gtsmodel.Status) error { // populateMentionTarget tries to populate the given
// Allocate new slice to take the yet-to-be created mention IDs. // mention with the correct TargetAccount and (if not
status.MentionIDs = make([]string, len(status.Mentions)) // yet set) TargetAccountURI, returning the populated
// mention.
for i := range status.Mentions { //
mention := status.Mentions[i] // Will check on the existing status if the mention
// is already there and populated; if so, existing
// Look for existing mention with target account URI first. // mention will be returned along with `true`.
existing, ok := existing.GetMentionByTargetURI(mention.TargetAccountURI) //
if ok && existing.ID != "" { // Otherwise, this function will try to parse first
status.Mentions[i] = existing // the Href of the mention, and then the namestring,
status.MentionIDs[i] = existing.ID // to see who it targets, and go fetch that account.
continue func (d *Dereferencer) populateMentionTarget(
ctx context.Context,
mention *gtsmodel.Mention,
requestUser string,
existing, status *gtsmodel.Status,
) (
*gtsmodel.Mention,
bool, // True if mention already exists in the DB.
error,
) {
// Mentions can be created using Name or Href.
// Prefer Href (TargetAccountURI), fall back to Name.
if mention.TargetAccountURI != "" {
// Look for existing mention with this URI.
// If we already have it we can return early.
existingMention, ok := existing.GetMentionByTargetURI(mention.TargetAccountURI)
if ok && existingMention.ID != "" {
return existingMention, true, nil
} }
// Ensure that mention account URI is parseable. // Ensure that mention account URI is parseable.
accountURI, err := url.Parse(mention.TargetAccountURI) accountURI, err := url.Parse(mention.TargetAccountURI)
if err != nil { if err != nil {
log.Errorf(ctx, "invalid account uri %q: %v", mention.TargetAccountURI, err) err = gtserror.Newf("invalid account uri %q: %w", mention.TargetAccountURI, err)
continue return nil, false, err
} }
// Ensure we have the account of the mention target dereferenced. // Ensure we have the account of the mention target dereferenced.
mention.TargetAccount, _, err = d.getAccountByURI(ctx, requestUser, accountURI) mention.TargetAccount, _, err = d.getAccountByURI(ctx, requestUser, accountURI)
if err != nil { if err != nil {
log.Errorf(ctx, "failed to dereference account %s: %v", accountURI, err) err = gtserror.Newf("failed to dereference account %s: %w", accountURI, err)
return nil, false, err
}
} else {
// Href wasn't set. Find the target account using namestring.
username, domain, err := util.ExtractNamestringParts(mention.NameString)
if err != nil {
err = gtserror.Newf("failed to parse namestring %s: %w", mention.NameString, err)
return nil, false, err
}
mention.TargetAccount, _, err = d.getAccountByUsernameDomain(ctx, requestUser, username, domain)
if err != nil {
err = gtserror.Newf("failed to dereference account %s: %w", mention.NameString, err)
return nil, false, err
}
// Look for existing mention with this URI.
mention.TargetAccountURI = mention.TargetAccount.URI
existingMention, ok := existing.GetMentionByTargetURI(mention.TargetAccountURI)
if ok && existingMention.ID != "" {
return existingMention, true, nil
}
}
// At this point, mention.TargetAccountURI
// and mention.TargetAccount must be set.
return mention, false, nil
}
func (d *Dereferencer) fetchStatusMentions(ctx context.Context, requestUser string, existing, status *gtsmodel.Status) error {
// Allocate new slice to take the yet-to-be created mention IDs.
status.MentionIDs = make([]string, len(status.Mentions))
for i := range status.Mentions {
var (
mention = status.Mentions[i]
alreadyExists bool
err error
)
mention, alreadyExists, err = d.populateMentionTarget(
ctx,
mention,
requestUser,
existing,
status,
)
if err != nil {
log.Errorf(ctx, "failed to derive mention: %v", err)
continue continue
} }
if alreadyExists {
// This mention was already attached
// to the status, use it and continue.
status.Mentions[i] = mention
status.MentionIDs[i] = mention.ID
continue
}
// This mention didn't exist yet.
// Generate new ID according to status creation. // Generate new ID according to status creation.
// TODO: update this to use "edited_at" when we add // TODO: update this to use "edited_at" when we add
// support for edited status revision history. // support for edited status revision history.

View file

@ -21,17 +21,20 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"strings"
"codeberg.org/gruf/go-logger/v2/level" "codeberg.org/gruf/go-logger/v2/level"
"github.com/superseriousbusiness/activity/pub" "github.com/superseriousbusiness/activity/pub"
"github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/config"
"github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/id"
"github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/messages"
"github.com/superseriousbusiness/gotosocial/internal/util"
) )
// Create adds a new entry to the database which must be able to be // Create adds a new entry to the database which must be able to be
@ -288,11 +291,34 @@ func (f *federatingDB) createStatusable(
} }
func (f *federatingDB) shouldAcceptStatusable(ctx context.Context, receiver *gtsmodel.Account, requester *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { func (f *federatingDB) shouldAcceptStatusable(ctx context.Context, receiver *gtsmodel.Account, requester *gtsmodel.Account, status *gtsmodel.Status) (bool, error) {
host := config.GetHost()
accountDomain := config.GetAccountDomain()
// Check whether status mentions the receiver, // Check whether status mentions the receiver,
// this is the quickest check so perform it first. // this is the quickest check so perform it first.
// Prefer checking using mention Href, fall back to Name.
for _, mention := range status.Mentions { for _, mention := range status.Mentions {
if mention.TargetAccountURI == receiver.URI { targetURI := mention.TargetAccountURI
return true, nil nameString := mention.NameString
if targetURI != "" {
if targetURI == receiver.URI || targetURI == receiver.URL {
// Target URI or URL match;
// receiver is mentioned.
return true, nil
}
} else if nameString != "" {
username, domain, err := util.ExtractNamestringParts(nameString)
if err != nil {
return false, gtserror.Newf("error checking if mentioned: %w", err)
}
if (domain == host || domain == accountDomain) &&
strings.EqualFold(username, receiver.Username) {
// Username + domain match;
// receiver is mentioned.
return true, nil
}
} }
} }