Skip to content

Client.server() returns malformed server name #6598

Description

@hughns

Based on the original PR that added .server() I believe it is intended to return the "server name" as defined by https://spec.matrix.org/v1.18/appendices/#server-name

e.g. for the matrix.org homeserver the server name is matrix.org

The first thing that is confusing is that if you use the matrix-sdk crate directly then you find that the return type is Url not String. matrix.org isn't a URL, how could it be represented as a Url? 🤔

Then, if you actually build a client with server name matrix.org then calling .server() always returns https://matrix.org/ which is not a server name as per the spec.

Reading the inline comments is also confusing because it says:

    /// Not to be confused with the `Self::homeserver`. `server` is usually
    /// the server part in a user ID, e.g. with `@mnt_io:matrix.org`, here
    /// `matrix.org` is the server, whilst `matrix-client.matrix.org` is the
    /// homeserver (at the time of writing — 2024-08-28).

...which correctly says that matrix.org is the server [name], but then it says that matrix-client.matrix.org is the homeserver [base URL] which should actually be https://matrix-client.matrix.org (because matrix-client.matrix.org isn't a valid URL).

Thoughts/suggestions on how to resolve:

  • The behaviour that I would expect as a consumer of the SDK:
    • for an unauthenticated client
      • if the client was built by providing a server name then use it
      • otherwise, it is None (because the spec does not provide a way to convert a base URL into a server name)
    • for an authenticated client (i.e. that has an access token)
      • the server name should always be derived from the MX ID returned by whoami (i.e. UserId::server_name()
  • Use OwnedServerName internally (e.g. in ClientInner) instead of Url
  • Fix the incorrect examples in the inline docs
  • Clarify inline that homeserver() is the homeserver base URL from https://spec.matrix.org/v1.18/client-server-api/#getwell-knownmatrixclient_response-200_homeserver-information
  • Clarify inline that server() is the server name from https://spec.matrix.org/v1.18/appendices/#server-name
  • Maybe rename server() to server_name() which at least then matches the name of the thing in the spec
  • I didn't find any test coverage for this at the moment

CC @poljar

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions