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

Strip trailing / on HS & IS URLs #995

Closed
ara4n opened this issue Feb 1, 2017 · 2 comments · Fixed by #6318
Closed

Strip trailing / on HS & IS URLs #995

ara4n opened this issue Feb 1, 2017 · 2 comments · Fixed by #6318
Labels
A-Media O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems Z-Papercuts Visible. Impactful. Predictable to action.

Comments

@ara4n
Copy link
Member

ara4n commented Feb 1, 2017

Much confusion arises if you specify a custom HS URL with a trailing /, as Riot/iOS then starts forming URLs like https://matrix.wherever.com//_matrix... which may well 404 due to the double slash. We must canonicalise the URLs.

See https://twitter.com/MacLemon/status/826606192323497984

@MacLemon
Copy link

MacLemon commented Feb 2, 2017

To not only tweet, but give a proper bug report:

Summary:
Custom Home Server URLs ending in / leads to pictures not being shown in chats.

Steps to Reproduce:
Run a Synapse Server at matrix.example.com where synapse is behind an nginx reverseproxy for port 443 for clients.
In Riot/iOS 0.3.8 configure a custom Home Server with the URL https://matrix.example.com/ (trailing slash).
Both, nginx and Apache do merge double slashes to a single slash. The documentation of synapse for nginx says to use a location { } context for /_matrix to proxy_pass to synapse.
In this case nginx will not merge slashes as expected, hence it cannot deliver picture assets requested by Riot.app starting with a path of //_matrix/client…

Interestingly Riot.app only does this for pictures, but not for all the other API calls.

Expected Results:
Riot.app should sanitize the home server URL for all requests and be resilient to users adding a trailing slash to the server URL.

Actual Results:
When using a home server URL with a trailing slash, everything seems to work, but pictures aren't displayed in chats. This is the case for pictures you uploaded yourself, as well as pictures uploaded by your chat contacts.
Riot clients configured without a trailing slash show all pictures, those with a trailing slash show none.

Regression:
n/a

Notes:
Loggin out Riot.app and reconnecting without a trailing slash fixes the issue but has the inconvenience of loosing a lot of history in encrypted chats, including your own messages.

@TheEagleByte
Copy link

This is still an issue. Can we get this prioritized?!!

@pixlwave pixlwave added A-Media O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems Z-Papercuts Visible. Impactful. Predictable to action. labels Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Media O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems Z-Papercuts Visible. Impactful. Predictable to action.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants