Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Accept headers when fetching AP objects to match spec #30354

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheOneric
Copy link

ActivityPub spec section 3.2 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.

Currently Mastodon omits the profile in its dereferences (but not the fetch service) and only lists application/ld+json as one of several possible types. This breaks spec and allows spec-compliant implementations to refuse any such fetch requests.

Resolve this by adding the required profile and while at it, make the only spec-compliant type the first listed choice in all relevant places.
While unlikely to be a problem due to other parts already including a profile, also keep a profile-less JSON-LD type where it existed before to ensure this doesn't break federation with a hypothetical buggy implemenetation relying on this current Mastodon quirk.

Section 7 also specifies the same media type MUST be used in the Content-Type header of for POST requests, but here we can't specify alternatives, so for now keep the current type.

Fixes a part of #22720


This is a more conservative alternative to #22722.

  • no Accept types are removed (keeping them is imho a good idea anyway)
  • AP S2S POST requests are left with the non-compliant application/acitivity+json; i believe those should be changed to match spec too but afaik there’s no way to do this without fully removing the current type. If there’s any interest in doing so anyway, i’d be happy to add changes for them too

With the proper spec-compliant type now being the first choice on fetches at least, hopefully we’ll get less cases like #22722 (comment)

ActivityPub spec section 3.2 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.

Currently Mastodon omits the profile in its dereferences (but not the
fetch service) and only lists application/ld+json as one of several
possible types. This breaks spec and allows spec-compliant
implementations to refuse any such fetch requests.

Resolve this by adding the required profile and while at it,
make the only spec-compliant type the first listed choice in all
relevant places.
While unlikely to be a problem due to other parts already including a
profile, also keep a profile-less JSON-LD type where it existed before
to ensure this doesn't break federation with a hypothetical buggy
implemenetation relying on this current Mastodon quirk.

Section 7 also specifies the same media type MUST be used
in the Content-Type header of for POST requests, but here
we can't specify alternatives, so for now keep the current type.

Fixes a part of mastodon#22720
@renchap renchap added the activitypub Protocol-related changes, federation label May 17, 2024
@TheOneric
Copy link
Author

The CI failure for Ruby Testing / test(3.1) seems unrelated (API V2 Admin Accounts GET #index with limit param sets the correct pagination headers) and doesn't occur when running directly on the current commit: https://github.com/TheOneric/mastodon/actions/runs/9134282607/job/25133406209

Let me know if i missed and should fix something

@renchap
Copy link
Sponsor Member

renchap commented May 18, 2024

This looks like to be a flaky test. Maybe @mjankowski want to have a look?

@renchap renchap requested a review from a team May 20, 2024 10:39
@mjankowski
Copy link
Contributor

I don't see the failure at that link, but I think I found it here - https://github.com/mastodon/mastodon/actions/runs/9134408456/job/25122194677 - and yeah, might be a sort-order dependent intermittent failure ... will see if I can replicate locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
activitypub Protocol-related changes, federation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants