Add mod_php7 check to htaccess and remove php5 code

so something like this:

So something like: http://stackoverflow.com/questions/12710842/php-internal-encoding ?

Can we get a code comment that explains why we're setting those settings?

Yes we can...

I don't see why not, you opened an issue for that didn't you? #2455455: Fix outdated Unicode requirements check

Just for reference, the PHP5 block looks like this (values all aligned vertically):

<IfModule mod_php5.c>
  php_flag session.auto_start               off
  php_value mbstring.http_input             pass
  php_value mbstring.http_output            pass
  php_flag mbstring.encoding_translation    off
</IfModule>

If it's a standard, that's the only place where it's being used as far as I can see ;)

I do too, aligning vertically can actually be problematic and cause conflicts or changes to irrelevant lines when doing patches.

@benjy could you review #2455455: Fix outdated Unicode requirements check as well? As you had created that issue to begin with :)

+++ b/.htaccess
@@ -41,6 +41,16 @@ AddEncoding gzip svgz
+# PHP 5.6 and up introduces default encoding settings. Set these to UTF-8
+# to comply with Drupal's Unicode requirements check.
+# See https://wiki.php.net/rfc/default_encoding.
+<IfModule mod_php7.c>

Here's the thing if these are introduced by PHP5.6 shouldn't we be setting these for PHP5 as well?

Relatedly I think we'll need to cope with the always_populate_raw_post_data setting - see #2456025: PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting

This will still be fine on PHP <5.6?

+++ b/.htaccess
@@ -41,6 +41,22 @@ AddEncoding gzip svgz
+<IfModule mod_php5.c>
+  php_value default_charset "UTF-8"
+  php_value php.input_encoding "UTF-8"
+  php_value php.internal_encoding "UTF-8"
+  php_value php.output_encoding "UTF-8"
+</IfModule>

Just use the section for php5 above.

Please move *all* of those into a .user.ini file. They just don't belong into .htaccess, and when there they only apply to mod_php installation, which should be a infinite part of the PHP production installations nowadays (including all major cloud platforms, which are all using PHP-FPM).

Can't we do both .htaccess and .user.ini, so this still works with mod_php? See #2463967: Add .user.ini

@Damien Tournoud: in .user.ini files we cannot test for PHP versions, so if we were to ship a .user.ini along with Drupal, it may include deprecated settings for PHP7 users. Would this pose problem or does PHP7 not log any warnings if we use those?

Discussed with @alexpott. This issue can probably remain normal since sites can fix the .htaccess as needed, but it is a bug.

Let's update the issue summary to specifically explain the impacts of this.

Patch no longer applies.

Re-rolled patch

UTF-8 is the default encoding in PHP7, so leaving out the encoding settings is fine.

Attached wrong patch. Here is the right one.

The default values for PHP character encoding are correct, so they should not be set. However, .htaccess should contain a PHP7 section for settings that are required.

This installs cleanly against Drupal Core 8.8.0-dev. Marking RTBC.

Hi,
Is there a similar issue filed for the .htaccess that gets created in sites/default/files? I can't find one. The code is in includes/file.inc in file_htaccess_lines(). (In D8 this delegates to Drupal/Component/Phpstorage/FileStorage which has a similar function htaccessLines().)

Yes, the issue for the auto-generatated .htaccess in the files directory is #2820611: FileStorage generated .htaccess doesn't cover PHP 7.

Ah thanks.

I don't think this is a bug per-se. Nothing is broken by not having this in your .htaccess and as a change to the default .htaccess we should have a change record and some release notes because root .htaccess changes can require special upgrade steps.

I created a change record.

Might this be something that could be backported to 8.7.x presuming it is included in 8.8.x?

This is looking good but whilst we're here we could remove all the PHP5 stuff from .htaccess files at the same time. We can do this now we don't support PHP5.

It'd be good to remove the PHP5 stuff from \Drupal\Component\PhpStorage\FileStorage::htaccessLines too.

Might it be worth splitting off the PHP5 removal into a separate issue as this one seems like a tiny shoe-in?

Agreed. This is ready to go as is and can be back-ported to 8.7. We can remove PHP 5 support from 8.8 in a separate task.

We've got another copy of htaccess file that needs to be updated as well.
https://git.drupalcode.org/project/drupal/blob/8.8.x/core/assets/scaffol...

This patch addresses #56, #57 and #63. There are no remaining references to mod_php5 in the codebase following this.

Patch looks good. Now that Drupal requires PHP 7, this is a bug. It could have been fixed years ago if the scope of the issue had not been expanded unnecessarily.

As this is a change in the root .htaccess we need to add a release note about this. I/e complete the @todo section.

I've actually often wondered about this! Glad to see this happening.

Committed a585dd6a7e and pushed to 9.1.x. Thanks!

Going to ping a release manager about backport to 9.0.x and 8.9.x

@catch +1'd the backport.

Needs to fix version and publish

Done.

Added version