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

getHttpUriForMxc no longer respects paths in baseUrl parameter #4658

Closed
begincalendar opened this issue Jan 27, 2025 · 5 comments
Closed

getHttpUriForMxc no longer respects paths in baseUrl parameter #4658

begincalendar opened this issue Jan 27, 2025 · 5 comments
Labels

Comments

@begincalendar
Copy link

Prior to 00aba74, if the baseUrl parameter had a path in it, the returned URL would contain that base URL path.

The new URL() call here doesn't respect the path in baseUrl. E.g.

const url = new URL('/_matrix/media/v3/download/myhomeserver.com/abc1234', 'https://myhomeserver.com/basepath');

...gives you:

https://myhomeserver.com/_matrix/media/v3/download/myhomeserver.com/abc1234

...discarding the /basepath. I.e. the result should be: https://myhomeserver.com/basepath/_matrix/media/v3/download/myhomeserver.com/abc1234

@dosubot dosubot bot added the T-Defect label Jan 27, 2025
@richvdh
Copy link
Member

richvdh commented Jan 27, 2025

It's not obvious to me that baseUrls that contain a path component are valid; in particular there is no way to get working federation for a matrix server that does not expose /_matrix/federation at the root.

If you need this behaviour you'll probably have to work at getting it specced.

@richvdh
Copy link
Member

richvdh commented Jan 27, 2025

See also: matrix-org/matrix-spec#693

@begincalendar
Copy link
Author

What about in scenarios where federation is unnecessary/undesirable? This is a scenario I’ve been operating in.

I don’t want to divert Element’s resources from more important work, but the non-federated use-case seems like it might have been missed in this case (and the linked issue).

@richvdh
Copy link
Member

richvdh commented Jan 28, 2025

In general, we're unlikely to expend effort fixing issues that only arise in exotic setups that don't support federation; and nor do I think we should complicate the code by doing so: it increases the risk of security issues. (Note that the fix here was introduced to fix a path traversal attack.)

@begincalendar
Copy link
Author

Understood.

I’ll leave it up to you to decide what to do with this issue.

@richvdh richvdh closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants