[bugfix] Don't return Internal Server Error when searching for URIs t…

…hat don't return AP JSON (#2550) * [bugfix] Don't return Internal Server Error when searching for URIs that don't return AP JSON * don't pass map pointer
superseriousbusiness · Feb 14, 2024 · ad6f756 · ad6f756
1 parent 1188971
commit ad6f756
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 19 deletions.
diff --git a/internal/ap/resolve.go b/internal/ap/resolve.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/superseriousbusiness/activity/pub"
 	"github.com/superseriousbusiness/activity/streams"
+	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 )
 
@@ -56,6 +57,35 @@ func putMap(m map[string]any) {
 	mapPool.Put(m)
 }
 
+// bytesToType tries to parse the given bytes slice
+// as a JSON ActivityPub type, failing if the input
+// bytes are not parseable as JSON, or do not parse
+// to an ActivityPub that we can understand.
+//
+// The given map pointer will also be populated with
+// the parsed JSON, to allow further processing.
+func bytesToType(
+	ctx context.Context,
+	b []byte,
+	raw map[string]any,
+) (vocab.Type, error) {
+	// Unmarshal the raw JSON bytes into a "raw" map.
+	// This will fail if the input is not parseable
+	// as JSON; eg., a remote has returned HTML as a
+	// fallback response to an ActivityPub JSON request.
+	if err := json.Unmarshal(b, &raw); err != nil {
+		return nil, gtserror.NewfAt(3, "error unmarshalling bytes into json: %w", err)
+	}
+
+	// Resolve an ActivityStreams type.
+	t, err := streams.ToType(ctx, raw)
+	if err != nil {
+		return nil, gtserror.NewfAt(3, "error resolving json into ap vocab type: %w", err)
+	}
+
+	return t, nil
+}
+
 // 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) {
@@ -121,15 +151,11 @@ func ResolveStatusable(ctx context.Context, b []byte) (Statusable, error) {
 	// destination.
 	raw := getMap()
 
-	// Unmarshal the raw JSON data in a "raw" JSON map.
-	if err := json.Unmarshal(b, &raw); err != nil {
-		return nil, gtserror.Newf("error unmarshalling bytes into json: %w", err)
-	}
-
-	// Resolve an ActivityStreams type from JSON.
-	t, err := streams.ToType(ctx, raw)
+	// Convert raw bytes to an AP type.
+	// This will also populate the map.
+	t, err := bytesToType(ctx, b, raw)
 	if err != nil {
-		return nil, gtserror.Newf("error resolving json into ap vocab type: %w", err)
+		return nil, gtserror.SetWrongType(err)
 	}
 
 	// Attempt to cast as Statusable.
@@ -166,15 +192,11 @@ func ResolveAccountable(ctx context.Context, b []byte) (Accountable, error) {
 	// destination.
 	raw := getMap()
 
-	// Unmarshal the raw JSON data in a "raw" JSON map.
-	if err := json.Unmarshal(b, &raw); err != nil {
-		return nil, gtserror.Newf("error unmarshalling bytes into json: %w", err)
-	}
-
-	// Resolve an ActivityStreams type from JSON.
-	t, err := streams.ToType(ctx, raw)
+	// Convert raw bytes to an AP type.
+	// This will also populate the map.
+	t, err := bytesToType(ctx, b, raw)
 	if err != nil {
-		return nil, gtserror.Newf("error resolving json into ap vocab type: %w", err)
+		return nil, gtserror.SetWrongType(err)
 	}
 
 	// Attempt to cast as Statusable.

diff --git a/internal/ap/resolve_test.go b/internal/ap/resolve_test.go
@@ -47,6 +47,29 @@ func (suite *ResolveTestSuite) TestResolveDocumentAsAccountable() {
 	suite.Nil(accountable)
 }
 
+func (suite *ResolveTestSuite) TestResolveHTMLAsAccountable() {
+	b := []byte(`<!DOCTYPE html>
+	<title>.</title>`)
+
+	accountable, err := ap.ResolveAccountable(context.Background(), b)
+	suite.True(gtserror.IsWrongType(err))
+	suite.EqualError(err, "ResolveAccountable: error unmarshalling bytes into json: invalid character '<' looking for beginning of value")
+	suite.Nil(accountable)
+}
+
+func (suite *ResolveTestSuite) TestResolveNonAPJSONAsAccountable() {
+	b := []byte(`{
+  "@context": "definitely a legit context muy lord",
+  "type": "definitely an account muy lord",
+  "pee pee":"poo poo"
+}`)
+
+	accountable, err := ap.ResolveAccountable(context.Background(), b)
+	suite.True(gtserror.IsWrongType(err))
+	suite.EqualError(err, "ResolveAccountable: error resolving json into ap vocab type: activity stream did not match any known types")
+	suite.Nil(accountable)
+}
+
 func TestResolveTestSuite(t *testing.T) {
 	suite.Run(t, &ResolveTestSuite{})
 }
diff --git a/internal/gtserror/error.go b/internal/gtserror/error.go
@@ -55,9 +55,15 @@ func SetUnretrievable(err error) error {
 	return errors.WithValue(err, unrtrvableKey, struct{}{})
 }
 
-// IsWrongType checks error for a stored "wrong type" flag. Wrong type
-// indicates that an ActivityPub URI returned a type we weren't expecting:
-// Statusable instead of Accountable, or vice versa, for example.
+// IsWrongType checks error for a stored "wrong type" flag.
+// Wrong type indicates that an ActivityPub URI returned a
+// type we weren't expecting. For example:
+//
+//   - HTML instead of JSON.
+//   - Normal JSON instead of ActivityPub JSON.
+//   - Statusable instead of Accountable.
+//   - Accountable instead of Statusable.
+//   - etc.
 func IsWrongType(err error) bool {
 	_, ok := errors.Value(err, wrongTypeKey).(struct{})
 	return ok