Closed khimaros closed 2 years ago
It's not specified to include or not include the trailing slash. The reason for riot-web (and probably the other riots) not exploding is https://github.com/matrix-org/matrix-js-sdk/blob/35f1cdf89c26a0a6567e8983a03a178d0b198b49/src/autodiscovery.js#L462-L464
I'd personally recommend that Pattle do the same, as Synapse is not the only producer of well-known files.
@wilkomanger who is the author of Pattle.
Making the clients more robust seems reasonable to me.
Also, removing the trailing slash from the returned JSON in synapse
still seems like a good move. It's a bit odd that the trailing slash is added to m.homeserver
but not m.identityserver
.
It is also the case that the canonical Matrix server https://matrix.org/.well-known/matrix/client returns a result without trailing slash for both keys.
Thanks for notifying me, this should be handled correctly in Pattle anyway.
Is this still a concern to anyone?
I'm minded to think that synapse should be stripping trailing slashes rather than adding them, and ideally this should be mentioned in the spec.
Maybe it makes sense to just normalize multiple slashes instead? At least in Linux land it's common practice to handle multiple slashes as single slash, exactly to prevent issues when two parts of a path both specify a trailing and a preceding slash
For example https://example.com/ +/_matrix/client/ + /r0/login is https://example.com//_matrix/client//r0/login and could be normalized to https://example.com/_matrix/client/r0/login before handling the request
This is also an issue for sailtrix, a Sailfish OS Matrix client.
The ticket for this issue on the sailtrix side.
I think a better way to do this would be using urllib.parse.urlparse
to ensure canonicalization.
I think the spec should be explicit about expected behaviour here: https://github.com/matrix-org/matrix-doc/issues/3465
Per discussion on the Matrix Spec https://github.com/matrix-org/matrix-doc/issues/3465#issuecomment-980449599 we've ultimately decided that mandating either that trailing slashes are stripped or that they're added is fraught with peril, and that the only practically viable path forward is to explicitly instruct clients to handle both cases. This has now become part of the Matrix spec, rendering this issue moot.
Description
Matrix clients such as Pattle honor the output of
/.well-known/matrix/client
literally.A block in
synapse/config/server
modifies the value of public_baseurl by appending a/
if one is not already present. The value ofpublic_baseurl
is returned unmodified bysynapse/rest/well_known
which causes the errant trailing/
to reach the user. By way of example:Steps to reproduce
public_baseurl
tohttps://myhome.server
or equiv.Pattle simply appends
/_matrix
to the URL returned inm.homeserver
which results in requests to the Matrix server of the form:synapse refuses to handle these requests due to the double
//
Removing all three lines in this block appears to have no ill effect on Riot Android or Riot Web and fixes the discovery issue for Pattle.
Version information
All versions, including HEAD.