Restore original D6 behaviour to prevent logging E_STRICT warnings

I can confirm that patch #2 got rid of my PHP 5.4 errors on dblog for Drupal 6.28

Same here. I've tested this on both PHP 5.3 and 5.4 without any problems.

I tested this on PHP 5.4 and it works like a charm!

Partially worked for me (PHP 5.4.19, D6.22, Views 2.16, Date 2.7), stopping all the strict errors generated by Views. Would like to see this rolled into Drupal 6.

It didn't work for the date module, where I see:

warning: Creating default object from empty value in
/sites/all/modules/date/date/date.module on line 661.

This post may help solve it.

can we merge this in alraedy?

Patch in #2 helped for me as well!! Each time a client's site ends up on a php 5.4 server, which is only going to happen more often, this really scares clients who aren't familiar with why this would happen, makes them thing the sky is falling, and takes extra time off my plate explaining why the newer server php version 'broke' their website... while actually trying to explain that it didn't. : )

Thanks for the patch!

Applied patch #2, no longer have warnings related to strict errors generated by Views logged. Please note that this was extremely important to me because the "recent log entries" was absolutely worthless, being clogged with those php warnings.
Thank you.

Will a patch for 6.29 be created? Just upgraded and php error messages are back.

thank you

2: common.inc_6.28.patch queued for re-testing.

I confirm #11. The submitted patch worked for the 6.28 version of Core, but now, after the update from 6.28 to 6.29, it doesn't work anymore. Anyone knows why?

Thanks.

Attached is a re-roll of the patch for Drupal 6.29 but it will need review and testing as I am not using myself.

Rerolled patch for 6.30. Will not apply against current HEAD of 6.x as E_NOTICE has been removed from the line the patch replaces.

holist, your patch for common.inc_6.30 worked wonders. I inserted the few lines of code into my website and the offending "strict warning" message disappeared.

I tried other methods and spent hours trying to find a resolution to this problem (I'm not a coder). There must be a simpler way to solve this problem on a higher level.

Thank you - common.inc_6.30 made my life a whole lot less miserable. I wonder if I should be worried about any side effects.

@tanisharbaap
Zaiten and I developed that patch over a year ago and have patch well over 100 Drupal 6 and some Drupal 5 sites with it and we have not had any problems caused by using it. It only prevents strict warnings from being displayed and logged.

In the case anyone wonders, the patch in #16 applies on 6.31.

16: common.inc_6.30.patch queued for re-testing.

anyone have any luck installing this patch on d.31 ?

I get the following error message

patching file common.inc
Hunk #1 FAILED at 665.
1 out of 1 hunk FAILED -- saving rejects to file common.inc.rej

I reworked this patch but only for 6.x HEAD. Also, improved a bit to not repeat constants (DRY rule).

This is the previous patch reworked for 6.33 branch. It does not applies to 6.x HEAD.

Fixed version of 6.33 version of the patch. I made two typo errors.

Is this the only change you need to run Drupal 6 on PHP 5.4?

Instead of disabling the notices, we should actually resolve the coding problems that are generating them.

@JamesK
You are 100% correct and with Drupal 7 that is exactly what happened, but I think we need to be realistic here. This is Drupal 6 and I am very doubtful that contrib developers and core developers are going to be spending to much time make sure D6 works on the latest versions of PHP. Additional to this many people are still stuck on D6 and it works for them. Hopefully they will at some point when they can afford it, upgrade to D7, but until then I think this is a good practical middle ground.

Just my two cents.

Actually, there aren't any errors or warnings emitted by Drupal 6 core on PHP 5.4; at least, not the last time I checked. The issue is mostly with contrib, and Views in particular. Drupal 6 and Views 2 were both released when PHP 4 was current, and the maintainers refuse to break that compatibility. See #465332: Strict warning: non-static method view::load() should not be called statically for further info on that.

The issue here is simply that PHP changed how it reports errors. The patch in #25 passed tests, so this issue should probably be RTBC.

@ergonlogic: what tests, there are no tests on D6?!

Also Drupal 6 changed policy to not necessarily work with PHP 4 anymore. See https://www.drupal.org/node/2159315. That should allow contrib to fix things. Are there actual core issues?

Drupal 6 core throws a warning with PHP 5.4 for every time an object property is referenced without first declaring the variable as an object (for example, referencing $MyObject->prop without first doing $MyObject = new stdClass();) at the very least.

Another PHP 5.4 warning is solved by #1680614: Illegal string offset 'region' and 'status' when saving block admin

@Gábor Hojtsy: Well, at very least the patch seems to apply cleanly :)

@JamesK: The examples in #38 also throw warnings under PHP 5.3. While I think it's a great idea to clean up any such warnings in D6, that should really be a separate issue. Also, as @NaX pointed out, it's not likely to ever happen for much of D6 contrib, which is minimally maintained, at this point.

This issue is only about keeping error reporting consistent across PHP versions.

FWIW, Aegir has been shipping with some version of this patch for over a year, with good results.

I think a sensible approach on this would be to optionally suppress the E_STRICT warnings but display them by default.

Could be something like this:

<?php
if (!variable_get('e_strict_suppress') && version_compare(phpversion(), '5.4.0') > 0) {
  $error_const ^= E_STRICT;
}
?>

The PHP version comparison needs to check if the current version is 5.4.0 or higher. Thus

if (version_compare(phpversion(), '5.4.0') > 0)

must be changed to either

if (version_compare(phpversion(), '5.4.0') >= 0)

or better

if (version_compare(phpversion(), '5.4.0', '>='))

Would be great if we are allowed to set our own desired value of error_reporting, though.

I'm confused as to why we can't just roll out the previous made patch. It works great! To be honest I don't know anyone who wants a flood of error warnings in their database logs everytime cron runs. It's a nightmare to find anything useful for regular issues.

@ShaneOnABike: Patch from #25 contains the bug I've pointed out in #43.

There is no need to check the PHP version at all. The attached patch is a much easier solution.

@roball
I personally don't agree with this approach as it does not account for differences between versions. The issue write-up explains this very nicely.

With your one size fits all approach the error constant will have a different value for <= 5.3 compared >= 5.4 and this inconstancy I personally would like to avoid, so I know how a site will perform regardless of the server my client decided to put the site on, something I don't always have control of.

The last thing I want is my client calling me in a panic about warnings because he moved the site without checking with me first and ended up going back a PHP version. Clients don't always care about things like PHP versions they care about costs, so this can happen very easily.

I think your suggestion at #43 would make this patch complete, robust and consistent and ready to be committed.

I don't get your point. Drupal 6 requires at least PHP 5.0.0 and this is exactly the version since the E_STRICT constant is available and always defined. This constant always equals to binary 100000000000 which is decimal 2¹¹ = 2048, regardless of the PHP version. There are no "differences between versions."

@roball
The problem is not E_STRICT being different but E_ALL is different on different versions.

E_ALL = 32767 in PHP 5.4.x
E_ALL = 30719 in PHP 5.3.x

Now I am not good at bitwise operations, they still hurt my simple little brain, but my understanding is that your final value will be different on the different version because E_ALL is different.

So E_ALL & ~E_STRICT & ~E_DEPRECATED will be different on a PHP 5.4 vs 5.3 because the one version E_ALL has changed to include E_STRICT on versions that have not yet included E_STRICT (<= 5.3) if you had to remove E_STRICT you will get a different value.

To me your version is perfectly acceptable if we stop supporting all version before 5.4. Unless I am misunderstanding how bitwise operations work again.

E_ALL = 32767 in PHP 5.4.x
E_ALL = 30719 in PHP 5.3.x

Yes, that is the only defined error level constant that differs between 5.0 <= PHP <= 5.3 and PHP >= 5.4.

my understanding is that your final value will be different on the different version because E_ALL is different.

No, since I combine E_ALL and E_STRICT with a logical AND NOT instead of a XOR (as suggested in the original patch), the result is the same in both cases!

(32767 & ~2048) == 30719  // PHP >= 5.4
(30719 & ~2048) == 30719  // PHP < 5.4

However, my logic still had a bug for PHP versions < 5.3, where E_DEPRECATED is not defined. Revised patch is attached. The original patches also fail to work properly on PHP < 5.3, so the conditional version checking there only partly makes sense!

In my current patch, I am doing (E_ALL & ~E_STRICT ^ E_DEPRECATED), so the end result of this operation, depending on the PHP version, is as follows:

(30719 ^ 8192) == 22527  // PHP >= 5.3
(30719 ^ UNDEFINED) == 30719  // PHP < 5.3

Using E_DEPRECATED on PHP versions < 5.3 will of course trigger the E_NOTICE message Use of undefined constant E_DEPRECATED. This is a bug that is reported at #524664: E_DEPRECATED may be used even if it is not defined (affecting PHP versions < 5.3). The patch at https://www.drupal.org/node/524664#comment-9933000 (#16) fixes that bug.

The patch attached here also fixes that bug, and incorporates the feature request of this issue. It handles all cases for all PHP versions, without triggering any notice message.

@roball
Thanks for the detailed explanation, this latest version looks good to me. Do you think we should maybe include a comment as we know are accounting for two differences in error constants? Otherwise I think this patch is RTBC.

OK, I have re-posted the patch of the parent issue with comments added at #524664: E_DEPRECATED may be used even if it is not defined (affecting PHP versions < 5.3), comment #17. If you review that patch, I will post the corresponding one here.

New patch attached that is based on https://www.drupal.org/node/524664#comment-9936490 including code comments.

Turning off error messages is not any kind of fix. This approach is a non-starter.

@SeanA, nobody recommended turning off error messages! Drupal 6 had never turned on E_STRICT warnings. PHP 5.4 added E_STRICT to E_ALL, thus these warnings have been implicitly turned on when upgrading PHP, resulting in suddenly having the dblog full with thousands of such warnings. My patch just gives Drupal 6 back the original and intended behaviour. Of course it would be optimal if all modules would have been written cleanly to not trigger PHP warnings, but unfortunately - that is not the case, including D6 core modules.

In Drupal 7, E_STRICT have been turned on from the beginning, thus the code of most D7 modules have been written more cleanly than those for D6. I do not argue to turn off existing warnings or even error messages!

I don't think this approach has any chance of going anywhere. However, there is a good chance we can fix the problematic contrib modules if we keep the momentum going on those issues, mainly with Views and CCK. The maintainers have not said that they refuse to fix them. Since a lot of sites are still using D6, and probably will be for some time to come, it would be a good idea to get them patched up properly.
#2411093: PHP strict warnings: Declaration of ... should be compatible with ...
#2327005: E_STRICT compliance

I agree with roball, this restores the original way it worked and is the best solution with the lest amount of effort, whether it will ever be committed is another story.

Changed this issue's title to include the intention of the patch. It is not a new approach, instead it's the opposite - the new behaviour that was silently introduced should be restored back to the original one. There is not enough time to fix all affected modules until EOL of D6, given that the first RC of D8 has already been released, so at least this single patch could make the situation better.

Roball's approach is much more practical. D6 modules are unlikely to be fixed at this point. Requiring that contrib modules be 'fixed' to conform to new PHP requirements strikes me as more of a non-starter than turning off the logging of these warnings.

Shooting down this patch is basically thumbing the nose at people still stuck on D6 -- and having been in the role of someone supporting multiple clients on D6, I can tell you they are mostly not there because they want to be, so it does nothing but create ill will. As roball notes, these are warnings, not errors -- the intended purpose of them is to serve as a heads-up that the feature's been deprecated. It's a warning that you'll be in a bad position in the future. They do not indicate vulns nor do they represent failures of the code to function as-designed.

The problematic code should be fixed, yes, for the sake of D6 users. I still support clients on D6 as well, else why would I even care? I listed 2 issues where it's currently being worked on and suggest that efforts be focused there.

PHP strict error reporting isn't the problem, Drupal 6 core isn't the problem, mainly Views is the problem. The fixes are simple, and surely it's not a bad idea to follow best practices for writing PHP code by declaring things properly?

This issue is over 2 years old now, if the D6 modules have not been fixed and patched by now they probably never will be. There is very little development effort behind Drupal 6 maintenance at the moment. Doing things correct is always first prize however it needs to be practical taking all affecting factors into account and I don't think its practical to try and fix and patch all contrib modules when they have not been fixed in the years since the start of the problem.

This patch restores core error reporting back to what it was when first released and is the most practical solution to the problem taking all factors into account including the release of D8 and possible approaching EOL.

My only suggestion if people have a problem with this patch is to add a setting to the error reporting page the allows users to change the error reporting back to display strict warnings, but the default is to suppress. Maybe this would be a good compromise.

+1 for patch #57 to be committed to D6.
After all it is only a warning and advancing to php 5.4 or even 5.5 is important even for D6 sites.

Whatever happens the patch is here for the applying.

Efforts to fix code giving rise to these is not really worth the effort at this time.

Sure people can learn from finding and examining the code and that's great but it's old code.

Issue #2454439: [META] Support PHP 7

and one of the links, http://php.net/supported-versions.php

shows what's really important now.

Thanks so much to every one who has contributed their comments and patches to benefit us all.

I missed the suggestion by @NaX

My only suggestion if people have a problem with this patch is to add a setting to the error reporting page the allows users to change the error reporting back to display strict warnings, but the default is to suppress.

Sure this would be good.

Hello
Really need help here... I switched to php 5.5 to support another site running D8 and getting these errors (see below)..
I have these issues and trying to follow up with these comments but must admit I don't really understand.
Can anyone help by explaining in simple terms what I need to do to get rid of such errors?
Running drupal 6.36..

/home/trainef1/public_html/sites/all/modules/views/handlers/views_handler_field.inc on line 1148.
strict warning: Declaration of content_handler_field::element_type() should be compatible with views_handler_field::element_type($none_supported = false, $default_empty = false, $inline = false) in /home/trainef1/public_html/sites/all/modules/cck/includes/views/handlers/content_handler_field.inc on line 229.
strict warning: Declaration of content_handler_field_multiple::pre_render() should be compatible with views_handler_field::pre_render(&$values) in /home/trainef1/public_html/sites/all/modules/cck/includes/views/handlers/content_handler_field_multiple.inc on line 322.
strict warning: Declaration of views_handler_sort::options_validate() should be compatible with views_handler::options_validate($form, &$form_state) in /home/trainef1/public_html/sites/all/modules/views/handlers/views_handler_sort.inc on line 165.
strict warning: Declaration of views_handler_sort::options_submit() should be compatible with views_handler::options_submit($form, &$form_state) in /home/trainef1/public_html/sites/all/modules/views/handlers/views_handler_sort.inc on line 165.
strict warning: Declaration of views_handler_sort::query() should be compatible with views_handler::query($group_by = false) in /home/trainef1/public_html/sites/all/modules/views/handlers/views_handler_sort.inc on line 165.
strict warning: Declaration of views_handler_filter::query() should be compatible with views_handler::query($group_by = false) in /home/trainef1/public_html/sites/all/modules/views/handlers/views_handler_filter.inc on line 599.
strict warning: Declaration of views_plugin_query::options_submit() should be compatible with views_plugin::options_submit($form, &$form_state) in /home/trainef1/public_html/sites/all/modules/views/plugins/views_plugin_query.inc on line 181.
strict warning: Declaration of views_plugin_row::options_submit() should be compatible with views_plugin::options_submit($form, &$form_state) in /home/trainef1/public_html/sites/all/modules/views/plugins/views_plugin_row.inc on line 136.
No main frame module is enabled for views slideshow.
strict warning: Only variables should be passed by reference in /home/trainef1/public_html/sites/all/modules/captcha/captcha.inc on line 61.
strict warning: Declaration of views_plugin_pager_none::post_execute() should be compatible with views_plugin_pager::post_execute(&$result) in /home/trainef1/public_html/sites/all/modules/views/plugins/views_plugin_pager_none.inc on line 69.
No main frame module is enabled for views slideshow.

@zisser
Try applying the patch from #57.

Just for information. If you use any released version of Drupal 6, the patch of #57 will not apply, because it is targeted to HEAD (which is correct, of course). If you don't want to upgrade to the newest DEV-Version of Drupal just for applying the patch you could modify one line of it. Instead of

- if ($errno & (E_ALL ^ E_DEPRECATED)) {

write

- if ($errno & (E_ALL ^ E_DEPRECATED ^ E_NOTICE)) {

But, you may want to use the latest DEV of Drupal 6 anyway, because of its other improvements and patches. Then you can use the original patch #57.

The patch at #57 is targeted against the 6.x branch. To get the functionality in, the easiest would be if someone commits and pushes the patch to 6.x!

I second committing the patch especially as D6 will be reaching an EOF soon. One last release including this patch will make a lot peoples lives a lot easier.

I have found an easy solution which works great:
I installed the module "Disable strict warnings".

Attached is an updated patch that works both against the 6.x branch, and with the 6.38 release.

Even though this patch still applies to D6.46 LTS it is not needed to suppress the notices. Much has been done and accomplished to address the notices and even move D6 forward to being php 7.2 compatible for those sites needing it. Thanks to the drupal community.