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

PHP_MEMORY_LIMIT memory limit override does not work #19

Closed
jessedobbelaere opened this issue Sep 9, 2024 · 3 comments
Closed

PHP_MEMORY_LIMIT memory limit override does not work #19

jessedobbelaere opened this issue Sep 9, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@jessedobbelaere
Copy link

jessedobbelaere commented Sep 9, 2024

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

memory_limit=${PHP_MEMORY_LIMIT}

but also by php-fpm.conf using:

php_admin_value[memory_limit] = "256M"

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

  • Craft version: /
  • PHP version: 8.3
  • Database driver & version: /
  • Plugins & versions: /
@jessedobbelaere jessedobbelaere added the bug Something isn't working label Sep 9, 2024
@internetztube
Copy link

I also still have the same problem. @jasonmccallister

@jasonmccallister
Copy link
Member

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
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants