Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-13306

Add error level to the error collector

    Details

    • Type: Improvement
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1.1
    • Fix Version/s: 3.1.3-RC1
    • Component/s: None
    • Labels:
      None

      Description

      The error_collector class is installed in this method:
      https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/phpbb/error_collector.php#L27

      However, set_error_handler does not have a second argument to accept an error level. By default this means it will process all errors, ignoring the error_reporting() level set either by phpbb itself or by the user. In particular this is problematic if the user is attempting to suppress E_STRICT errors that any arbitrary function called by a process which invokes the error_collector (such as the custom mailer function) could cause, leading to spam in the error log.

      I would suggest either adding a second argument like "E_ALL & ~E_STRICT" or some other approach to allow the user to determine what level of errors should be processed.

      ------
      [EDIT] for posterity...

      The patch for this issue creates a constructor parameter for the error_collector class which accepts a mask of PHP error types that the collector should keep. If set to null (default) then the collector will use the value of error_reporting() at the time of the install method being called instead.

      If future core files are changed to use this parameter, the value should be a config setting, not a hard-coded value, otherwise the benefit of this patch will be lost.

        Activity

        Hide
        bantu Andreas Fischer added a comment -

        Capturing all errors is the intended behaviour for class error_collector. You seem to be confusing class error_collector and function msg_handler.

        Show
        bantu Andreas Fischer added a comment - Capturing all errors is the intended behaviour for class error_collector. You seem to be confusing class error_collector and function msg_handler.
        Hide
        omniError omniError added a comment - - edited

        Let's look at the example I referred to, the custom mail function:
        https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/includes/functions_messenger.php#L1706

        collector operates under assumed E_ALL and collects everything ($err_msg), which goes back to..
        https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/includes/functions_messenger.php#L537

        and the error method in this class sends everything to the error log without regard to the error_reporting() level. So it's not possible in this sequence to suppress E_STRICT.

        Show
        omniError omniError added a comment - - edited Let's look at the example I referred to, the custom mail function: https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/includes/functions_messenger.php#L1706 collector operates under assumed E_ALL and collects everything ($err_msg), which goes back to.. https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/includes/functions_messenger.php#L537 and the error method in this class sends everything to the error log without regard to the error_reporting() level. So it's not possible in this sequence to suppress E_STRICT.
        Hide
        bantu Andreas Fischer added a comment -

        Thanks for making that clear.

        Show
        bantu Andreas Fischer added a comment - Thanks for making that clear.
        Hide
        omniError omniError added a comment -

        simplest solution, set the error level to the existing error_reporting() level, then the user can change it if needed

        Show
        omniError omniError added a comment - simplest solution, set the error level to the existing error_reporting() level, then the user can change it if needed

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            omniError omniError
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development