From 701475b6e008d1236d5a85e49f34185d0d1148fd Mon Sep 17 00:00:00 2001 From: Cory Slep Date: Wed, 4 Apr 2018 00:23:55 +0200 Subject: [PATCH] Fix bugs with delivering messages. This was uncovered during writing tests where both the Social and Federative APIs are enabled. The actorObject interface was overused partially due to my earlier confusion; the introduction of the internal actor interface resolved this. Finally, the HttpClient interface was introduced to be able to mock out calls in testing. It may also prove useful for future applications using this library. --- pub/fed.go | 6 +++--- pub/interfaces.go | 12 ++++++++++++ pub/internal.go | 21 +++++++++++---------- pub/resolvers.go | 17 ++++++++++++++--- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/pub/fed.go b/pub/fed.go index d16736c..02df564 100644 --- a/pub/fed.go +++ b/pub/fed.go @@ -66,7 +66,7 @@ func NewSocialPubber(clock Clock, app Application, socialApp SocialApp, cb Callb // servers. All are required unless explicitly stated otherwise. type DeliveryOptions struct { // Client to use when sending http requests to federating peers. - Client *http.Client + Client HttpClient // Agent string to send in http requests. Agent string // Maximum unnesting done when determining who to send messages to. Must @@ -110,7 +110,7 @@ func NewFederatingPubber(clock Clock, app Application, fApp FederateApp, cb Call // NewPubber provides a Pubber that implements both the Social API and the // Federating API in ActivityPub. -func NewPubber(clock Clock, app Application, sApp SocialApp, fApp FederateApp, server, client Callbacker, d DeliveryOptions) Pubber { +func NewPubber(clock Clock, app Application, sApp SocialApp, fApp FederateApp, client, server Callbacker, d DeliveryOptions) Pubber { return &federator{ Clock: clock, App: app, @@ -151,7 +151,7 @@ type federator struct { // Client is used to federate with other ActivityPub servers. // // It is only required if EnableServer is true. - Client *http.Client + Client HttpClient // Agent is the User-Agent string to use in HTTP headers when // federating with another server. It will automatically be appended // with '(go-fed ActivityPub)'. diff --git a/pub/interfaces.go b/pub/interfaces.go index 3e573b1..90bff18 100644 --- a/pub/interfaces.go +++ b/pub/interfaces.go @@ -25,6 +25,11 @@ type Clock interface { Now() time.Time } +// HttpClient sends http requests. +type HttpClient interface { + Do(req *http.Request) (*http.Response, error) +} + // Application is provided by users of this library in order to implement a // social-federative-web application. // @@ -184,6 +189,13 @@ type typeIder interface { Serialize() (m map[string]interface{}, e error) } +// actor is an object that is an ActivityPub Actor. The specification is more +// strict than what we include here, only for our internal use. +type actor interface { + HasInbox() (ok bool) + GetInbox() (v url.URL) +} + // actorObject is an object that has "actor" or "attributedTo" properties, // representing the author or originator of the object. type actorObject interface { diff --git a/pub/internal.go b/pub/internal.go index 4acf2c3..5f03bc0 100644 --- a/pub/internal.go +++ b/pub/internal.go @@ -63,7 +63,7 @@ func isPublic(s string) bool { // dereference makes an HTTP GET request to an IRI in order to obtain the // ActivityStream representation. -func dereference(c *http.Client, u url.URL, agent string) ([]byte, error) { +func dereference(c HttpClient, u url.URL, agent string) ([]byte, error) { // TODO: (section 7.1) The server MUST dereference the collection (with the user's credentials) req, err := http.NewRequest("GET", u.String(), nil) if err != nil { @@ -86,7 +86,7 @@ func dereference(c *http.Client, u url.URL, agent string) ([]byte, error) { // postToOutbox will attempt to send a POST request to the given URL with the // body set to the provided bytes. -func postToOutbox(c *http.Client, b []byte, to url.URL, agent string) error { +func postToOutbox(c HttpClient, b []byte, to url.URL, agent string) error { byteCopy := make([]byte, 0, len(b)) copy(b, byteCopy) buf := bytes.NewBuffer(byteCopy) @@ -234,11 +234,11 @@ func (c *federator) prepare(o deliverableObject) ([]url.URL, error) { // resolveInboxes takes a list of Actor id URIs and returns them as concrete // instances of actorObject. It applies recursively when it encounters a target // that is a Collection or OrderedCollection. -func (c *federator) resolveInboxes(r []url.URL, depth int, max int) ([]actorObject, error) { +func (c *federator) resolveInboxes(r []url.URL, depth int, max int) ([]actor, error) { if depth >= max { return nil, nil } - a := make([]actorObject, 0, len(r)) + a := make([]actor, 0, len(r)) for _, u := range r { // Do not retry here -- if a dereference fails, then fail the // entire delivery. @@ -250,7 +250,7 @@ func (c *federator) resolveInboxes(r []url.URL, depth int, max int) ([]actorObje if err = json.Unmarshal(resp, &m); err != nil { return nil, err } - var actor actorObject + var actor actor var co *streams.Collection var oc *streams.OrderedCollection var cp *streams.CollectionPage @@ -314,7 +314,7 @@ func (c *federator) resolveInboxes(r []url.URL, depth int, max int) ([]actorObje } // getInboxes extracts the 'inbox' IRIs from actors. -func getInboxes(a []actorObject) []url.URL { +func getInboxes(a []actor) []url.URL { var u []url.URL for _, actor := range a { if actor.HasInbox() { @@ -393,9 +393,10 @@ func dedupeIRIs(recipients, ignored []url.URL) (out []url.URL) { ignoredMap[elem] = true } outMap := make(map[url.URL]bool, len(recipients)) - for k, _ := range outMap { - if !ignoredMap[k] { + for _, k := range recipients { + if !ignoredMap[k] && !outMap[k] { out = append(out, k) + outMap[k] = true } } return @@ -559,7 +560,7 @@ func getAudienceIRIs(o deliverableObject) []url.URL { // doForCollectionPage applies a function over a collection and its subsequent // pages recursively. It returns the first non-nil error it encounters. -func doForCollectionPage(h *http.Client, agent string, cb func(c vocab.CollectionPageType) error, c vocab.CollectionPageType) error { +func doForCollectionPage(h HttpClient, agent string, cb func(c vocab.CollectionPageType) error, c vocab.CollectionPageType) error { err := cb(c) if err != nil { return err @@ -614,7 +615,7 @@ func doForCollectionPage(h *http.Client, agent string, cb func(c vocab.Collectio // doForOrderedCollectionPage applies a function over a collection and its // subsequent pages recursively. It returns the first non-nil error it // encounters. -func doForOrderedCollectionPage(h *http.Client, agent string, cb func(c vocab.OrderedCollectionPageType) error, c vocab.OrderedCollectionPageType) error { +func doForOrderedCollectionPage(h HttpClient, agent string, cb func(c vocab.OrderedCollectionPageType) error, c vocab.OrderedCollectionPageType) error { err := cb(c) if err != nil { return err diff --git a/pub/resolvers.go b/pub/resolvers.go index 7616c0c..85d778b 100644 --- a/pub/resolvers.go +++ b/pub/resolvers.go @@ -37,11 +37,11 @@ func ToPubObject(m map[string]interface{}) (t []PubObject, e error) { func getActorObject(m map[string]interface{}) (actorObject, error) { var a actorObject - err := toActorResolver(&a).Deserialize(m) + err := toActorObjectResolver(&a).Deserialize(m) return a, err } -func toActorResolver(a *actorObject) *streams.Resolver { +func toActorObjectResolver(a *actorObject) *streams.Resolver { return &streams.Resolver{ AnyObjectCallback: func(i vocab.ObjectType) error { if o, ok := i.(actorObject); ok { @@ -52,7 +52,18 @@ func toActorResolver(a *actorObject) *streams.Resolver { } } -func toActorCollectionResolver(a *actorObject, c **streams.Collection, oc **streams.OrderedCollection, cp **streams.CollectionPage, ocp **streams.OrderedCollectionPage) *streams.Resolver { +func toActorResolver(a *actor) *streams.Resolver { + return &streams.Resolver{ + AnyObjectCallback: func(i vocab.ObjectType) error { + if o, ok := i.(actor); ok { + *a = o + } + return nil + }, + } +} + +func toActorCollectionResolver(a *actor, c **streams.Collection, oc **streams.OrderedCollection, cp **streams.CollectionPage, ocp **streams.OrderedCollectionPage) *streams.Resolver { r := toActorResolver(a) r.CollectionCallback = func(i *streams.Collection) error { *c = i