Apply a timeout to reading the body when fetching a file.

[clokep]

Fixes #8302.

Applies a timeout to reading the body for SimpleHttpClient.get_file(). This brings the code more inline with MatrixFederationHttpClient.get_file, although that uses addTimeout instead of timeout_deferred.

I tested this by attempting to preview a stream (I used a local radio station), waiting several minutes and noticing the file was still growing. With this patch the download stops after 60 seconds and the client receives an error message:

{"errcode":"M_TOO_LARGE","error":"Requested file took too long to download"}

We are rather inconsistent with using timeouts when reading bodies, we seem to do it in some places, but not many other places. It is unclear if this is on purpose or not though. This particular method is only used by URL previews.

[anarcat]

60 seconds is still a long time. shouldn't there be a byte size limit instead? or better: both?

[clokep]

shouldn't there be a byte size limit instead? or better: both?

There is also a byte-size limit, it is implemented inside of the read_body_with_max_size, the actual limit is configurable, but defaults to 10 megabytes, I believe.

[reivilibre]

Sounds sensible

[dkasak]

FWIW, my instinct is also to make this shorter, probably closer to 10 seconds. HTTP requests usually don't take this long to complete and failure is not catastrophic anyway given that this is just for URL previews.

[clokep]

FWIW, my instinct is also to make this shorter, probably closer to 10 seconds. HTTP requests usually don't take this long to complete and failure is not catastrophic anyway given that this is just for URL previews.

I chose 60 seconds fairly arbitrarily (and it matched some other timeouts in our code). 10 seconds seems rather short if a large image is being previewed, but maybe not (for reference, what bandwidth would it take to hit the 10M limit at a certain time -- 10s: 1000 kbps vs. 60s: 167 kbps).

I'm also a bit worried that some people might have cranked the max preview size up even more than the 10M limit. Would lowering it to 30s sound reasonable?

[dkasak]

There's probably a relationship here between the max file size preview and the max timeout, since you probably don't want to wait a very long time for previews to show up since they're much less useful if they're significantly late.

30s sounds reasonable and an improvement over 60s, so I think that's a good compromise, let's go with that?