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 FetchAtomService content type handling #9132

Merged
merged 3 commits into from
Oct 30, 2018
Merged

Fix FetchAtomService content type handling #9132

merged 3 commits into from
Oct 30, 2018

Conversation

valerauko
Copy link
Contributor

@valerauko valerauko commented Oct 28, 2018

Issue

Fixes #8773

Changes

  • add profile to json+ld in the Accept header as required by the spec
  • use headers['Content-type'] instead of mime_type
    The latter strips out the profile part of the content type header, so compliant ActivityPub services don't get handled properly. headers['Content-type'] has the "raw" content type with the profile, so use that for checking.

It's required by the ActivityPub spec
mime_type strips the profile from the content type, but it's still available raw in the headers hash
@valerauko valerauko changed the title Fix FetchAtomService Accept header Fix FetchAtomService content type handling Oct 30, 2018
@Gargron Gargron merged commit c36a4a1 into mastodon:master Oct 30, 2018
@valerauko valerauko deleted the fix-atom-accept branch October 30, 2018 14:50
@Gargron
Copy link
Member

Gargron commented Oct 30, 2018

This broke remote resource resolving because the "raw" header contains stuff like ; charset=utf-8 so of course none of the comparisons match. Also, application/ld+json is a superset of the one with the profile, so I do not follow why it would matter. If I pass an Accept header without the profile part, the responding server is still free to include the profile part in the response, and I am free to ignore it. There aren't many purely ld+json things out there in the wild for that check to be important.

I missed this bug during review and I am very regretful about it. I need to revert this patch.

@valerauko
Copy link
Contributor Author

Sorry about the trouble caused.

rtucker pushed a commit to vulpineclub/mastodon that referenced this pull request Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants