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

Fixed large upload sizes #280

Closed
wants to merge 2 commits into from
Closed

Conversation

illuzn
Copy link

@illuzn illuzn commented Dec 19, 2023

Proposed Changes

Using the environment variable, FILE_UPLOAD_SIZE_LIMIT, the file upload size may be set to an arbitrary value.

This is supported in /addon-bookstack/bookstack/rootfs/etc/nginx/nginx.conf by client_max_body_size 4G.

However, this does not correctly override the nginx server limits and php limits of 64M respectively.

This changes the nginx server_params.conf and the php81 www.conf to 4G which mirrors the above.

Consider whether it is appropriate to set limits of 0 (i.e. unlimited). I didn't do this since it may cause other issues.

These changes confirmed working on my production instance by manually changing those files and restarting the nginx and php81 services.

Related Issues

N/A

@sinclairpaul
Copy link
Member

I think this is slightly excessive, and would need adjustments to PHP memory usage.

If it needs to be adjusted higher, why wouldn't we just set it to whatever the variable is set to?

@illuzn
Copy link
Author

illuzn commented Dec 20, 2023

I think this is slightly excessive, and would need adjustments to PHP memory usage.

If it needs to be adjusted higher, why wouldn't we just set it to whatever the variable is set to?

Or to be honest we could make all of the limits something reasonable but not out of the realm of possibility say 300M. I ran into this problem when trying to upload an instruction manual to my home wiki which was 150mb and configuring the maximum file size via envvars didn't fix it.

Edit: I see what you mean now. #238 is tangentially related as is commit 926c223.

I note the default permitted upload size is 50mb. On my config, a 50mb jpeg on the lowest compression unpacks to around 200mb of memory (almost hitting the php memory limit) - obviously with higher compression this would exceed the memory limit. Perhaps this is best addressed by a note in the documentation about memory usage for large image files. Actually, I'm very surprised that #238 hit the memory limit (must be jpg with very high compression) because I cannot reproduce the issue locally (but can with a 50mb jpeg).

@sinclairpaul
Copy link
Member

I think the default is 128, I didn't implement the change yet.

Ideally what I think I would like to do is enumerate the FILE_UPLOAD_SIZE_LIMIT variable and set the relevant settings based on that.

@illuzn
Copy link
Author

illuzn commented Dec 20, 2023

I'd give it a crack but hassio addons are above my paygrade I'm afraid.

@frenck
Copy link
Member

frenck commented Mar 22, 2024

Because there hasn't been any activity on this PR for quite some time now, I've decided to close it for being stale.

Feel free to re-open this PR when you are ready to pick up work on it again 👍

@frenck frenck closed this Mar 22, 2024
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

Successfully merging this pull request may close these issues.

3 participants