Skip to content

Fix FetchAtomService content type handling#9132

Merged
Gargron merged 3 commits intomastodon:masterfrom
valerauko:fix-atom-accept
Oct 30, 2018
Merged

Fix FetchAtomService content type handling#9132
Gargron merged 3 commits intomastodon:masterfrom
valerauko:fix-atom-accept

Conversation

@valerauko
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

Sorry about the trouble caused.

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