Downloading a folder of files results in `downloader::recursive Timeout` error

[JoshuaCrewe]

Description of the bug

I am using an instance with a slow(ish) upload (2.5mb), I have a folder of video files, each file about 300mb each and about 6 files in the folder. When downloading that folder, I get a zip which has one of those files and an error.log which reports downloader::recursive Timeout

Step by step instructions to reproduce the bug

I was able to reproduce it in particular circumstances which might not be helpful but are pretty real world

1. Upload a folder of large files
2. Select the folder in filestash UI (ctrl+click)
3. Download the folder
4. after some slow progress the download stops at about the size of one of those files

Can you replicate that error from the demo?

I can't replicate it on the demo. This leads me to look at the network speed (given the timeout error) or the file sizes.

Observed behavior

Downloading a folder of largish files on a slow network results in a failed download zip

Expected behavior

I would expect it to match the behaviour of the demo site where a zip of all files is downloaded without an error file.

[JoshuaCrewe]

I have discovered features.protection.zip_timeout which appears to make a difference. If I bump that up to 240 then I can get two of the files downloaded before it errors out. I would expect that the timeout needs to be higher than the amount of time it takes to download the files. It doesn't take more than 60 seconds before it fails though so perhaps this is just a coincidence.

I would have expected the network to handle a faster download to be honest as the ISP allows up to 20mb/s so there is that aspect I can investigate. Is the solution to set a really long timeout?

Edit : Setting a high timeout still fails after 2 of 6 items downloaded.

[mickael-kerjean]

You can put the timeout to multiple days if you want, the idea there is to protect the server against people doing crazy things like trying to download a folder that contains recursive link for which it's impossible to create such zip or trying to zip something that's way too big. On the second scenario, I wouldn't want somebody trying to zip a folder that could potentially contains PB of data to use the server resources forever without any actual value

[c0xc]

I hit this bug when I was trying to see if the upload bug (issue #650) is fixed. As a user, I don't get an error message but I end up with a zip file that contains an error.log ("downloader::recursive Timeout") and which, after extracting, turns out to be incomplete. Such a silent bug is the worst kind of bug because during everyday usage, nobody would notice that a bunch of files are missing! If someone notices, it could be weeks later, when it's impossible to find out where those files were lost. At that point, they might be long gone from filestash, in other words this bug can cause users to lose files.

As for the last comment, I respectfully disagree. Trying to protect against recursive symlinks is relevant but randomly stopping file collection after x seconds and returning an incomplete zip file is not the way to do it.

[mickael-kerjean]

If you have a better idea, feel free to make a PR, if something better comes in than the current approach and doesn't introduce any footgun or drawback it will definitely get merged.