dmstr / docker-php-yii2

:whale: :cd: Docker PHP image containing extensions and libraries for Yii 2.0 Framework
99 stars 33 forks source link

add client_max_body_size to http section also #21

Closed StalkAlex closed 7 years ago

StalkAlex commented 7 years ago

Without adding it doesn't work at all. Here's related problem https://stackoverflow.com/questions/2056124/nginx-client-max-body-size-has-no-effect

schmunk42 commented 7 years ago

We've check this with curl in the container and it should work as expected. Settings for nginx and php (upload_max_filesize, post_max_size) are all set to 512M.

The manual and the answers in Stackoverflow are mentioning the same behavior. Can you describe the action which you take and the errors reported by the server.

StalkAlex commented 7 years ago

@schmunk42 we're using this extension https://github.com/kartik-v/yii2-widget-fileinput, files uploading via ajax, it started showing uploading progress bar as usual, then it throws exception 413 Entity too large. But I see there's already client_max_body_size 512 in server configuration, it just doesn't respect it. This is problem is not only I faced with. Please pay attention to the second and the third answers on the stackoverflow link above. I thought it might be helpful for someone, if I added a little clarification to their suggestions. For starters, please be certain you have included your increased upload directive in ALL THREE separate definition blocks (server, location & http). Each should have a separate line entry. The result will like something like this (where the ... reflects other lines in the definition block)

As of March 2016, I ran into this issue trying to POST json over https (from python requests, not that it matters). The trick is to put "client_max_body_size 200M;" in at least two places http {} and server {}: I'm not sure if this is absolutely correct solution or not, but it definetely worked for us, error has gone after specifying it in http section. Php configured correctly and our site works through https if it matters.

schmunk42 commented 7 years ago

We'll double-check this, thanks for you report.

CC: @handcode

schmunk42 commented 7 years ago

I'm not sure if this is absolutely correct solution or not, but it definetely worked for us, error has gone after specifying it in http section. Php configured correctly and our site works through https if it matters.

@StalkAlex Do you connect to this php container via HTTPS (port: 443) or HTTP (port: 80)?

StalkAlex commented 7 years ago

@schmunk42 via https

schmunk42 commented 7 years ago

But you did additional modification to this image, since by default we're only listening on port 80?!

StalkAlex commented 7 years ago

Yep, that's right, I added default.conf to conf.d folder if it counts as modification.

schmunk42 commented 7 years ago

I assume that there might be an issue with https, but I wasn't able to test it out-of-the-box. At least this would make sense after all.

Can you confirm, that there's no problem with the upload process you mentioned, when connecting to the container via http?

handcode commented 7 years ago

@StalkAlex can you provide us your parsed nginx config (nginx -T from inside container) so we can review and test? thanks in advance

StalkAlex commented 7 years ago

@handcode here's config https://gist.github.com/StalkAlex/6d5ac60fa30b14a2c2a87f837bd0dcbb @schmunk42 now I can't confirm for http version as we don't use it.

handcode commented 7 years ago

@StalkAlex thanks, i've reviewed and testet with your config

As assumed, the "problem" is your conf.d/default. conf

let me explain:

If you add a client_max_body_size line to your server{} block, file uploads will work as expected.

# patch conf.d/default.conf
--------------------------------schnipp-----------------------------
--- image-files/default.conf~    2017-09-15 09:38:09.000000000 +0200
+++ image-files/default.conf    2017-09-15 10:09:53.000000000 +0200
@@ -8,6 +8,8 @@
     error_log       /var/log/nginx/error.log;
     index           /index.php;

+    client_max_body_size 512M;
+
     error_page   495  496  /400.html;

     ssl                  on;
--------------------------------schnapp-----------------------------

If your interested, here are the scratch docs from my tests https://gist.github.com/handcode/3e7e804ddaa37e4770b60ab2fa1b7b92

StalkAlex commented 7 years ago

@handcode great thank you, didn't know about inheritance unavailability! What if I want to use your config, but with ssl additions? How would it look like correctly?

handcode commented 7 years ago

The only reason why we took the nginx into the PHP/FPM container was to be able to "cheap" deliver static files without the FPM, not to have a way to provide non-trivial web server configurations.

We could add an optional include for eg. /etc/nginx/vhost.d/*.conf in the server{} block of the default configuration so that it would be possible to inject files inside the default server{} block.

But that would not cover all possible usecases. For example, it would not be possible to extend the location ~ \. php$ {} block or change other options already set.

Long story short:

If you want to customize the vhost configuration in this container, the way via a file in conf.d like you did is ok. You just have to define the whole server{} block according to your requirements.

But i would recommend you to setup a nginx proxy before the PHP container and make the "advanced" configuration there.

For example, in our setups we usually use the https://github.com/jwilder/nginx-proxy or simple nginx container which we customize/build specifically for a project.

StalkAlex commented 7 years ago

I get it, I think I will split nginx and php into two containers. Thank you for your help! It was really nice.

schmunk42 commented 7 years ago

I get it, I think I will split nginx and php into two containers.

I won't really recommend to split them (you'll need container volumes for static files, which is heavily annoying), but to use a SSL termination proxy like @handcode described it.

StalkAlex commented 7 years ago

We're already using volumes, just one - host project folder to be more precise, now it binds to php container, after splitting it will bind to nginx and php both, not a big problem. Our application isn't runnig in clouds, we're using hardware server and we don't use load balancers yet. Wouldn't be extra proxy more like overhead now?

schmunk42 commented 7 years ago

Wouldn't be extra proxy more like overhead now?

The proxy has no noticeable overhead (in our setups).

Just FYI: If you're running nginx in a separate container you can also use the standard flavours of this image (without -nginx in the tag)

StalkAlex commented 7 years ago

As I understand correctly recommended way is to have not splitted image and connect it to another nginx or jwilder proxy container?

schmunk42 commented 7 years ago

As I understand correctly recommended way is to have not splitted image and connect it to another nginx or jwilder proxy container?

Running PHP-fpm and nginx in one image with forego is not 100% the workflow Docker suggests, but it has been proven to be much less error-prone that splitting those processes into two containers.

And yes, If you want SSL termination, we would recommend using a proxy container for that.