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

Add LimitRequestBody 0 to .htaccess #37696

Closed
wants to merge 1 commit into from
Closed

Conversation

mgutt
Copy link

@mgutt mgutt commented Apr 12, 2023

"LimitRequestBody 0" allows uploading files bigger than 1024 MB, which seems to be the default apache webserver value. This is important as some browsers do not respect the chunking (for some of my Safari users this happens randomly) and the upload fails with the error "Expected filesize of 1207907122 bytes but read (from Nextcloud client) and wrote (to Nextcloud storage) 0 bytes.".

In addition it allows setting a chunking size bigger than 1024 MB.

"LimitRequestBody 0" allows uploading files bigger than 1024MB, which seems to be the default apache webserver value. This is important as some browsers do not respect the chunking (for some of my Safari users this happens randomly) and the upload fails with the error "Expected filesize of 1207907122 bytes but read (from Nextcloud client) and wrote (to Nextcloud storage) 0 bytes.".

Signed-off-by: mgutt <[email protected]>
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szaimen szaimen added the 3. to review Waiting for reviews label Apr 12, 2023
@szaimen szaimen requested a review from a team April 12, 2023 20:48
@mgutt
Copy link
Author

mgutt commented Apr 12, 2023

Hi, see https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/big_file_upload_configuration.html

I know that article, but finally it does not contain any explanation regarding the LimitRequestBody size, why it could be relevant and how to change it. To be exact: It is not possible to change it as every container upgrade overwrites the .htaccess file (that's why I use a cronjob to check and add the setting if its missing).

In addition the article describes solutions which shouldn't be needed at all as Nextcloud's chunking bypasses all those limits. With the default settings (10 MB chunking, 512MB PHP_UPLOAD_LIMIT/PHP_MEMORY_LIMIT) it was no problem to upload huge files to my servers for 99% of my users. Only a tiny amount of them had the problem that their browser randomly fail to chunk the files and by that those tiny amount hits the PHP, Apache and Proxy Server limits. So we try to create a workaround for random situations, which I think is ok as finally the user experience is the most important thing.

I think we have to solutions to solve this:
A) Add LimitRequestBody 0 as the new default
B) Use the container's PHP_UPLOAD_LIMIT value and set the LimitRequestBody accordingly (in bytes of course)

@szaimen
Copy link
Contributor

szaimen commented Apr 13, 2023

It is not possible to change it as every container upgrade overwrites the .htaccess file

See nextcloud/docker#1796

@mgutt
Copy link
Author

mgutt commented Apr 13, 2023

See nextcloud/docker#1796

This is not a solution, it is only a workaround because the user has to create the following dirs with the proper perms BEFORE installing the container (unRAID paths as an example):

/mnt/user/appdata/Nextcloud/etc
/mnt/user/appdata/Nextcloud/etc/apache2
/mnt/user/appdata/Nextcloud/etc/apache2/conf-enabled

And this file with the proper content:
/etc/apache2/conf-enabled/nextcloud-apache.conf

Even advanced users would now struggle as they can't know the perms and the needed content of the file before installing the container. So they will install the container first.

Then they would create the file with the needed configs and add a new mounted path and restart the container.

By that it is NOT possible to prepare the docker command (compose, docker run
..., or unraid templates) for non-advanced users as docker creates this empty dir if the file does not already exist:
/etc/apache2/conf-enabled/nextcloud-apache.conf/

In addition this solution blocks all changes through future apache upgrades.

And it does not make sense at all. I mean why does the container offer to change the PHP_UPLOAD_LIMIT if it fails with a higher value than 1024? This value must change LimitRequestBody, too, or it should be unlimited by default. I don't see an other solution for non-advanced users.

@szaimen
Copy link
Contributor

szaimen commented Apr 13, 2023

What I was pointing out that this is a bug in the docker image and needs to be fixed there 😉

It works fine e.g. with https://github.com/nextcloud/all-in-one

@J0WI
Copy link
Contributor

J0WI commented Aug 20, 2023

@szaimen why do you think this is an issue in the Docker image alone? This affects all installations running on Apache 2.4.53+. IMHO this should either be fixed in the .htaccess to ship a sane default for all or we all stick with the 1024 MB default and all users with other requirements have to customize their configurations as documented.

@kesselb
Copy link
Contributor

kesselb commented Aug 21, 2023

sane default

Do you have a value in mind?

If we add LimitRequestBody to our .htaccess file, you cannot override it anymore.
In my tests, the value in .htaccess wins over the web server configuration.

@mgutt
Copy link
Author

mgutt commented Sep 10, 2023

Do you have a value in mind?
0

I don't see a reason to have a limit at all.

If we add LimitRequestBody to our .htaccess file, you cannot override it anymore.
Could be added to httpd.conf as well. Why not.

@J0WI
Copy link
Contributor

J0WI commented Sep 14, 2023

No limit is always bad and Apache changed the default due a DoS vulnerability (thanks to @henryk for the pointer).

How are upload_max_filesize/post_max_size handled? Can't we use the same mechanics for LimitRequestBody?

@kesselb
Copy link
Contributor

kesselb commented Sep 15, 2023

How are upload_max_filesize/post_max_size handled?

.user.ini + ini_set

We don't increase the limits for upload_max_filesize/post_max_size in server anymore.
I confused it with set_time_limit/max_execution_time.

Can't we use the same mechanics for LimitRequestBody?

We try to increase the max_execution_time for PHP automatically via set_time_limit (in the past also upload_max_filesize/post_max_size via ini_set).

This mechanics is only available for some PHP configurations.
We cannot influence apache's configuration from PHP.

@mgutt
Copy link
Author

mgutt commented Sep 15, 2023

Not possible

Why shouldn't it be possible to set the value in httpd.conf by dockers entrypoint script?!

@kesselb
Copy link
Contributor

kesselb commented Sep 15, 2023

@mgutt reworked my previous comment.

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

Successfully merging this pull request may close these issues.

4 participants