Can someone give feedback on this?

It's a problematic breaking change and is not mentioned in the migration guide or php doc.

We do shared hosting and this php_admin_value[error_reporting] config, not bypassable by user code, give us the ability to simplify third party code management a lot.

We rely on this in production and we used it successfully with magento wordpress drupal moodle and others without any contraindication.

php_admin_value setted ini options values should not be bypassable/changed by user code  even if the option is error_reporting, as all previous version of php have done.

please fix this incompatible and wrong behavior in php7!

This occurs also with php_admin_value error_reporting set from Apache config.
Problem not specific to FPM.
I also find it a considerable nuisance.

I made a change to ZEND_FUNCTION(error_reporting)in
zend_builtin_functions.c which corrects this error.
I reverted this to something based on code in previous version of PHP.
As I am not experienced in changing PHP, I cannot say if this is a good
fix, but it establishes what the problem is.

New code:
ZEND_FUNCTION(error_reporting)
{
	zval *err;
	int old_error_reporting;

#ifndef FAST_ZPP
	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|z", &err) == FAILURE) {
		return;
	}
#else
	ZEND_PARSE_PARAMETERS_START(0, 1)
		Z_PARAM_OPTIONAL
		Z_PARAM_ZVAL(err)
	ZEND_PARSE_PARAMETERS_END();
#endif

	old_error_reporting = EG(error_reporting);
	if(ZEND_NUM_ARGS() != 0) {
		zend_string *key = zend_string_init("error_reporting",
sizeof("error_reporting")-1, 0);
		zend_string* value = zval_get_string(err);
		zend_alter_ini_entry(key, value, ZEND_INI_USER,
ZEND_INI_STAGE_RUNTIME);
		zend_string_release(key);
	}

	RETVAL_LONG(old_error_reporting);
}

Changed package category.

Changed OS, php version and description to reflect current status

Changed summary to better reflect the problem

Restored summary, i hate the 60 character limit, too restrictive!

this problem persists in the last version

the problem still persists on the latest version :( please fix this...it's a matter of a simple "if", rabell has also found where the code should be fixed!! someone can merge that?

also got hit by this in php 7.0.8. 

a composer module sets error_reporting(E_ALL) which breaks parts of our app due to E_NOTICE being thrown. previously, we could suppress those through php_admin_value[error_reporting]

Still persists in PHP 7.2.5. When it will be fixed?

besides that fpm seems to have still a lot of issues given that it is called the recommended way to run PHP versus a rock-stable mod_php:

"a composer module sets error_reporting(E_ALL) which breaks parts of our app due to E_NOTICE being thrown" is no compliement for your app - in two aspects - a) it should run clean with E_ALL and b) you must not spit out errors/warnings to the client

Will be there any answer from php kernel developers?

Hard to say exactly what changed between 5 and 7 to be responsible for this, but I think the change to bring back this behavior is straightforward: in error_reporting(), only do the change if p->modifiable allows for it. Or possibly switch to using zend_alter_ini_entry() instead.

this is the required patch, ive talked about it on irc with php devs a while ago,
a test for this should also be added to be sure does not happens again,
it's obviusly a bug, someone at some point copy pasted the some wrong code from the utility functions instead of calling it,
also there is some ppl that think the new backward incompatible and buggy behavior is right, it's funny
please someone show some love to this bug :(



@@ -673,38 +673,10 @@ ZEND_FUNCTION(error_reporting)
 	old_error_reporting = EG(error_reporting);
 	if (ZEND_NUM_ARGS() != 0) {
 		zend_string *new_val = zval_get_string(err);
-		do {
-			zend_ini_entry *p = EG(error_reporting_ini_entry);
-
-			if (!p) {
-				p = zend_hash_find_ptr(EG(ini_directives), ZSTR_KNOWN(ZEND_STR_ERROR_REPORTING));
-				if (p) {
-					EG(error_reporting_ini_entry) = p;
-				} else {
-					break;
-				}
-			}
-			if (!p->modified) {
-				if (!EG(modified_ini_directives)) {
-					ALLOC_HASHTABLE(EG(modified_ini_directives));
-					zend_hash_init(EG(modified_ini_directives), 8, NULL, NULL, 0);
-				}
-				if (EXPECTED(zend_hash_add_ptr(EG(modified_ini_directives), ZSTR_KNOWN(ZEND_STR_ERROR_REPORTING), p) != NULL)) {
-					p->orig_value = p->value;
-					p->orig_modifiable = p->modifiable;
-					p->modified = 1;
-				}
-			} else if (p->orig_value != p->value) {
-				zend_string_release(p->value);
-			}
-
-			p->value = new_val;
-			if (Z_TYPE_P(err) == IS_LONG) {
-				EG(error_reporting) = Z_LVAL_P(err);
-			} else {
-				EG(error_reporting) = atoi(ZSTR_VAL(p->value));
-			}
-		} while (0);
+		zend_string *ini_name;
+		ini_name = zend_string_init("error_reporting", sizeof("error_reporting") - 1, 0);
+		zend_alter_ini_entry(ini_name, new_val, ZEND_INI_USER, ZEND_INI_STAGE_RUNTIME);
+		zend_string_release(ini_name);
 	}
 
 	RETVAL_LONG(old_error_reporting);

error_reporting() was likely switched away from zend_alter_ini_entry() for performance reasons. In any case changing this would be easy enough, but like whoever you talked to on IRC I am not convinced that this is a bug. Preventing a change of error_reporting seems like a very questionable practice to me, and I'm not sure we want to support this.

ok this is a huge problem/change that complicate hosting of php sites a lot,
i can explain the details but i've already tried and no one cared,
at least list it on the backward incompatible list of things between php 5 and 7 
 AND in the doc, because the doc still explain the behavior of php5 :|

Please also note that using php_admin_value has only one meaning, preventing change to ini directive at user level, and also using php_admin_value is not mandatory, so i really can't see the questionable part of the discussion.

Sorry for being raw, but i'm in a bad mood, and i can't find better words, despite also not knowing english very well

there is nothing questionable - php_value versus php_admin_value and the same for 'flag' has a defined behavior no matter what value you want not get changed by a script, on my servers I would use it to disallow idiots lower the reporting level to force them write clean code running with E_ALL or go away

The thing is, error_reporting is more than just an INI value. It has a much larger impact on PHP at runtime than options like include_path do. And while include_path probably only needs to be set once or twice in code, during an initialization phase, it's perfectly reasonable to change the error_reporting value at any time in any number of circumstances.
If no changes to the error reporting level are allowed at all, what about the @ operator? Should that be blocked because it temporarily suppresses the level entirely?

Perhaps a php_admin_value for error_reporting should be treated as more than just an immutable number. Say, as a bitmask that is always ORed with a desired new value? That would guarantee errors of a certain type are always visible while allowing code and libraries to opt into other types. I think that would be a more common use case than needing to ensure a maximum level (ANDing) so code would only be able to turn off error types - which is probably needed for specific situations that could be addressed with careful use of @.

The problem I see is that error_reporting is a somewhat more fundamental configuration option with direct language integration. If you take your premise that php_admin_value should affect the error_reporting() function one step further, you could also argue that the @ operator should become ineffective if php_admin_value[error_reporting] is used (which I think would be quite the disaster...)

Instead of starting ethical discussions about how php error design works please keep in mind:

1) php has ever worked that way, no one had never had problems about it

2) between php 5 and 7 has been introduced a backward incompatible change, without documenting it

3) shops that do php hosting want to be like php itself "do the right thing" in a pragmatic way,
   if our clients/or use code from third parties that do messy things about error_reporting we want all will work anyway in most cases without touching 
   their code, for example wordpress and plugins
   we can't afford and it's a also bad for all if who hosts had review all the code of our clients/third parties plugins etc...while switching from php 5 to 7
   also keep in mind that, the already slow adoption of php 7, for hosting providers will slow more
   before php 7 we had the option to "php_admin_value[error_reporting] = E_ALL & ~E_NOTICE & ~E_DEPRECATED & ~E_WARNING & ~E_STRICT" and all was good in case of problems

4) if php want to introduce a new better way/design to handle errors a new separate mode can be added but should be optional until all php code/devs in the 
   word will know about that

5) we are not switching to php7 because of this

@ operator has nothing to do with a ini-value

hell what is the problem simply ignore ini_set() for values or flags set with php_admin_* which should have no other runtime impact or even offer optimizations because its known not to change

for anyone who thinks "php_admin_value[error_reporting]" should not be immutable: nobody is forced to use this. if you want to allow apps/users to change the error level, use "php_value[error_reporting]" and you're good.

whoever sets "php_admin_value[error_reporting]" certainly has her/his reasons - there *are* valid reasons for doing so, some outlined in this bug's comments.

Any news?

Up. Any progress?

Up. What with this problem?

I haven't been convinced by what has been said and it seems that @requinix concurs, so I'm switching this to a documentation issue. This backwards-incompatible change needs to be mentioned in the PHP 7.0 upgrading guide.

I can only say that is a shame that core php devs are totally unaware on how useful this feature is in general and how is used from an hosting provider point of view.

And we are not convinced about your decision on how good breaking backward compatibility is and also the other performance motivations behind this change.

I'm partially undecided but if the choice is between the old and new behavior then I'm in favor of the new. I understand people want fix the value but I still don't understand why; have one error_reporting value as a default and I see no reason why a user should not be able to change it in their code. Why does it matter? It's their site, their code, their error logs...
And no, that argument does not apply to every INI setting so there's no need to go there: error_reporting is an exception to the rule. I'd even say it's not the only one, like I could very easily argue that include_path should get special treatment too.

I'm not one of the core devs, I just hang around the bug tracker, but to me what PHP is doing now makes sense.

As far as this bug report is concerned, it would be NAB because the behavior is intentional, however the docs should mention it and don't now so instead this should be repurposed into a docs bug. Which is what nikic just did. Beyond that, changing the behavior to go back to the PHP 5 version is clearly controversial and so should be discussed in the appropriate place: the internals list and not here.

I'm partially undecided but if the choice is between the old and new behavior then I'm in favor of the new.
----------------------
It's a bad argument to me, using this setting with admin_value IS NOT MANDATORY FOR ANYONE, so backward compatibility is always preferable in such cases, strange no one understand this way of thinking. dunno i'm strange probably! but it feels is not the php way to make this breaking changes for no good reason!



I understand people want fix the value but I still don't understand why; have one error_reporting value as a default and I see no reason why a user should not be able to change it in their code. Why does it matter? It's their site, their code, their error logs...
----------------------
Exactly it's their code (or their third party code) and we as hosting providers need a way to make it work in some way even a bad one without having to touch all their code (or their third party code) and this method of fixing it had been working for years! it's obvious you never had to handle such problems, and it's the cause you don't know why this is useful, as you see if you are not open minded enought it's not possible to convince you about that, we have already given this answers about cases where this is useful!



And no, that argument does not apply to every INI setting so there's no need to go there: error_reporting is an exception to the rule. I'd even say it's not the only one, like I could very easily argue that include_path should get special treatment too.
I'm not one of the core devs, I just hang around the bug tracker, but to me what PHP is doing now makes sense.
As far as this bug report is concerned, it would be NAB because the behavior is intentional, however the docs should mention it and don't now so instead this should be repurposed into a docs bug. Which is what nikic just did. Beyond that, changing the behavior to go back to the PHP 5 version is clearly controversial and so should be discussed in the appropriate place: the internals list and not here.
----------------
i only see changing not mandatory options and proclaiming it as good pratice because are special as controversial!
if we have some free time we will try to escalate this issue to php core devs, for now we are forced to use a patched version of php seems, unfortunate.

If we want to talk about a clean and elegant way to fix this mess with error handling in php then i can propose an error_reporting(E_ALL){... php code here ...} block to make clear and obvious the scope of the error_reporting settings....but that's another matter...

I have a lot of websites and i maintain them.
And i want to have centralized point where i can see problems from all of my sites.
I want to run journalctl --follow --unit=php-fpm.service for tracking errors for all of my sites.
Not going to every site for modifying error_reporting or error_log (syslog) or something else.
Maybe this is not a best solution for developers, who wants to have a full control, but a best solution for hostings / sysops / etc.
But now we are dropped from a boat. That's very sad...

I amazed that the "new" behavior would be considered the desired one, as that makes absolutely no sense. there is *no other way* to enforce a consistent error logging when this backwards incompatible bug is kept. 

on the other way, nobody *was forced to use* "php_admin_value[error_reporting]": if you did not want an immutable error_reporting, use "php_value[error_reporting]" in your fpm config or "error_reporting" in php.ini or use the error_reporting function. "php_admin_value[error_reporting]" wasn't a default setting either, and those that used it, knew why and what it would mean.

this takes away a really useful feature from those that had their reasons to use it, without any gain to the php code in general. there has been no reason shown in this bug reports comment what the php world has gained by introducing the backward breaking change.

A few thoughts from the peanut gallery:

* What are the possible workarounds for this? Ideally ones that don't involve patching PHP.

For example, maybe adding "disable_functions = error_reporting" but that'd add a bunch of E_WARNING's. Not ideal, are there others?

* For clarity, does this PHP 7 change allow error_reporting(foo) but continue to disallow ini_set("error_reporting", foo) with php_admin_*? If so then that'd be weird and likely unintentional, right?

> For example, maybe adding "disable_functions = error_reporting" but that'd add a bunch of E_WARNING's. Not ideal, are there others?

Very bad idea. For example, Roundcube with "disable_functions = error_reporting" doesn't work...

> For clarity, does this PHP 7 change allow error_reporting(foo) but continue to disallow ini_set("error_reporting", foo) with php_admin_*? If so then that'd be weird and likely unintentional, right?

Code:
	echo "php_admin_value: " . ini_get("error_reporting") . "<br>";
	ini_set("error_reporting", 8);
	echo "ini_set: " . ini_get("error_reporting") . "<br>";
	error_reporting(8);
	echo "error_reporting: " . ini_get("error_reporting") . "<br>";

Return:
	php_admin_value: 2
	ini_set: 2
	error_reporting: 8

But there must be all "2".
PHP 7.2.6

This isn't a documentation bug as recently marked here. It is a big fat SECURITY BUG.
The ability for override whatever logging level with NONE has an immense security implication...
See my notes about it here: https://www.getpagespeed.com/server-setup/security/php-security-disable-error_reporting-now
The current wave of malware targeting PHP 7 effectively uses this bug to hide itself.

Based on @philip's request I'm taking another look at this. However, the resolution is unchanged.

> * What are the possible workarounds for this? Ideally ones that don't involve
> patching PHP.
> 
> For example, maybe adding "disable_functions = error_reporting" but that'd add > a bunch of E_WARNING's. Not ideal, are there others?

To clarify my position: This is exactly the intended outcome. In my opinion, and the opinion of all other developers I talked to regarding this issue, a hosting provider should not have the possibility to enforce an error_reporting level for the application.

From the comments in this bug report, it is apparent that this functionality has been used by certain hosting providers to disable error_reporting for notices and deprecation warnings. Historically, the willingness of developers to ignore such "low level" diagnostics, which nonetheless may indicate severe programming errors, has been an issue for the PHP programming language. Perpetuating this kind of development practice by not only disabling notices by default, but going so far as to prevent applications from explicitly re-enabling them, is damaging to the overall PHP community.

While we cannot prevent you from patching PHP, or from writing an extension that intercepts the error reporting mechanism, we can at least remove dedicated support for this functionality.

> This isn't a documentation bug as recently marked here. It is a big fat SECURITY BUG.
> The ability for override whatever logging level with NONE has an immense security implication...
> See my notes about it here: https://www.getpagespeed.com/server-setup/security/php-security-disable-error_reporting-now
> The current wave of malware targeting PHP 7 effectively uses this bug to hide itself.

It's nice that you have friendly malware that generates lots of errors that make it easy to detect, but that's certainly not a characteristic you can rely on. Malware could just as easily be completely error-free code, and if this is your only way of detecting it, I'm afraid to say that it is likely not particularly effective.

As this issue is clearly important to you, I would recommend to start a discussion about this on the PHP internals mailing list. Discussion in a broader context may yield a more favorable outcome.

1) php has ever worked that way, no one had never had problems about it
-- Remember "fractal of bad design"

5) we are not switching to php7 because of this

-- Yeah.. dont bother, your clients will find better hosting providers.

I can only say that is a shame that core php devs are totally unaware on how useful this feature is in general and how is used from an hosting provider point of view.

-- In another news, Apparently some core-developer thought "register_globals" as a useful function. Its *idiots* like you make a php a joke.

https://www.getpagespeed.com/server-setup/security/php-security-disable-error_reporting-now

-- Amazing. Like this article implies error_reporting is bad, lets disable functions to delete|update|rename|create a file|folder, as hey, it can users data (based on programmers logic)..