Use .user.ini file for PHP settings

Should we add this file to the FilesMatch directive in the default .htaccess and web.config files?

# Protect files and directories from prying eyes.
<FilesMatch "\.(engine|inc|install|make|module|profile|po|sh|.*sql|theme|twig|tpl(\.php)?|xtmpl|yml)(~|\.sw[op]|\.bak|\.orig|\.save)?$|^(\.(?!well-known).*|Entries.*|Repository|Root|Tag|Template|composer\.(json|lock)|web\.config|yarn\.lock|package\.json)$|^#.*#$|\.php(~|\.sw[op]|\.bak|\.orig|\.save)$">

There is also a PHP settings section in default.settings.php that may need to reference this new file. Possibly also need to review example.settings.local.php and ht.router.php

zend.assertions 1 more meaningful as assert.exception 0 by default

#42, adding a rule to block serving ini files in web.config and .htaccess sounds reasonable in this case, but it means we'll block any ini file. While this is still reasonable, I'm not sire if this would be a breaking change. Should we discuss this here? Should we create a new issue? Should we just add that blocking rule here?

About example.settings.local.php, this is what is said there about assertions:

/**
 * Assertions.
 *
 * The Drupal project primarily uses runtime assertions to enforce the
 * expectations of the API by failing when incorrect calls are made by code
 * under development.
 *
 * @see http://php.net/assert
 * @see https://www.drupal.org/node/2492225
 *
 * It is strongly recommended that you set zend.assertions=1 in the PHP.ini file
 * (It cannot be changed from .htaccess or runtime) on development machines and
 * to 0 or -1 in production.
 *
 * @see https://wiki.php.net/rfc/expectations
 */
assert_options(ASSERT_ACTIVE, TRUE);
assert_options(ASSERT_EXCEPTION, TRUE);

Because zend.assertions can only be changed on runtime when its value is 0 I guess it's not needed to say anything here. See https://www.php.net/manual/en/ini.core.php#ini.zend.assertions for more information.

#43, as explained in the previous link, zend.assertions can be changed on runtime with some limitations (for example, if php.ini set it to -1 there's nothing you can do because assertion code is not even generated), so I guess we should stick to assert.exception because it can be changed on runtime.

So I guess we only need to decide if we block ini files or not to push this issue forward.

adding a rule to block serving ini files in web.config and .htaccess sounds reasonable in this case, but it means we'll block any ini file

We already block web.config (not *.config), can't we use the same pattern to block this file?

We already block web.config (not *.config), can't we use the same pattern to block this file?

Yes, you are right, we can just block that file instead of using a wildcard.

Branch rebased, added .user.ini to web.config and .htaccess so they are not served, and test updated so this is tested.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

probably false positive

I think this change looks good now, the change record that was requested has been added. MR needs to be updated because it looks like it is not mergable.

tunic → closed merge request !498

I've created a new MR for 9.5.x: For this, I had to create a new commit including commits from 10.1.x. Not sure if this is the best way but O had some troubles cherry-picking the commits, I hope this is ok.

As said previously, I've closed the 9.2.x MR because that Drupal release is not maintained anymore.

10.1.x branch looks good. The 9.5.x branch has trailing space in one of the web.config files and is missing a .user.ini test fixture. Once those errors are fixed, all tests should pass.

This is making changes to .htaccess let's have a release note snippet to add to the release notes.

#56 not sure about the release notes tag, it is just a way to locate all the issues for a release when writing the release note or do we need to write an actual release note? In that case, would be the linked change record enough?

Tests are green, let's go to Needs Review.

The MR at #61 (2463967-add-user.ini) has exactly the same changes as the on open at #27 (10.1.x), what is the reason to open it?

Well, comment in #62 is not true anymore because I've rebased the two initial merge requests. Anyway, if the last open is better for any reason we can close the other and rebase that one.

I opened the last one for the 11.x branch. I wasn't able to change the target branch of the previous merge request.

Ops, I didn't see the destination of the merge, sorry. So we have 3 MR, one per release (9.x, 10.x and 11.x). Thanks!

I am doing triage on the core RTBC queue → .

The issue summary does not have a proposed resolution. That means the reviewer has no way to know that the changes in the MR are what has been agreed to. However, in this case it is pretty obvious from the first paragraph what this is doing. There are no remaining tasks either.

I skimmed through the comments and found that all questions (#30, #37) have been answered (#38), except one for me.

#56/#56. Changes to files such as .htacccess usually have an sentence or two in the Release notes to help the site owner know what they need to do. For example, see drupal 10.1.0-rc1 → in the 'Changes to site-owner managed files' section. It that sort of sentence that should be in the 'release note snippet' of the Issue Summary.

I then read the CR and think it needs work. I think it should have something to help readers know if they must or should change something on their site. Maybe even a before and after section.

@Darren Oh, there are instructions for rebasing on the same branch or a difference branch in the wiki. See https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → .

Adding some credit, but did not complete the process.

I considered leaving this RTBC for other committers to review but decided to set to NW to ensure that the CR and release note snippet was ready.

Oh, and not surprising, the diff for 11.x no longer applies, others wise I would have reviewed the comments.

There is another issue: assert.active shouldn't be use:

assert.active bool
Enable assert() evaluation. zend.assertions should be used instead to control the behaviour of assert().

See https://www.php.net/manual/en/info.configuration.php#ini.assert.active.

It seems we should then zend.assertions. However, in the Drupal wiki, page "Runtime Assertions" ( https://www.drupal.org/node/2492225 → ) the docs still says assert.active should be set to 1 (see https://www.drupal.org/node/2492225#s-configuring-your-servers → ).

I think first we need to agree on what to do with those two settings (assert.active and zend.assertions). I think Drupal should use the recommendation by PHP. Also, assert.active is deprecated on PHP 8.3 and "the zend_assertions INI setting has superseded it for a while", see https://wiki.php.net/rfc/assert-string-eval-cleanup.

But I'm not sure if this should be discussed here or in another issue.

From my point of view we should:

• - Update this patch to use zend.assertions
• - Update the linked documentation

tunic, the scope of this issue is limited to making .user.ini files match .htaccess and web.config. Changes to the contents of those files should be discussed in a different issue.

Follow up added: ✨ Use zend.assertions PHP setting instead of assert.active Closed: duplicate .

@tunic there's pre-existing issue 📌 Fix deprecated assert_options() function usage for PHP 8.3 Needs review it needs to do the same steps (even update docs) so follow-up #70 can be closed as duplicate and existing issue just needs polishing summary from ✨ Use zend.assertions PHP setting instead of assert.active Closed: duplicate

Moving to NW for the change record updates requested per #66

The requested change record updates have already been made.

The requested change record updates have already been made, so removing the tag.

Bumping this to the 10.2.0 release notes tag as 10.1.0 is already out the door.

This looks super simple and straightforward. RTBC.

I rebased the branch and edit the MR to be merged into 10.2.x. No other changes done, so I guess is still RTBC.

The change record should be updated to tell people how to use the scaffold's replace or append operations to modify the core provided .user.ini. Using a patch to do this is unnecessary. See https://www.drupal.org/docs/develop/using-composer/using-drupals-compose... →

The merge requests seem to be a little bit broken too.

Given that 📌 Remove mentions of assert.active from .htaccess Needs work is removing the only piece of code in .htaccess that is being copied to the .user.ini in this issue, I guess this issue is now obsolete because the .user.ini file will be completely empty.

Therefore, I'm closing this as won't fix.

That issue is for D11 and has not been committed. This is ready to go once the change record is updated.

Just fyi 11.x is the “main” branch and D10 releases are tagged off it. So if that issue lands it’ll be included in 10.2

If we include this in 10.2 what would happen in 11? Given that 11 removed the code in .htaccess that this issue copies to .user.ini then.user.ini in 11 would be blank file.

Change record has been updated. Some of these changes are needed even if Drupal does not ship any PHP settings. Helping users who are not PHP experts find the right place to set PHP options is also valuable.

There are three merge requests in the summary. Two should be closed. I assume the ones labeled for 9.5.x and 10.1.x are wrong, but the comments above and changes to the target branch make me unsure. Please close the incorrect merge requests, or document which should be closed in an issue comment and under an independent header in the IS. Thanks!

tunic → closed merge request !3004

tunic → closed merge request !3868

MRs removed as requested.

Committed de1a11e and pushed to 11.x. Thanks!

diff --git a/.user.ini b/.user.ini
index 598176ba2f..95d89560ff 100644
--- a/.user.ini
+++ b/.user.ini
@@ -4,7 +4,7 @@
 ; at runtime.
 ;
 ; PHP only reads settings from this file if it is running as a CGI script. If
-; PHP is running as an Apache module, see .htaccess. 
+; PHP is running as an Apache module, see .htaccess.
 
 ; Disable PHP assertions.
 assert.active = 0
diff --git a/composer.lock b/composer.lock
index 15d5a5ed79..e239adab8e 100644
--- a/composer.lock
+++ b/composer.lock
@@ -9634,5 +9634,5 @@
     "platform-overrides": {
         "php": "8.1.0"
     },
-    "plugin-api-version": "2.3.0"
+    "plugin-api-version": "2.6.0"
 }
diff --git a/core/assets/scaffold/files/user.ini b/core/assets/scaffold/files/user.ini
index 598176ba2f..95d89560ff 100644
--- a/core/assets/scaffold/files/user.ini
+++ b/core/assets/scaffold/files/user.ini
@@ -4,7 +4,7 @@
 ; at runtime.
 ;
 ; PHP only reads settings from this file if it is running as a CGI script. If
-; PHP is running as an Apache module, see .htaccess. 
+; PHP is running as an Apache module, see .htaccess.
 
 ; Disable PHP assertions.
 assert.active = 0

Made the above fixes on commit.

Looks it was not pushed and assert will throw deprecation on PHP 8.3 https://www.php.net/manual/en/info.configuration.php#ini.assert.active

Thanks for pointing that out @andypost. NW for #94.

@andypost why doesn't our htaccess trigger deprecations then?

So 📌 Remove mentions of assert.active from .htaccess Needs work is the issue to remove that - I think that once that one lands we could actaully close this as won't fix as user.ini will be empty.

I bet this deprecation is going to apache logs so invisible

The deprecation notice is out of scope for this issue. This issue should not be closed even if .user.ini is empty. It provides important documentation for users. Those of you who are PHP experts may not be able appreciate how many hundreds of hours have been lost because we have avoided implementing this for so many years. If you think everyone knows when they need to use .user.ini, just look at the history of this issue. Our reluctance to move this issue forward has hindered Drupal adoption for no good reason.

I still see no reason to make a common practice reading 2 config files per each request for Apache.

Moreover composer overrides allow to append core's default .htaccess or replace it - that's documentation which needs improvement instead of performance regression

#102, as said in the issue's body the two files are not processed in the same request because user.ini is only processed when CGI/FastCGI SAPI is used and .htaccess is only processed when PHP run as Apache module. Unless this information is wrong there should not be a performance penalty for having both files and both are needed to be able to configure both ways of using PHP (although user.ini will be currently empty).

Changed the title to account for the possibility that PHP settings might be empty by the time this is committed.

I'm confused here, there's a commit showing in December from @alexpott with the same diff as the MR - so what's the status of this issue - is it only needs review for the release note?

It was set back to Needs Review due to a question that was answered.

Didn't https://git.drupalcode.org/project/drupal/-/commit/77af69069979b46f2b5df... revert this?

Given 📌 Remove mentions of assert.active from .htaccess Needs work I think this should be won't fix.

Even after the deprecated assert.active directive is fixed, having a user.ini that matches .htaccess is necessary documentation even for advanced PHP users, as the discussion in this issue has demonstrated.

I'm pretty certain we can remove the changes to web.config and .htaccess from this merge request - they are not necessary - which makes this change much easier to land. Leave the changes to HtAccessTest in and you'll see that access to .user.ini is still blocked.

Untagging, since this did not land in time for 10.3.

Currently there are many .htaccess files with the following directive:

<IfModule mod_php.c>
  php_flag engine off
</IfModule>

For example in: /tmp, files, private, ...
This directive is not read by php-fpm, so php remains active in these folders.
I believe this is a serious security problem.
A .user.ini file should also be inserted with the directive:
engine = off