matrix-org / matrix-org/matrix-rust-sdk

`Client.server()` returns malformed server name

Open
#6,598 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2.3k
Forks
500
Avg merge
1d 16h
Merged PRs (30d)
106

Description

Based on the original [PR](https://github.com/matrix-org/matrix-rust-sdk/pull/3908) 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

Contributor guide

Open the contributing guide

Research direction

Start with Client.server(), homeserver(), and ClientInner, then inspect UserId::server_name() and the authentication/whoami flow. Review the inline documentation and existing client tests, noting that the issue reports no current coverage. Done means the API consistently distinguishes server names from homeserver URLs, authenticated behavior follows the issue's expectations, and the examples and tests are updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.