craftcms / image

Container images that are used as the base for Craft CMS container applications
https://craftcms.com
11 stars 0 forks source link

`PHP_MEMORY_LIMIT` memory limit override does not work #19

Closed jessedobbelaere closed 1 month ago

jessedobbelaere commented 2 months ago

Description

tl;dr The README mentions you can specify the env variable PHP_MEMORY_LIMIT, but this does not work.

I figured out that memory_limit is set by both 60-craftcms.ini

https://github.com/craftcms/image/blob/77f50583aff272056529b1cebceb77f07984581f/etc/php.d/60-craftcms.ini#L1

but also by php-fpm.conf using:

https://github.com/craftcms/image/blob/77f50583aff272056529b1cebceb77f07984581f/etc/php-fpm/php-fpm.conf#L225

As a recap for php_admin_value, these cannot get overwritten with regular .ini files:

Unlike php_value, php_admin_value stands immune to local overwrites. Regardless of the attempts made in individual directories or files to alter the same configuration setting, the global directive set via php_admin_value remains supreme. This characteristic renders it an excellent choice for safeguarding vital configurations against inadvertent alterations.

So in fact, there's no way that any .ini file or env variable PHP_MEMORY_LIMIT can override settings like memory_limit.

Steps to reproduce

  1. Clone this repository
  2. cd examples/nginx
  3. Add an env variable to the docker-compose.yaml, like advertised in the README.md:
version: "3.8"
services:
  web:
    image: craftcms/web:local
    build:
      context: .
      dockerfile: Dockerfile
      args:
        - php_version=8.2 # this is passed to the FROM in the Dockerfile
    ports:
      - "8080:8080"
    volumes:
      - ./local:/app
+   environment:
+       PHP_MEMORY_LIMIT: 512M
  1. docker compose up -d
  2. Visit http://localhost:8080 and search for memory_limit which is set to 256M which is the value used by php_admin_value

Screenshot 2024-09-10 at 01 19 14@2x

  1. As a workaround i'm commenting out the line in /etc/php-fpm.conf in a Dockerfile build that extends from the 8.3 image 😅 After commenting it out, the override actually works.
RUN sed -i '/^php_admin_value\[memory_limit\]/s/^/;/' /etc/php-fpm.conf

[!NOTE]
I have a similar issue with open_basedir which is set through php_admin_value so I cannot override it with a .ini file. I have to use sed to comment out the open_basedir first before overriding it. I need to override the open_basedir because of jpegoptim, optipng, cwebp, cavif, ... which are all common image optimizers used in Craft CMS. Should we just remove all php_admin_value settings and turn them into regular .ini settings so devs have the option to add overrides?

Additional info

internetztube commented 2 months ago

I also still have the same problem. @jasonmccallister

jasonmccallister commented 1 month ago

Thank you for the report.

I'm going to remove these setting from the php-fpm.conf file:

; Performance settings
php_admin_value[memory_limit] = "256M"
php_admin_value[max_execution_time] = "120"

; Upload settings
php_admin_value[post_max_size] = "100M"
php_admin_value[upload_max_filesize] = "100M"
jasonmccallister commented 1 month ago

This should be changed in these builds:

  1. https://github.com/craftcms/image/actions/runs/11128759446
  2. https://github.com/craftcms/image/actions/runs/11128753428
  3. https://github.com/craftcms/image/actions/runs/11128714069

Please reopen if there is anything out of the ordinary.

Thank you @jessedobbelaere @internetztube for the report again!