Bug #66763 [Sus]: always_populate_raw_post_data=0 BC issue with in-built web server

Edit report at https://bugs.php.net/bug.php?id=66763&edit=1 ID: 66763 Updated by: tyrael@php.net Reported by: sixd@php.net Summary: always_populate_raw_post_data=0 BC issue with in-built web server Status: Suspended Type: Bug Package: Built-in web server Operating System: Linux PHP Version: 5.6Git-2014-02-24 (Git) Assigned To: tyrael Block user comment: N Private report: N New Comment: "Just to repeat/confirm: we do not use $HTTP_RAW_POST_DATA and we only use php://input. Yet the deprecated notice is thrown. Isn't that a bug?" the deprecated notice only presented when $HTTP_RAW_POST_DATA is populated, regardless if you touch the $HTTP_RAW_POST_DATA variable or not. but I think I already mentioned this, and also explained why we can't only throw the deprecated notice when the variable is actually accessed. "at least the error message should be changed because it is currently incorrect? (It suggests us to use php://input but this is what we are already using this AFAIK.)" it suggests you to change your php.ini and use php://input instead of $HTTP_RAW_POST_DATA. if you aren't using $HTTP_RAW_POST_DATA you only have to change your php.ini to explicitly opt-out from populating $HTTP_RAW_POST_DATA. "It's bad practise but it's also the default configuration. Bad practise == default == widespread (ie. dozens of thousands of servers running with display_errors on). " you are right about that one. I've started a thread back like 2 years ago to sync our defaults with what we have in php.ini-production but we didn't get a consensus on it. "Now I have an idea that could solve the problem nicely for us: could you make ini_set work for this setting value?" when your code(containing your ini_set call) is executed, the population of this variable already happened, so ini_set won't work for it. and afaik we don't have a way for jit population for non-superglobal variables (as I mentioned $HTTP_RAW_POST_DATA is not a superglobal unfortunatelly). Previous Comments: ------------------------------------------------------------------------ [2014-10-21 22:54:49] matt at piwik dot org > The current error message tells you the problem ($HTTP_RAW_POST_DATA is deprecated, and will be > gone in a future version), and how to remove the warning (always_populate_raw_post_data=-1) and what > should you use instead of $HTTP_RAW_POST_DATA(php://input). Just to repeat/confirm: we do not use $HTTP_RAW_POST_DATA and we only use php://input. Yet the deprecated notice is thrown. Isn't that a bug? at least the error message should be changed because it is currently incorrect? (It suggests us to use php://input but this is what we are already using this AFAIK.) > running with display_errors=On in production is a bad practice It's bad practise but it's also the default configuration. Bad practise == default == widespread (ie. dozens of thousands of servers running with display_errors on). > imo a normal user would just either change this in the php.ini or complain to their > hoster/developer to fix the issue after upgrading their php installation. agreed but still it means thousands of hours of frustration for users. Now I have an idea that could solve the problem nicely for us: could you make ini_set work for this setting value? ini_set('always_populate_raw_post_data', -1); This would provide very nice workaround for us and many other PHP apps that could just add this ini_set. Thanks for considering! ------------------------------------------------------------------------ [2014-10-21 12:58:03] tyrael@php.net and ofc. feel free to escalate this issue (drop a mail to internals@) if you think that it would require a bigger audience than those following this bugreport. ------------------------------------------------------------------------ [2014-10-21 12:56:49] tyrael@php.net "The message only mentions $HTTP_RAW_POST_DATA and does not mention MIME type. Maybe the error message could explain that the MIME type used is relevant to the warning being raised if applicable?" I disagree. The current error message tells you the problem ($HTTP_RAW_POST_DATA is deprecated, and will be gone in a future version), and how to remove the warning (always_populate_raw_post_data=-1) and what should you use instead of $HTTP_RAW_POST_DATA(php://input). I don't think it would be a good idea to start documenting the various ways which can trigger the population of the $HTTP_RAW_POST_DATA variable in the error message, you can check out the online documentation for that. I do agree, that the current way for triggering the deprecated message is a bit sub-optimal (we don't show you the message when you try to access the $HTTP_RAW_POST_DATA variable, but when $HTTP_RAW_POST_DATA is populated. This is a technical limitation which we can't really help ($HTTP_RAW_POST_DATA is not a super global, but a normal global variable, so adding a check which emits the warning when a global variable accessed with this name would slow down every access to any global variable), "Likely from my experience even more people will be surprised when the BC is broken in PHP 5.6 because of un-documented MIME types (and possibly other codepaths) that throw a "headers already sent" error. Breaking BC with a not so useful error message that mentions a feature that is not used and not found in most codebases." I disagree. 1, We don't consider adding new warnings/notices/deprecated messages BC breaks, because those shouldn't affect your production code (running with display_errors=On in production is a bad practice). 2, Having a clear error message which also tells you how to fix the problem is a much better thing than changing the default behavior causing some random error in your app about $HTTP_RAW_POST_DATA being undefined. "Ideally PHP should not break BC in this way, because PHP should not require users to have access to their php.ini to fix default PHP 5.6 config. As a popular PHP app maker we would still kindly request to consider setting this to -1. " As I mentioned we don't consider deprecated error messages as BC breaks (even thought that it is a sad truth that some/many people do use display_errors=On in production). I'm hesitant to change the default value to -1 in a micro version, as it would change the default behavior and we would start getting the complaints about $HTTP_RAW_POST_DATA not getting populated, because many/most people depending on this variable does not explicitly specified always_populate_raw_post_data in their php.ini either. "dozens of people online with this issue and already 4-5 hours on our end spent understanding this. We are PHP experts.... imagine what normal PHP users will go through. Thanks for considering :-)" imo a normal user would just either change this in the php.ini or complain to their hoster/developer to fix the issue after upgrading their php installation. I think that most of the confusion will be cleared after the documentation gets updated to exmplain the situation. ------------------------------------------------------------------------ [2014-10-21 04:01:58] matt at piwik dot org > always_populate_raw_post_data=0 does not mean, that $HTTP_RAW_POST_DATA won't be > populated, it only means, that it will be only populated when a supported MIME type is present. The message only mentions $HTTP_RAW_POST_DATA and does not mention MIME type. Maybe the error message could explain that the MIME type used is relevant to the warning being raised if applicable? > won't be surprised when the support for it is dropped in a future version. Likely from my experience even more people will be surprised when the BC is broken in PHP 5.6 because of un-documented MIME types (and possibly other codepaths) that throw a "headers already sent" error. Breaking BC with a not so useful error message that mentions a feature that is not used and not found in most codebases. Asking confirmation: maybe the error is also thrown because we use 'php://input' ? if that's the case could the error message be updated to mention that both HTTP_RAW_POST_DATA and php://input and MIME types cause the warning to be raised? Ideally PHP should not break BC in this way, because PHP should not require users to have access to their php.ini to fix default PHP 5.6 config. As a popular PHP app maker we would still kindly request to consider setting this to -1. PS: dozens of people online with this issue and already 4-5 hours on our end spent understanding this. We are PHP experts.... imagine what normal PHP users will go through. Thanks for considering :-) ------------------------------------------------------------------------ [2014-10-20 12:56:30] tyrael@php.net "Currently the message implies that we are using $HTTP_RAW_POST_DATA but actually it's not used." you are potentially using the $HTTP_RAW_POST_DATA if you have anything but always_populate_raw_post_data=-1 in your config. always_populate_raw_post_data=0 does not mean, that $HTTP_RAW_POST_DATA won't be populated, it only means, that it will be only populated when a supported MIME type is present. as I've stated in my recent email(http://news.php.net/php.internals/78156) unfortunatelly it isn't possible (in an acceptable manner) to detect if your code actually touches the $HTTP_RAW_POST_DATA variable, but we still wanted to add a deprecated notice so people can start moving away from $HTTP_RAW_POST_DATA and won't be surprised when the support for it is dropped in a future version. As I mentioned in my email, I'm planning to better explain the situation in the online documentation/migration guide. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=66763 -- Edit this bug report at https://bugs.php.net/bug.php?id=66763&edit=1