jedchecker icon indicating copy to clipboard operation
jedchecker copied to clipboard

Discussion: error_reporting(0) rule

Open dryabov opened this issue 4 years ago • 1 comments

Currently this rule detects just an error_reporting(0) code only (note: error_reporting(1-1) or error_reporting( 0 ) are passed). I suggest to revise this rule to detect usage of any function that may affect PHP code execution (independently of arguments passed):

assert_options
error_reporting
gc_disable
gc_enable
ini_set (ini_alter)
putenv

I'm not sure about:

  • set_time_limit: it is frequently used in the case of heavy server-side processing, I'd keep it as allowed one,
  • set_include_path: may be used to run some legacy libraries,

There may be rarely cases where ini_set is necessary, so it may be allowed (if followed by restoring settings back before script returns to Joomla).

What do you think?

dryabov avatar Mar 23 '21 11:03 dryabov

error_reporting is the most common way that a developer can behave in an uncivil manner and affect the rest of the installed extensions, overriding the Error Reporting managed by the Joomla Core.

Generally, when a developer wants to hide notices and warnings, the extension includes error_reporting(0).

There are valid error_reporting cases. For instance, when the extension implements an API, and other extensions generate warning/notices.

It is difficult to decide what to do with error_reporting. As you describe @dryabov, there are also other ways to alter PHP behaviour; however, error_reporting(0) is the way that most junior/mid-level developers use.

I think that we must keep the rule, but improve it to detect any usage of error_reporting for manual review. The other cases are rare.

anibalsanchez avatar Mar 23 '21 11:03 anibalsanchez