PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting

Highly obscure. So as the linked blog post points out we cannot use ini_set() to fix this. Which means we can only fix this for Apache as you did in your patch.

So people will run into this problem when they run the tests on Nginx for example. We should probably document at https://www.drupal.org/requirements/php#settings or https://www.drupal.org/requirements/php#drupalversions that Drupal 8 needs always_populate_raw_post_data = -1 for REST.

Maybe we should also document at the test cases that they will fail on PHP 5.6 when this setting is wrong?

Added a little warning in the test cases and got rid of vertical alignment which could be problematic down the line

1. +++ b/.htaccess
   @@ -35,10 +35,13 @@ AddEncoding gzip svgz
   -  php_flag session.auto_start               off
   -  php_value mbstring.http_input             pass
   -  php_value mbstring.http_output            pass
   -  php_flag mbstring.encoding_translation    off
   +  php_flag session.auto_start off
   +  php_value mbstring.http_input pass
   +  php_value mbstring.http_output pass
   

   I think we should not change the indentation of the assigned values in this issue, let's leave it as is.

2. +++ b/core/modules/rest/src/Tests/CreateTest.php
   @@ -302,6 +302,8 @@ public function createAccountPerEntity($entity_type) {
   +    // Note: this will fail with PHP 5.6 and always_populate_raw_post_data
   +    // set to something other than -1. See https://www.drupal.org/node/2456025.
   

   I think we should replace the "and" with "when".

How about we implement hook_requirements() in rest.install to check PHP 5.6 for this setting?

1. +++ b/core/modules/rest/rest.install
   @@ -0,0 +1,30 @@
   +  if (ini_get('always_populate_raw_post_data') != -1) {
   

   We should also check the PHP version here, since this is only a problem on PHP >= 5.6.0. We don't need to nag users if it does not concern them.

2. +++ b/core/modules/rest/rest.install
   @@ -0,0 +1,30 @@
   +  else {
   +    $requirements['always_populate_raw_post_data'] = array(
   +      'title' => t('always_populate_raw_post_data PHP setting'),
   +      'value' => t('Set to -1.'),
   +      'severity' => REQUIREMENT_OK
   +    );
   

   I don't think we need the REQUIREMENT_OK part, since this information is not that important and just clutters the status report page.

3. +++ b/core/modules/rest/src/Tests/CsrfTest.php
   @@ -80,6 +80,8 @@ public function testCookieAuth() {
   +    // Note: this will fail with PHP 5.6 and always_populate_raw_post_data
   +    // set to something other than -1. See https://www.drupal.org/node/2456025.
   

   This comment should also be updated to use "when", same as the other one.

Thanks a lot, looks good to me now.

Did a bit of manual testing on Ubuntu 15.04 and hook_requirements() triggers as expected when the ini setting is wrong.

Committed and pushed to 8.0.x. Thanks!

So this broke InstallUninstallTest in PHP 7. See #2454439-98: [META] Support PHP 7 for the fun details. We need an additional condition to not run this on PHP 7 since this ini setting no longer exists there.

No need to revert, we just need to extend it a bit. I'll open a issue tomorrow, but if someone wants to work on it, please do so.

The follow-up issue: #2496943: Add condition to ignore PHP 7 on rest requirements check

This has been reported in D7 #2524240: $HTTP_RAW_POST_DATA Deprecated in PHP 5.6, Causing Warnings in D7 Error Logs.

There is a typo in the error message in this patch ("HP" instead of "PHP"), and shouldn't we take into account #2496943: Add condition to ignore PHP 7 on rest requirements check at the same time and make this apply to PHP 5.6 only, rather than 5.6 and up?

Thank you @longwave. The reason that happened is too silly to mention here.

Hi
I just walked through the following test on a Debian Jessie with PHP 5.6.9

1. Installed minimal clean Drupal 7.38
2. Enabled the file module (not enabled in the minimal profile)
3. Verified the value of always_populate_raw_post_data to be 0 according to admin/reports/status/php
4. Verified that I could reproduced the notice via curl -X POST -H "Content-Type: application/json" -d "{foo: bar}" http:/// (see https://www.bram.us/2014/10/26/php-5-6-automatically-populating-http_raw..., TL;DR with always_populate_raw_post_data set to 0 the notice is issued whenever a supported MIME-type is detected, setting the Content-Type will do that)
5. Applied patch from #21
6. Verified that always_populate_raw_post_data changed value to -1
7. Verified that I could no longer reproduce the notice via curl
8. Changed the always_populate_raw_post_data back to 0 in .htaccess
9. Verified that the always_populate_raw_post_data requirement failed and I got the "The always_populate_raw_post_data PHP setting should be set to -1..."

All in all the patch seems to work as expected.

From #20:

shouldn't we take into account #2496943: Add condition to ignore PHP 7 on rest requirements check at the same time and make this apply to PHP 5.6 only, rather than 5.6 and up?

Yeah, I agree we should incorporate that at the same time. Marking needs work for that.

Also a minor issue (and I think affects Drupal 8 too):

+        'description' => t('The always_populate_raw_post_data PHP setting should be set to -1 in PHP versions 5.6 and up. Please check the <a href="https://php.net/manual/en/ini.core.php#ini.always-populate-raw-post-data">PHP manual</a> for information on how to correct this.'),

We usually like to leave the /en/ out of php.net URLs in the user interface. That way we don't assume a language and the user gets sent to the correct language via whatever mechanism php.net uses to detect that.

Incorporated comments from #20 and #28. Also opened #2591919: Reference php.net 'international' URL in help text for D8 (indeed it affects Drupal 8 too).

I ran into this while doing a fresh D8 RC2 install on PHP 5.6.15 on Apache 2 with PHP being run via the CGI interface. The .htaccess configuration that is supposed to set always_populate_raw_post_data to -1 had no effect because it is inside a <IfModule mod_php5.c> tag and so will not be run when using anything other than mod_php on Apache. This resulted in the D8 install hanging at "Initializing" in the Install Site stage of the install.

I think it would make sense to test for the PHP version and the configuration of always_populate_raw_post_data to -1 at the Verify Requirements stage of the install process to provide a warning and instructions on how to correct the problem in php.ini. This will help people using PHP 5.6.x with something other than Apache and mod_php.

@Beanjammin This one has already been committed to Drupal 8. Please open a new follow-up issue for the additional work and link it here.

Thanks for letting me know, with the issue still flagged as needing review it wasn't obvious that it had already been commited.

I have posted a patch to #2485847: Automatically populating $HTTP_RAW_POST_DATA is deprecated and will be removed in a future version

This causes the symptoms of issue #2501851: Install Fails on WAMP (Drupal\Core\Config\UnmetDependenciesException: Configuration objects) to appear on my Lighttpd 1.4 installation with PHP-FPM 5.6.14.

P.S: php-fpm uses .user.ini files, where the php.ini settings can be overridden on a per-application / per-directory basis.

Hi, To which file to apply the patch on the directory..

Edit: my bad. Sorry for spam.

This was committed before 8.0.0. For a backport to D7, a new issue should be created.

Note this also introduced a small regression: #2931883: Unneeded always_populate_raw_post_data requirements check while on CLI.

It's best not to just close issues that have a patch to review, without first making sure there is actually another issue open to track it. Otherwise, the patch tends to get lost.

In this case, there was #2524240: $HTTP_RAW_POST_DATA Deprecated in PHP 5.6, Causing Warnings in D7 Error Logs, but it was marked as a duplicate of this one. I reopened it now to discuss the Drupal 7 backport. I'm not sure this actually does make sense to backport, but in any case, discussion can continue there.

I agree in theory, but that requires that in practice D7 maintainers do file such issues. This hasn't moved forward since November 2015, >2 years ago, so rather than keeping things confusing for related D8 issues, I'd rather have clarity for D8, rather than ambiguity for both D7 and D8.

I don't think it's specifically the responsibility of D7 maintainers to do that (and in any case, there are very few D7 maintainers still active, maybe like 5%). If we're going to ask them to do anything at all, I think closing old issues and then opening new ones for the same purpose is not high on the priority list :) It really should be up to whoever wants the issue closed to make sure there are followups created for whatever was keeping it open in the past.

Can someone please tell me which patch is for D7?