Use standard-compliant Accept header when fetching

Oneric commented

2024-04-11 20:59:24 +00:00

Member

Quoting #730

Section 3.2 of the ActivityPub spec reads

The client MUST specify an Accept header with the application/ld+json; profile="https://www.w3.org/ns/activitystreams" media type in order to retrieve the activity.

But currently, we perform fetches with application/activity+json instead

lib/pleroma/object/fetcher.ex
Line 327 in 2d43903
[{"accept", "application/activity+json"}]

The same section also tells us

Servers MAY use HTTP content negotiation as defined in [RFC7231] to select the type of data to return in response to a request, but MUST present the ActivityStreams object representation in response to application/ld+json; profile="https://www.w3.org/ns/activitystreams", and SHOULD also present the ActivityStreams representation in response to application/activity+json as well.

The SHOULD part is why we have (currently) little trouble with this in practice, but still we shouldn’t break spec here for no reason.

In the end only mocks from one file needed updating for the new headers.

Fixes: #730

Quoting #730 > [Section 3.2 of the ActivityPub spec](https://www.w3.org/TR/activitypub/#retrieving-objects) reads > > > The client MUST specify an Accept header with the application/ld+json; profile="https://www.w3.org/ns/activitystreams" media type in order to retrieve the activity. > > But currently, we perform fetches with application/activity+json instead > https://akkoma.dev/AkkomaGang/akkoma/src/commit/2d439034ca801b704536cb05483e012d62c2d52e/lib/pleroma/object/fetcher.ex#L327 > > The same section also tells us > > > Servers MAY use HTTP content negotiation as defined in [RFC7231] to select the type of data to return in response to a request, but MUST present the ActivityStreams object representation in response to application/ld+json; profile="https://www.w3.org/ns/activitystreams", and SHOULD also present the ActivityStreams representation in response to application/activity+json as well. > > The SHOULD part is why we have (currently) little trouble with this in practice, but still we shouldn’t break spec here for no reason. In the end only mocks from one file needed updating for the new headers. Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/730

floatingghost commented

2024-04-11 21:01:55 +00:00

Owner

hm, but we can't actually process jsonLD - my first thought is that if we were actually given pure LD we'd just crash out

Oneric commented

2024-04-11 21:25:10 +00:00

Author

Member

I don’t think this makes any difference here, the ActivityStream profile the AP-spec-compliant header references already imposes some restrictions (e.g. having gone through JSON-LD compaction) and in fact is the same spec which defines application/activity+json. The ActivityStreams 2.0 spec also already declares both those media types to be equivalent (but unlike the higher-level AP spec, AS prefers activity+json).

Taking a look at other servers now, interestingly most seem to use multiple Accept types though details differ and i’m not sure if this is actually (still) required:

Misskey uses adds both to Accept, giving activity+json higher priority
GTS adds both to Accept, giving the ld+json one higher priority
Mastodon also adds two types, the first one being application/activity+json and the second application/ld+json without a profile (now this would be problematic for us)

(I’m not sure about any of them, but i believe Mastodon and Misskey don’t process pure JSON-LD either?)

I don’t think this makes any difference here, the ActivityStream profile the AP-spec-compliant header references already imposes some restrictions *(e.g. having gone through JSON-LD compaction)* and in fact is the same spec which defines [`application/activity+json`](https://www.w3.org/TR/activitystreams-core/#media-type). The ActivityStreams 2.0 spec also already declares both those media types to be equivalent *(but unlike the higher-level AP spec, AS prefers `activity+json`)*. Taking a look at other servers now, interestingly most seem to use multiple `Accept` types though details differ and i’m not sure if this is actually (still) required: - [Misskey uses](https://github.com/misskey-dev/misskey/blob/eb1ef1484afbdb09407a603ff69414e7f88bb9ff/packages/backend/src/core/HttpRequestService.ts#L119) adds both to `Accept`, giving `activity+json` higher priority - [GTS adds both](https://github.com/superseriousbusiness/gotosocial/blob/2bafd7daf542d985ee76d9079a30a602cb7be827/internal/transport/dereference.go#L56C95-L56C97) to `Accept`, giving the `ld+json` one higher priority - [Mastodon also adds two types](https://github.com/mastodon/mastodon/blob/05eda8d19330a9c27c0cf07de19a87edff269057/app/helpers/jsonld_helper.rb#L222), the first one being `application/activity+json` and the second `application/ld+json` **without** a profile *(now this would be problematic for us)* *(I’m not sure about any of them, but i _believe_ Mastodon and Misskey don’t process pure JSON-LD either?)*

Oneric force-pushed fetch_std-accept-hdr from a4ce87a231 to fae0a14ee8

2024-04-11 22:29:41 +00:00

Compare

Oneric commented

2024-04-11 22:34:44 +00:00

Author

Member

Ok, GoToSocial added application/activity+json in 2022.05, to workaround Owncast mistakenly only responding to application/activity+json and application/json. The Owncast bug was fixed a month prior to that (but presumably didn’t make it into a release yet).

Although there’s no known implementation requiring the non-compliant header, perhaps it’s best to still set both values in Accept (preferring the spec-compliant one). It’s easy for us to do and given the prevalence of multiple Accept values it seems unlikely any implementation will have issues with more than one entry here.

Updated the patch accordingly; using two "accept" headers rather than a pre-merged one with a separating comma seemed more idiomatic for Plug and allows get_*_headers to correctly return a list of all accepted types in tests

Also made sure this can still fetch from Akkoma, Misskey and Mastodon instances (if auth-fetch is disabled)

Ok, [GoToSocial added `application/activity+json`](https://github.com/superseriousbusiness/gotosocial/pull/611) in 2022.05, to workaround Owncast mistakenly only responding to `application/activity+json` and `application/json`. The Owncast bug [was fixed](https://github.com/owncast/owncast/issues/1827) a month prior to that *(but presumably didn’t make it into a release yet)*. Although there’s no known implementation requiring the non-compliant header, perhaps it’s best to still set both values in `Accept` (preferring the spec-compliant one). It’s easy for us to do and given the prevalence of multiple `Accept` values it seems unlikely any implementation will have issues with more than one entry here. Updated the patch accordingly; using two `"accept"` headers rather than a pre-merged one with a separating comma seemed more idiomatic for Plug and allows `get_*_headers` to correctly return a list of all accepted types in tests Also made sure this can still fetch from Akkoma, Misskey and Mastodon instances *(if auth-fetch is disabled)*

floatingghost commented

2024-04-12 18:28:22 +00:00

Owner

all passes, should be good, thanks a lot!

floatingghost merged commit e2e4f53585 into develop

2024-04-12 18:28:27 +00:00

floatingghost deleted branch fetch_std-accept-hdr

2024-04-12 18:28:28 +00:00

floatingghost referenced this pull request from a commit

2024-04-12 18:28:29 +00:00

Merge pull request 'Use standard-compliant Accept header when fetching' (#740) from Oneric/akkoma:fetch_std-accept-hdr into develop

Use standard-compliant Accept header when fetching #740