Nheko-Reborn / mtxclient

Client API library for Matrix, built on top of libcurl
MIT License
40 stars 25 forks source link

mtxclient does not handle redirects for image downloads #109

Open nishanthkarthik opened 1 month ago

nishanthkarthik commented 1 month ago

` QML QQuickImage: Failed to download image: (qrc:/resources/qml/delegates/ImageMessage.qml:112, )

`

https://github.com/Nheko-Reborn/mtxclient/blob/a0b203980491ddf2e2fe4f1cd6af8c2562b3ee35/lib/http/client.cpp#L840

Sometimes the matrix image urls are redirects to S3 buckets elsewhere so this callback above needs to follow redirects.

When I explicitly handle http code 307 by making another get request to the location header, it works as expected.

if (err.has_value() && err->status_code == 307) {
                             _this->p->client.get(fields->at("location"), [cb = std::move(cb)](const coeurl::Request &r) {
                                 cb(std::string(r.response()), r.response_headers(), {});
                             }, {}, 10);
                         }

I'm certain there's a better way of doing this but simply setting num_redirects to 10 in the top level get did not work. I did not look into why.

Thanks

nishanthkarthik commented 1 month ago

I did discover this in nheko originally. The root cause appears to be how mtxclient handles downloads

nishanthkarthik commented 1 month ago

Possibly related to a70753812e3fb832df12b24ee0ee6c70d68060d5?

deepbluev7 commented 1 month ago

I think you may have added the redirect support to the wrong endpoint? Is your server using the new or the old style urls? I.e. /media/v3 or /client/v1/media?

nishanthkarthik commented 1 month ago

It's /media/v3 used by beeper's fork of synapse https://github.com/beeper/synapse. This redirects to an S3 bucket.

Let me know how I can help! I can get more debugging info if you're interested (full urls, debug logs from nheko)