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

ZIP downloads should not be possible to disable #4345

Closed
jancborchardt opened this issue Aug 7, 2013 · 14 comments
Closed

ZIP downloads should not be possible to disable #4345

jancborchardt opened this issue Aug 7, 2013 · 14 comments

Comments

@jancborchardt
Copy link
Member

Why is it possible for admins to disable zip downloads, if the system can handle it? If for security it should be fixed differently. If it’s not supported by the server then it’s just not possible and the setting shouldn’t show.

Now apparently the reason for this setting is that the server can be overloaded through this feature. Now how exactly can that happen? And why is it ok to let people set an option then which potentially will kill their server. If it’s too many files, too many subsequent downloads or something like that it should be blocked in the code instead of having a zip-killswitch.

@LukasReschke @DeepDiver1975 what do you think?
(Issue originally from my comments on #1257 )

@karlitschek
Copy link
Contributor

I think the original idea was that an ownCloud server "explodes" if you try to create a huge zip file which than could kill the server for others too. Because of that we introduced it last year as far as I know. I think @blizzz worked on that. The perfect solution would be of course if ownCloud would handle this somehow internally without the config option. But might be tricky.

@blizzz
Copy link
Contributor

blizzz commented Aug 8, 2013

Exactly. Zip generation will eat all your CPU and if you try to download a huuuge folder, ownCloud (and other services running there) will be inaccessible.

@blizzz
Copy link
Contributor

blizzz commented Aug 8, 2013

P.S.: You can also only allow Zip generation if a certain input size is exceeded. If you dislike it being in the Settings UI, we may move it to config.php.

@DeepDiver1975
Copy link
Member

We have still an open pull request which tries to handle zip creation at a low memory profile - a solution to this?
#3439

@jancborchardt
Copy link
Member Author

New pull request seems to be at #6893

@PVince81
Copy link
Contributor

PVince81 commented May 2, 2014

Now that the zip streaming works, should we go ahead and remove that option ?

@PVince81
Copy link
Contributor

PVince81 commented May 2, 2014

@ghiamar @5meter I saw in separate bug reports that you guys had the option "allow zip download disabled". Can you confirm that you did this only to save some server resources and remove the need to have a temporary folder with available space ?

This is planned for removal in OC 7 because the zip file is now built on the fly and streamed directly back to the client (no more temporary file).

Thanks.

@mgscreativa
Copy link

Well, I really love to have that option disabled, because I dont want to give anyone the opsiility to download a complete shared folder generating a huge zip file...

@PVince81
Copy link
Contributor

PVince81 commented May 2, 2014

Even if that huge zip file isn't stored anywhere on the server and streamed directly to the client ?

@mgscreativa
Copy link

Well, if it doesn't kill my server ram, its ok I guess...

@mgscreativa
Copy link

@PVince81 Forgot to mention, thatnks for asking!

@PVince81
Copy link
Contributor

PVince81 commented May 5, 2014

Just re-read the comments above, would be good to double check if the current mechanism still "eats the CPU" when making a zip file.

@jancborchardt
Copy link
Member Author

Fixed by removal of the setting. Thanks @MorrisJobke! :)

@jancborchardt
Copy link
Member Author

(Also, by adding zipstreamer – thank you @McNetic! :)

@lock lock bot locked as resolved and limited conversation to collaborators Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants