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

spec does not say that invalid m.relates_to properties are rejected #504

Open
richvdh opened this issue Jul 15, 2019 · 7 comments
Open

spec does not say that invalid m.relates_to properties are rejected #504

richvdh opened this issue Jul 15, 2019 · 7 comments
Labels
A-Client-Server Issues affecting the CS API help wanted Interested in contributing to the spec? These would be great additions! spec-bug Something which is in the spec, but is wrong

Comments

@richvdh
Copy link
Member

richvdh commented Jul 15, 2019

If someone tries to send an event with an m.relates_to property which is not an object (for example, m.relates_to: null, per matrix-org/synapse#5404), should the server reject the attempt with a 400 error, or should the m.relates_to be ignored?

@turt2live
Copy link
Member

In practice it should be whatever the server would do for the wrong kind of body.

@turt2live turt2live added clarification An area where the expected behaviour is understood, but the spec could do with being more explicit A-Client-Server Issues affecting the CS API labels Jul 16, 2019
@richvdh
Copy link
Member Author

richvdh commented Jun 1, 2021

In practice it should be whatever the server would do for the wrong kind of body.

Synapse currently returns a 400 with: {"errcode":"M_UNKNOWN","error":"'body' not a string type"}. That's probably a reasonable precedent to follow here.

@richvdh
Copy link
Member Author

richvdh commented Nov 19, 2021

this is touched on in MSC2674, but still not really answered; see matrix-org/matrix-spec-proposals#2674 (comment)

@clokep
Copy link
Member

clokep commented Nov 19, 2021

I think I implicitly changed the behavior for this in matrix-org/synapse#11161 where it will just ignore the relation instead of error. 😢

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
@richvdh
Copy link
Member Author

richvdh commented Oct 13, 2022

this is touched on in MSC2674, but still not really answered; see matrix-org/matrix-spec-proposals#2674 (comment)

This got resolved during FCP on MSC2674; it says:

Servers should reject events not meeting these conditions with an HTTP 400 error when they are received via the client-server API.

However, the specced text seems to say something different:

Relationships which don't match the schema, or which break the rules of a relationship, are simply ignored.

So I think this is now a spec-bug.

@richvdh richvdh added spec-bug Something which is in the spec, but is wrong and removed clarification An area where the expected behaviour is understood, but the spec could do with being more explicit labels Oct 13, 2022
@turt2live
Copy link
Member

fwiw, the "ignore the thing" bit is still accurate to the MSC (it says that clients which receive an invalid relationship are to treat it as though there is no relationship). It's just the 400 error which didn't make it.

@turt2live turt2live added spec-problem and removed spec-bug Something which is in the spec, but is wrong labels Oct 13, 2022
@richvdh
Copy link
Member Author

richvdh commented Oct 13, 2022

sorry, yes.

@richvdh richvdh added the spec-bug Something which is in the spec, but is wrong label May 16, 2023
@richvdh richvdh changed the title how should an invalid m.relates_to be handled spec incorrectly says that invalid m.relates_to properties are ignored May 16, 2023
@richvdh richvdh added the help wanted Interested in contributing to the spec? These would be great additions! label May 16, 2023
@richvdh richvdh changed the title spec incorrectly says that invalid m.relates_to properties are ignored spec does not say that invalid m.relates_to properties are rejected May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client-Server Issues affecting the CS API help wanted Interested in contributing to the spec? These would be great additions! spec-bug Something which is in the spec, but is wrong
Projects
None yet
Development

No branches or pull requests

3 participants