[chore] chore rationalise http return codes for activitypub handlers (#2540)

* some small code fixups and changes

* add check in ResolveIncomingActivity for transient activity types (i.e. activity ID is nil)

* update test to handle new transient behaviour
This commit is contained in:
kim 2024-01-18 16:11:13 +00:00 committed by tobi
parent f5314c0068
commit 5d44ad7599
5 changed files with 29 additions and 30 deletions

View file

@ -56,8 +56,9 @@ func putMap(m map[string]any) {
mapPool.Put(m)
}
// ResolveActivity is a util function for pulling a pub.Activity type out of an incoming request body.
func ResolveIncomingActivity(r *http.Request) (pub.Activity, gtserror.WithCode) {
// ResolveActivity is a util function for pulling a pub.Activity type out of an incoming request body,
// returning the resolved activity type, error and whether to accept activity (false = transient i.e. ignore).
func ResolveIncomingActivity(r *http.Request) (pub.Activity, bool, gtserror.WithCode) {
// Get "raw" map
// destination.
raw := getMap()
@ -68,7 +69,7 @@ func ResolveIncomingActivity(r *http.Request) (pub.Activity, gtserror.WithCode)
// Decode the JSON body stream into "raw" map.
if err := json.NewDecoder(r.Body).Decode(&raw); err != nil {
err := gtserror.Newf("error decoding json: %w", err)
return nil, gtserror.NewErrorInternalError(err)
return nil, false, gtserror.NewErrorInternalError(err)
}
// Resolve "raw" JSON to vocab.Type.
@ -76,25 +77,29 @@ func ResolveIncomingActivity(r *http.Request) (pub.Activity, gtserror.WithCode)
if err != nil {
if !streams.IsUnmatchedErr(err) {
err := gtserror.Newf("error matching json to type: %w", err)
return nil, gtserror.NewErrorInternalError(err)
return nil, false, gtserror.NewErrorInternalError(err)
}
// Respond with bad request; we just couldn't
// match the type to one that we know about.
const text = "body json not resolvable as ActivityStreams type"
return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
return nil, false, gtserror.NewErrorBadRequest(errors.New(text), text)
}
// Ensure this is an Activity type.
activity, ok := t.(pub.Activity)
if !ok {
text := fmt.Sprintf("cannot resolve vocab type %T as pub.Activity", t)
return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
return nil, false, gtserror.NewErrorBadRequest(errors.New(text), text)
}
if activity.GetJSONLDId() == nil {
const text = "missing ActivityStreams id property"
return nil, gtserror.NewErrorBadRequest(errors.New(text), text)
// missing ID indicates a transient ID as per:
//
// all objects distributed by the ActivityPub protocol MUST have unique global identifiers,
// unless they are intentionally transient (short lived activities that are not intended to
// be able to be looked up, such as some kinds of chat messages or game notifications).
return nil, false, nil
}
// Normalize any Statusable, Accountable, Pollable fields found.
@ -104,7 +109,7 @@ func ResolveIncomingActivity(r *http.Request) (pub.Activity, gtserror.WithCode)
// Release.
putMap(raw)
return activity, nil
return activity, true, nil
}
// ResolveStatusable tries to resolve the given bytes into an ActivityPub Statusable representation.

View file

@ -478,15 +478,17 @@ func (suite *InboxPostTestSuite) TestPostEmptyCreate() {
targetAccount = suite.testAccounts["local_account_1"]
)
// Post a create with no object.
// Post a create with no object, this
// should get accepted and silently dropped
// as the lack of ID marks it as transient.
create := streams.NewActivityStreamsCreate()
suite.inboxPost(
create,
requestingAccount,
targetAccount,
http.StatusBadRequest,
`{"error":"Bad Request: missing ActivityStreams id property"}`,
http.StatusAccepted,
`{"status":"Accepted"}`,
suite.signatureCheck,
)
}

View file

@ -143,10 +143,12 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr
have not yet applied authorization (ie., blocks).
*/
// Obtain the activity; reject unknown activities.
activity, errWithCode := ap.ResolveIncomingActivity(r)
// Resolve the activity, rejecting badly formatted / transient.
activity, ok, errWithCode := ap.ResolveIncomingActivity(r)
if errWithCode != nil {
return false, errWithCode
} else if !ok { // transient
return false, nil
}
// Set additional context data. Primarily this means

View file

@ -45,16 +45,11 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
return nil // Already processed.
}
rejectObject := reject.GetActivityStreamsObject()
if rejectObject == nil {
return errors.New("Reject: no object set on vocab.ActivityStreamsReject")
}
for _, obj := range ap.ExtractObjects(reject) {
for iter := rejectObject.Begin(); iter != rejectObject.End(); iter = iter.Next() {
// check if the object is an IRI
if iter.IsIRI() {
if obj.IsIRI() {
// we have just the URI of whatever is being rejected, so we need to find out what it is
rejectedObjectIRI := iter.GetIRI()
rejectedObjectIRI := obj.GetIRI()
if uris.IsFollowPath(rejectedObjectIRI) {
// REJECT FOLLOW
followReq, err := f.state.DB.GetFollowRequestByURI(ctx, rejectedObjectIRI.String())
@ -78,15 +73,10 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
}
}
// check if iter is an AP object / type
if iter.GetType() == nil {
continue
}
if iter.GetType().GetTypeName() == ap.ActivityFollow {
if t := obj.GetType(); t != nil {
// we have the whole object so we can figure out what we're rejecting
// REJECT FOLLOW
asFollow, ok := iter.GetType().(vocab.ActivityStreamsFollow)
asFollow, ok := t.(vocab.ActivityStreamsFollow)
if !ok {
return errors.New("Reject: couldn't parse follow into vocab.ActivityStreamsFollow")
}

View file

@ -78,7 +78,7 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
}
}
return nil
return errs.Combine()
}
func (f *federatingDB) undoFollow(