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

MSC4213: Remove server_name parameter from join and knock endpoints #4213

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions proposals/4213-remove-server-name.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation requirements:

I'm tempted to mirror #4127 (comment) and say "none", but am second-guessing that MSC's requirements too. It might be beneficial to have evidence that developers are picking up the newer approaches before suggesting removal from the spec.

Thoughts very welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not count because they were all created by me but there are five implementations linked in the proposal's body, four of which have already landed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# MSC4213: Remove `server_name` parameter

[MSC4156] deprecated the `server_name` parameter on [`/_matrix/client/v3/join/{roomIdOrAlias}`]
and [`/_matrix/client/v3/knock/{roomIdOrAlias}`] in favor of a new parameter `via`. This change
shipped in [Matrix v1.12]. In line with the [deprecation policy], the `server_name` parameter
is now eligible for removal from the spec.


## Proposal

The deprecated `server_name` parameter is removed from [`/_matrix/client/v3/join/{roomIdOrAlias}`]
and [`/_matrix/client/v3/knock/{roomIdOrAlias}`].
clokep marked this conversation as resolved.
Show resolved Hide resolved


## Potential issues

None. Servers can continue advertising support for earlier versions of the spec that included
`server_name` via [`/_matrix/client/versions`].

As of writing, the following stable implementations of [MSC4156] are known to the author:

- synapse: https://github.com/element-hq/synapse/pull/17650
- dendrite: https://github.com/matrix-org/dendrite/pull/3438
- matrix-js-sdk: https://github.com/matrix-org/matrix-js-sdk/pull/4381
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible I'm confused but matrix-org/matrix-js-sdk#4381 removes the unstable prefix from the 'via' param. This MSC is about removing the old server_name param, which is a different thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked implementations are for MSC4156 which added the via param. They're listed to show that the old server_name parameter is safe to remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. I think the server_name parameter will probably stay in implementations for quite a while unless they consciously choose to only support Matrix >= 1.12. But we don't need it in the spec anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I see - I guess I was confused by your comment. In that case I guess I can vouch for the js-sdk impl, in that it now sends the via param.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server impl looks plausible too. I think we could remove the implementation-needs-checking label.?

- ruma: https://github.com/ruma/ruma/pull/1891
- trixnity: https://gitlab.com/trixnity/trixnity/-/merge_requests/478


## Alternatives

None.


## Security considerations

None.


## Unstable prefix

None.


## Dependencies

None.


[`/_matrix/client/v3/join/{roomIdOrAlias}`]: https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3joinroomidoralias
[`/_matrix/client/v3/knock/{roomIdOrAlias}`]: https://spec.matrix.org/v1.12/client-server-api/#post_matrixclientv3knockroomidoralias
[`/_matrix/client/versions`]: https://spec.matrix.org/v1.10/client-server-api/#get_matrixclientversions
[Matrix v1.12]: https://spec.matrix.org/v1.12/changelog/v1.12/
[MSC4156]: https://github.com/matrix-org/matrix-spec-proposals/pull/4156
[deprecation policy]: https://spec.matrix.org/v1.12/#deprecation-policy