diff --git a/internal/ap/client.go b/internal/ap/client.go index 5e94b1a..c6824b7 100644 --- a/internal/ap/client.go +++ b/internal/ap/client.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "math/rand/v2" + "mime" "net" "net/http" "net/url" @@ -970,6 +971,9 @@ func (c *Client) getOnce(ctx context.Context, iri string, mode fetchMode) (body switch { case resp.StatusCode >= 200 && resp.StatusCode < 300: + if err := checkJSONResponse(resp, body); err != nil { + return nil, false, fmt.Errorf("ap: GET %s: %w", target, err) + } return body, false, nil case resp.StatusCode == http.StatusGone: // 410 Gone is how Lemmy serves deleted objects (with a Tombstone @@ -999,6 +1003,25 @@ func (c *Client) getOnce(ctx context.Context, iri string, mode fetchMode) (body } } +// checkJSONResponse names the failure when a 2xx response is not JSON at +// all. It fires only when BOTH the body does not look like JSON AND the +// declared Content-Type is a non-JSON type (text/html …): a server that +// mislabels valid JSON still parses, and a JSON-typed garbage body still +// reaches the parser for its own error. Without this, a content-negotiation +// miss (an instance's proxy serving its HTML frontend with 200 to an AP +// fetch) surfaces as the opaque "payload is not a JSON object". +func checkJSONResponse(resp *http.Response, body []byte) error { + trimmed := bytes.TrimSpace(body) + if len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') { + return nil + } + mediaType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) + if err != nil || mediaType == "" || strings.Contains(mediaType, "json") { + return nil + } + return fmt.Errorf("response is %s, not JSON (content negotiation failed?)", mediaType) +} + // readBody drains the response body through the size cap and closes it. func (c *Client) readBody(resp *http.Response) ([]byte, error) { defer func() { _ = resp.Body.Close() }() diff --git a/internal/ap/client_test.go b/internal/ap/client_test.go index 22471f1..5330830 100644 --- a/internal/ap/client_test.go +++ b/internal/ap/client_test.go @@ -57,7 +57,10 @@ func TestFetchObject_SignedGET(t *testing.T) { assert.Equal(t, "https://lemmy.world/post/49131386", obj.ID) require.NotNil(t, sawRequest) - assert.Contains(t, sawRequest.Header.Get("Accept"), "application/activity+json") + // Pinned to the bare type, not a compound list: at least one Lemmy + // deployment's proxy (startrek.website) exact-matches Accept and serves + // its HTML frontend to anything else. See acceptActivityJSON. + assert.Equal(t, ContentTypeActivityJSON, sawRequest.Header.Get("Accept")) assert.Equal(t, "tidepool-test/0", sawRequest.Header.Get("User-Agent")) // The GET must carry a signature Lemmy would accept: verify it @@ -68,6 +71,54 @@ func TestFetchObject_SignedGET(t *testing.T) { assert.Equal(t, "(request-target) host date digest", fields["headers"]) } +// A 200 whose body is the instance's HTML frontend (content negotiation +// missed) must be reported as such — naming the media type — rather than as +// the opaque "payload is not a JSON object". Conversely, valid JSON served +// under a wrong Content-Type must still parse: the check is a diagnostic, +// not a strictness gate. +func TestFetchObject_NonJSONResponseNamesContentType(t *testing.T) { + t.Run("html with 200 is diagnosed", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = w.Write([]byte("\n \n spa shell")) + })) + defer server.Close() + + client := newTestClient(t, ClientOptions{}) + _, err := client.FetchObject(context.Background(), server.URL+"/c/startrek") + require.Error(t, err) + assert.Contains(t, err.Error(), "text/html") + assert.Contains(t, err.Error(), "not JSON") + assert.NotContains(t, err.Error(), "payload is not a JSON object") + }) + + t.Run("json under a non-json content-type still parses", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write(loadFixture(t, "page_lemmy_world.json")) + })) + defer server.Close() + + client := newTestClient(t, ClientOptions{}) + obj, err := client.FetchObject(context.Background(), server.URL+"/post/49131386") + require.NoError(t, err) + assert.Equal(t, TypePage, obj.Type) + }) + + t.Run("json content-type with a non-json body still reaches the parser", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", ContentTypeActivityJSON) + _, _ = w.Write([]byte(`"just a string"`)) + })) + defer server.Close() + + client := newTestClient(t, ClientOptions{}) + _, err := client.FetchObject(context.Background(), server.URL+"/x") + require.Error(t, err) + assert.Contains(t, err.Error(), "payload is not a JSON object") + }) +} + func TestFetchObject_StatusMapping(t *testing.T) { cases := []struct { status int diff --git a/internal/ap/vocab.go b/internal/ap/vocab.go index dc73047..d3d6b17 100644 --- a/internal/ap/vocab.go +++ b/internal/ap/vocab.go @@ -19,11 +19,20 @@ import ( ) // AS2 media types. Lemmy serves and accepts application/activity+json; -// Mastodon prefers the ld+json profile form. We send both in Accept. +// Mastodon prefers the ld+json profile form when SERVING. +// +// Accept is the bare activity+json type, deliberately not a compound +// "activity+json, ld+json; profile=...; q=0.9" list: some Lemmy deployments +// (startrek.website, 2026-08, an Elestio-packaged nginx) route to the +// backend only when Accept is byte-for-byte one of the two types — anything +// with a comma or q-value falls through to lemmy-ui and comes back as a +// 200 text/html SPA shell. The bare type is exactly what Lemmy's +// activitypub-federation crate, PieFed and Mbin send on their own fetches, +// and every AP implementation serves it, so nothing is lost by matching them. const ( ContentTypeActivityJSON = "application/activity+json" ContentTypeLDJSON = `application/ld+json; profile="https://www.w3.org/ns/activitystreams"` - acceptActivityJSON = ContentTypeActivityJSON + `, application/ld+json; profile="https://www.w3.org/ns/activitystreams"; q=0.9` + acceptActivityJSON = ContentTypeActivityJSON ) // PublicAudience is the special AS2 collection meaning "public".