Page MenuHomePhabricator
Create Task
Maniphest T9235

Add a warning about "always_populate_raw_post_data" being misconfigured
Closed, ResolvedPublic
Actions

Description

After I upgraded to the latest stable version, I encountered two problems

  1. storage upgrade can't automatically upgrade my schemes, eventually I upgraded it by myself one by one.
  2. when I clone my self-hosted repo. It always tell me fatal: protocol error: bad line length character: <br . git pullalso has the same problem. But git push seems work well. see below.
git clone http://ph.benemind.com/diffusion/EMSPEED/emspeed.git
Cloning into 'emspeed'...
fatal: protocol error: bad line length character: <br

http://ph.benemind.com/diffusion/EMSPEED/emspeed.git/git-upload-pack(You can't access the domain, it maps to a LAN ip address)
After I capture the HTTP frame, I found this in the response

<br />
<b>Deprecated</b>:  Automatically populating $HTTP_RAW_POST_DATA is deprecated and will be removed in a future version. To avoid this warning set 'always_populate_raw_post_data' to '-1' in php.ini and use the php://input stream instead. in <b>Unknown</b> on line <b>0</b><br />
<br />
<b>Warning</b>:  Cannot modify header information - headers already sent in <b>Unknown</b> on line <b>0</b><br />

Revisions and Commits

rP Phabricator
D16454rP72a03dc03e7d Add a setup warning for "always_populate_raw_post_data"

Related Objects

Event Timeline

• aelam created this task.Aug 21 2015, 7:56 AM
• aelam updated the task description. (Show Details)
• aelam added a project: Phacility.
• aelam added a subscriber: • aelam.
Herald added a subscriber: chad. · View Herald TranscriptAug 21 2015, 7:56 AM
• aelam updated the task description. (Show Details)Aug 21 2015, 7:57 AM
• aelam updated the task description. (Show Details)Aug 21 2015, 8:26 AM
epriestley removed a project: Phacility.Aug 21 2015, 11:00 AM
epriestley added a subscriber: epriestley.

To avoid this warning set 'always_populate_raw_post_data' to '-1' in php.ini

Did you try following these instructions?

chad added a project: Installing & Upgrading.Aug 21 2015, 5:14 PM
epriestley closed this task as Wontfix.Aug 23 2015, 2:37 PM
epriestley claimed this task.

Closing for lack of feedback.

• aelam added a comment.Aug 24 2015, 1:34 AM

sorry for the late response.
To avoid this warning set 'always_populate_raw_post_data' to '-1' in php.ini
I have known this settings, and it worked.

But it's really hard to find out the reason why git clone failed.
If I didn't set the proxy, I won't find out. Maybe some others may have the same problem

jbeta added a subscriber: jbeta.Aug 24 2015, 10:39 AM

I just hit problem (2) on this report myself, had to trace it the same way @aelam did.

It seems PHP 5.6, in its infinite wisdom, ships with a configuration which causes this warning even if you don't actually use $HTTP_RAW_POST_DATA. The end result in Phabricator (git chokes on a corrupted git pack) was confusing to me too. Perhaps a workaround or setup warning could be added to Phabricator?

jbeta added a comment.Aug 24 2015, 10:59 AM

See: https://bugs.php.net/bug.php?id=66763

It seems you also need both display_errors and display_startup_errors turned on to hit this, though. I don't know if that is something you'd want to support.

epriestley reopened this task as Open.Aug 24 2015, 12:09 PM

I think a setup warning is reasonable, particularly given that this looks like at least partially a PHP upstream configuration issue (not just one user running some weird custom-built PHP extension they wrote or whatever).

We disable display_errors in PhabricatorStartup and take over error handling shortly thereafter, but too late to prevent things like this. But I think it's probably OK (and maybe even desirable) for the display_errors stuff to be on during startup, at least until we encounter a problem it causes that can not be fixed by adjusting configuration, since it does make it much easier to figure out what's going on in cases like this. But I'm not sure if these options are really a net benefit, and perhaps the warning should be about the display_x_errors options.

That is, I think these are all reasonable:

  1. Warn about always_populate_raw_post_data not being set to -1 (on recent PHP versions only?).
  2. Warn about display_errors / display_startup_errors being enabled.
  3. Warn about both.

I'm slightly inclined to do (1) and wait for other problems which indicate (2) as good more conclusively, since it's vaguely possible that they make fixing some other set of problems which occur before we have a chance to do anything easier.

epriestley edited projects, added Setup; removed Installing & Upgrading.Sep 14 2015, 2:04 PM
epriestley added a project: Contributor Onboarding.Aug 22 2016, 6:33 PM
epriestley mentioned this in T8253: Upload file fails, probably my fault.Aug 22 2016, 8:41 PM
epriestley merged a task: T8253: Upload file fails, probably my fault.
epriestley added a subscriber: eadler.
epriestley reassigned this task from epriestley to jcox.Aug 22 2016, 8:47 PM

Welcome to PHP! This will familiarize you with PHP! PHP is a special language!

To reproduce this:

  • Set always_populate_raw_post_data = 1 in php.ini.
  • Also set display_errors and display_startup_errors to 1, I guess.
  • Probably restart your webserver to pick up the changes.
  • You'll probably get some garbage on pages? Or maybe only if you upload a file (T8253) or clone something (here) or maybe do Ajax things or try to download a raw file from Diffusion or Files. If you can't get any garbage to show up, let me know and I can find some better instructions which actually work.

Once you have the garbage showing up:

  • Modify PhabricatorPHPConfigSetupCheck to include a check for this setting being set to any value other than -1.
  • You can follow the general shape of the other existing checks in that file.
  • Text should be something like this:

You have "always_populate_raw_post_data" configured, but should disable this option.
You have "always_populate_raw_post_data" configured in your PHP settings. This option should be set to -1 to avoid deprecation warnings on startup.

To test:

  • Setup warnings are cached, so you may need to visit Config → Setup Issues to get your new warning to show up (this forces checks to re-run).
  • View the warning and make sure it looks reasonable.
  • Set always_populate_raw_post_data to -1 as instructed, restart webserver, warning should resolve itself.
epriestley renamed this task from schemes upgrades problem, and can't use `git clone` to Add a warning about "always_populate_raw_post_data" being misconfigured.Aug 22 2016, 8:54 PM
epriestley moved this task from Backlog to Basic on the Contributor Onboarding board.Aug 22 2016, 8:59 PM
jcox added a comment.Aug 25 2016, 8:40 PM

I've tried a whole bunch of different post requests (after setting those config values and restarting apache) but I haven't been able to get that garbage yet.

Herald added a subscriber: tycho.tatitscheff. · View Herald TranscriptAug 25 2016, 8:40 PM
epriestley added a comment.Aug 25 2016, 10:14 PM

Alright, let me see if I can get anywhere with this.

epriestley claimed this task.Aug 25 2016, 10:38 PM
epriestley added a subscriber: jcox.

I was ultimately able to reproduce this, but it took some work and the instructions above definitely weren't sufficeient:

  • Looks like it doesn't happen until PHP 5.6 or newer.
  • I set:
    • always_populate_raw_post_data = 1
    • display_errors = 1
    • display_startup_errors = 1
    • error_reporting = -1

Then I made a request with an unusual "Content-Type", a secret recipe I found by Googling random substrings of the error message:

$ curl -X POST -H "Content-Type: application/json" -d "{foo: bar}" http://t11507.epriestley.com/

This finally yielded results:

$ curl -X POST -H "Content-Type: application/json" -d "{foo: bar}" http://t11507.epriestley.com/
<br />
<b>Deprecated</b>:  Automatically populating $HTTP_RAW_POST_DATA is deprecated and will be removed in a future version. To avoid this warning set 'always_populate_raw_post_data' to '-1' in php.ini and use the php://input stream instead. in <b>Unknown</b> on line <b>0</b><br />
<br />
<b>Warning</b>:  Cannot modify header information - headers already sent in <b>Unknown</b> on line <b>0</b><br />
<!DOCTYPE html><html><head><meta charset="UTF-8" /><title>Login to Phabricator</title><meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1" /><link rel="mask-icon" color="#3D4B67" href="http://t11507.epriestley.com/res/phabricator/0460cb1f/rsrc/favicons/mask-icon.svg" /><link ...

I'll just add the setup warning and shoot you a diff, since I seem to have a stable repro case now.

epriestley added a revision: D16454: Add a setup warning for "always_populate_raw_post_data".Aug 25 2016, 10:49 PM
epriestley closed this task as Resolved by committing rP72a03dc03e7d: Add a setup warning for "always_populate_raw_post_data".Aug 26 2016, 2:38 PM
epriestley added a commit: rP72a03dc03e7d: Add a setup warning for "always_populate_raw_post_data".
epriestley mentioned this in 2016 Week 35 (Very Late August).Aug 27 2016, 12:42 AM
FauxFaux mentioned this in T9640: Make Phabricator compatible with PHP7.Jan 1 2017, 3:20 PM
Phabricator · Built by Phacility