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

mtxclient does not handle redirects for image downloads #109

Open
nishanthkarthik opened this issue Sep 11, 2024 · 4 comments
Open

mtxclient does not handle redirects for image downloads #109

nishanthkarthik opened this issue Sep 11, 2024 · 4 comments

Comments

@nishanthkarthik
Copy link

QML QQuickImage: Failed to download image: <url snip> (qrc:/resources/qml/delegates/ImageMessage.qml:112, ) [2024-09-11 14:53:06.869] [net] [error] Failed to download <url snip>: (connection: Operation was aborted by an application callback, http: 307, parser: [json.exception.parse_error.101] parse error at line 1, column 1: attempting to parse an empty input; check that your input string or stream contains the expected JSON [while parsing error]: )

get<std::string>(api_path,

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
Copy link
Author

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

@nishanthkarthik
Copy link
Author

Possibly related to a707538?

@deepbluev7
Copy link
Member

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
Copy link
Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants