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

According to the ActivityStreams vocabulary / JSON-LD specification inReplyTo is not limited to a single IRI #25588

Open
mariusor opened this issue Jun 26, 2023 · 3 comments
Labels
activitypub Protocol-related changes, federation bug Something isn't working

Comments

@mariusor
Copy link

mariusor commented Jun 26, 2023

Steps to reproduce the problem

  1. On main Mastodon page search for https://federated.id/objects/4cf520f4-7aec-47e3-a245-24a9a31f3360 (initially it was https://federated.id/objects/b21f0da2-1cfe-4a97-8fb7-af3233deaf7d, but I fixed the compaction issue mentioned below)
  2. The search API returns error 500.

Expected behaviour

Mastodon returns the Note object as a search result.

Actual behaviour

Reported "500 Internal Server Error" and no result.

Detailed description

The reason for the crash seems to be that the inReplyTo property of the object is an array instead of a single IRI and Mastodon expects it to either be null or a string.

The search page URL at the time of the bug is:
/api/v2/search?q=https:%2F%2Ffederated.id%2Fobjects%2F4cf520f4-7aec-47e3-a245-24a9a31f3360&resolve=true&limit=5

The Stack trace from the logs:

[cee0...7a45] method=GET path=/api/v2/search format=html controller=Api::V2::SearchController action=index status=500 error='TypeError: no implicit conversion of String into Integer' duration=690.37 view=0.00 db=5.94
[cee0...7a45]
[cee0...7a45] TypeError (no implicit conversion of String into Integer):
[cee0...7a45]
[cee0...7a45] app/helpers/jsonld_helper.rb:57:in `value_or_id'
[cee0...7a45] app/lib/activitypub/activity/create.rb:368:in `in_reply_to_uri'
[cee0...7a45] app/lib/activitypub/activity/create.rb:358:in `replied_to_status'
[cee0...7a45] app/lib/activitypub/activity/create.rb:307:in `poll_vote?'
[cee0...7a45] app/lib/activitypub/activity/create.rb:51:in `block in create_status'
[cee0...7a45] app/models/concerns/lockable.rb:12:in `block (2 levels) in with_lock'
[cee0...7a45] app/models/concerns/lockable.rb:10:in `block in with_lock'
[cee0...7a45] app/lib/redis_configuration.rb:10:in `with'
[cee0...7a45] app/models/concerns/redisable.rb:9:in `with_redis'
[cee0...7a45] app/models/concerns/lockable.rb:9:in `with_lock'
[cee0...7a45] app/lib/activitypub/activity/create.rb:50:in `create_status'
[cee0...7a45] app/lib/activitypub/activity/create.rb:13:in `perform'
[cee0...7a45] app/services/activitypub/fetch_remote_status_service.rb:54:in `call'
[cee0...7a45] app/services/fetch_remote_status_service.rb:12:in `call'
[cee0...7a45] app/services/resolve_url_service.rb:28:in `process_url'
[cee0...7a45] app/services/resolve_url_service.rb:16:in `call'
[cee0...7a45] app/services/search_service.rb:83:in `url_resource'
[cee0...7a45] app/services/search_service.rb:16:in `block in call'
[cee0...7a45] app/services/search_service.rb:12:in `call'
[cee0...7a45] app/controllers/api/v2/search_controller.rb:33:in `search_results'
[cee0...7a45] app/controllers/api/v2/search_controller.rb:12:in `index'
[cee0...7a45] app/controllers/concerns/localized.rb:11:in `set_locale'
[cee0...7a45] lib/mastodon/rack_middleware.rb:9:in `call'

Specifications

Mastodon v4.1.2
Ruby is 3.0.6p216
NodeJS is 14.21.3

@mariusor mariusor added the bug Something isn't working label Jun 26, 2023
@trwnh
Copy link
Member

trwnh commented Jun 26, 2023

it may be indeed the case that mastodon does not support inReplyTo having multiple values, but the linked AS2 document is invalid because it is not consistent with the compacted form. compacting a set that isn't explicitly declared to be a @set should reduce it to a direct IRI reference if the set only contains that one IRI node.

@mariusor
Copy link
Author

mariusor commented Jun 26, 2023

I'll give you that @trwnh, I'll add it (:partying_face: fixed!) to my own list of bugs, thank you. :)


later edit
Looking closer at objects in mastodon outboxes, I would say that your comment is a bit too nitpicky. Mastodon fails at the same compaction test for at least three properties:

  • to comes always as an array (I imagine the others recipients related properties also)
  • attachment is an array (even empty)
  • tag is always an array

Looking even deeper, it looks like the collections and paged collections properties items and orderdItems should also compact to a single node when applicable, which seems completely unwholesome.

@renchap renchap added the activitypub Protocol-related changes, federation label Jun 26, 2023
@trwnh
Copy link
Member

trwnh commented Jul 1, 2023

@mariusor i do dislike the "collapse the array when there is a single value" behavior of JSON-LD, yes -- see w3c/activitystreams#535 for more information about this regarding the as2 context.

regarding items and orderedItems collapsing... this is currently true for items, but NOT for orderedItems. this is because orderedItems is explicitly defined as @container: @list, which is an ordered array. similarly, as stated in w3c/activitystreams#535 we can have the same "force it to be an array" behavior by definining items as @container: @set, which is the same as the default value but without the collapsing functionality.

regarding mastodon: i'm not being nitpicky, it's just how the spec is defined right now -- consistency with compacted form against the normative context means that in theory this is a MUST clause being violated. spec compliance is obviously something that is fast and loose in the fediverse, so there are bound to be oversights in multiple projects. following JSON-LD and AS2, any term can be either a single value/node or an unordered array, unless it is marked "functional" in AS2-Vocab, in which case it must be a single value/node. the cases you describe in mastodon are likewise technically incompliant. with that said: this is all tangential to the issue, being that mastodon has incorrect expectations regarding which properties can be arrays and which ones can't be. the most correct thing to do would be to follow postel's law and author correctly-collapsed values while accepting more leniently incorrect serializations by other implementations. this should be applied to pretty much every property (again, anything not marked functional in as2-vocab).

also tangentially: if everyone is already doing the "wrong" thing, then it might be more prudent to upstream this and change the normative context as described in w3c/activitystreams#535 at least in part. you mention attachment and tag in particular, which is another issue: w3c/activitystreams#537

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

No branches or pull requests

3 participants