Authenticated media and animated thumbnails support #776
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Connection::loadCapabilities() is a better name for reloadCapabilities(); it also returns a JobHandle so that callers could act upon the job completion, successful or not - Connection::loadVersions() spawns GetVersionsJob to obtain supported spec versions - ...which are then exposed by Connection::supportedMatrixSpecVersions() - Connection::loadingCapabilities() is deprecated in favour of Connection::capabilitiesReady(): it was never meant to check whether capabilities are being loaded, rather that they _have been_ loaded and are available locally; the new name and semantics reflect that - HomeserverData encapsulates minimal information about the homeserver that is necessary to form network requests to it, encapsulating baseUrl but also including spec versions supported by the homeserver so that the right endpoint could be picked depending on those. Normally, clients should not use this structure directly; actually, it may end up blending into ConnectionData if/when it becomes thread-safe and therefore usable inside NetworkAccessManager. - NetworkAccessManager, respectively, now stores account's HomeserverData in its thread-safe storage, so that it could be used to convert mxc requests into DownloadFileJob requests in a following commit.
DownloadFileJob and MediaThumbnailJob now act as proxies to either GetContentJob/GetThumbnailJob or GetContentAuthedJob/GetThumbnailAuthedJob, respectively - selecting the right endpoint based on spec versions supported by the homeserver. To support that, BaseJob::doPrepare() virtual function now accepts ConnectionData so that derived jobs could check homeserverData() in it, and BaseJob::makeRequestUrl() gets HomeserverData instead of QUrl for the first parameter (effectively imposing the same requirement to makeRequestUrl() signatures in all other jobs). FTBFS; next commits for generated job classes will fix the build.
It doesn't seem to be in use anyway (downloadFile() is instead).
Looks like std::vector::emplace_back() from libc++ is/was seriously broken?
KitsuneRal
changed the title
|
KitsuneRal
added a commit
that referenced
this pull request
Jul 18, 2024
This was introduced as a workaround for cases when homeservers require authentication on media endpoints, deviating from the spec text of that time in order to protect themselves. This workaround is not necessary since #776; DownloadFileJob and MediaThumbnailJob now always supply the access token even when the homeserver doesn't require it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit implements a proxy layer (almost entirely in terms of existing facilities -
Connection::downloadFile()/DownloadFileJobandConnection::getThumbnail()/MediaThumbnailJobrespectively) to spare clients from having to figure out which API, old or new, they should use to retrieve media on any given homeserver. In all honesty, the code for the new API hasn't been tested yet but it's almost entirely based on the generated API classes, so nothing should go wrong because our CS API descriptions are perfect, right? :-DImplements MSC3916 and, while at it, [MSC2705](https://github.com/matrix-org/matrix-spec-proposals/pull/2705] (animated thumbnails).