Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: error_reporting(0) rule #98

Open
dryabov opened this issue Mar 23, 2021 · 1 comment
Open

Discussion: error_reporting(0) rule #98

dryabov opened this issue Mar 23, 2021 · 1 comment

Comments

@dryabov
Copy link
Collaborator

dryabov commented Mar 23, 2021

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?

@anibalsanchez
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants