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

FetchAtomService does not properly support AS2 mime type #8773

Closed
1 of 2 tasks
puckipedia opened this issue Sep 25, 2018 · 11 comments
Closed
1 of 2 tasks

FetchAtomService does not properly support AS2 mime type #8773

puckipedia opened this issue Sep 25, 2018 · 11 comments
Labels
bug Something isn't working

Comments

@puckipedia
Copy link
Contributor

From quick researching turns out: the HTTP gem actually strips anything after the semicolon from the mime type. This causes things to break horribly on newer Kroeg, as it doesn't recognize "application/ld+json" as appropriate AS2 json, since it wants a profile.


  • I searched or browsed the repo’s other issues to ensure this is not a duplicate.
  • This bug happens on a tagged release and not on master (If you're a user, don't worry about this).
@Gargron Gargron added the bug Something isn't working label Oct 20, 2018
@valerauko
Copy link
Contributor

Shouldn't application/activity+json work too?

Then again currently the request doesn't seem to have the profile set in the first place...
https://github.com/tootsuite/mastodon/blob/26fe37c41460d6db3b5dd975a86c14fdd8dfadf3/app/services/fetch_atom_service.rb#L32

@puckipedia
Copy link
Contributor Author

it should, but the spec actually requires understanding application/ld+json; profile="https://www.w3.org/ns/activitystreams"

@valerauko
Copy link
Contributor

valerauko commented Oct 28, 2018

I thought the spec required producing that and suggested understanding both?

It should be fixed Mastodon side if it's not produced correctly, but aren't you being too strict on the receiving side?

Do I understand correctly that the issue is the call to FetchAtomService.new.call(kroeg_endpoint) fails on your side due to the lack of that profile?

@puckipedia
Copy link
Contributor Author

the PR you fixed is also true, but the spec also requires you *understand* it too, which is the real reason i opened the issue. If i reply with Content-Type: application/ld+json; profile="https://www.w3.org/ns/activitystreams" the HTTP gem strips out the profile, which means that this is not true:

https://github.com/tootsuite/mastodon/blob/e84da282f6331d463678a7e3ccd37e66c0dfcab3/app/services/fetch_atom_service.rb#L42

The easy fix would be just removing the profile from the ld+json mime type in the array, which should solve this issue, and would also work for other ld+json responses, which I would like to see personally for varying reasons..

@valerauko
Copy link
Contributor

Oh I see. I'll look into sorting that out too.

@valerauko
Copy link
Contributor

valerauko commented Oct 30, 2018

@puckipedia I pushed a fix and added a test case for it too

@valerauko
Copy link
Contributor

@Gargron then this should be reopened.

Considering what happened, the solution's probably @puckipedia's suggestion of just removing the profile from the comparison altogether. Not worth the trouble it'd take otherwise.

I'll push a fix out for that later today if suitable.

@nightpool
Copy link
Member

@valerauko we should make sure that the specs actually reflect real-world requests this time though

@nightpool nightpool reopened this Oct 31, 2018
@nightpool
Copy link
Member

nightpool commented Oct 31, 2018

(the problem was that the actual Content-Type served my mastodon is application/activity+json; charset=utf-8, but the HTTP gem correctly abstracts that away)

my preference is not to use the Content-Type header directly. Instead, we should patch the upstream HTTP library to provide access to the profile of the returned information where available, the same way it parses out encoding

In the short term, let's just do the thing puck suggested in the first case and ignore the profile for now

@valerauko
Copy link
Contributor

Yeah, using mime_type is probably the way. I'll check if it parses the profile into the struct, but even if it does, comparing against that would make the conditions too complicated for what it's worth.

Ignoring it probably has the best ROI

@valerauko
Copy link
Contributor

Put up a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants