Bug #88444: TYPO3 error handler ignores error_reporting level of php.ini - TYPO3 Core - TYPO3 Forge

Actions

Copy link

Bug #88444

closed

TYPO3 error handler ignores error_reporting level of php.ini

Added by Stephan Großberndt almost 5 years ago. Updated almost 4 years ago.

Status:

Closed

Priority:

Should have

Assignee:

Category:

System/Bootstrap/Configuration

Target version:

Start date:

2019-05-27

Due date:

% Done:

Estimated time:

TYPO3 Version:

PHP Version:

Tags:

Complexity:

Is Regression:

Sprint Focus:

Description

TYPO3 uses the PHP error handling function for deprecation messages since TYPO3 v9.0, but defining a error_reporting level in php.ini like

error_reporting = E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED & ~E_STRICT

in order to ignore deprecation messages on a production server has no effect.

The deprecations are logged by calling trigger_error($message, E_USER_DEPRECATED) which leads to the php error handler being executed even if the currently configured error_reporting() would ignore E_USER_DEPRECATED messages.

The ErrorHandler only uses the value from [SYS][errorHandlerErrors] which does not offer an option to use the server defined error_reporting level of php.ini:

https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.7/typo3/sysext/core/Classes/Error/ErrorHandler.php#L66

    /**
     * Registers this class as default error handler
     *
     * @param int $errorHandlerErrors The integer representing the E_* error level which should be
     */
    public function __construct($errorHandlerErrors)
    {
        $excludedErrors = E_COMPILE_WARNING | E_COMPILE_ERROR | E_CORE_WARNING | E_CORE_ERROR | E_PARSE | E_ERROR;
        // reduces error types to those a custom error handler can process
        $this->errorHandlerErrors = $errorHandlerErrors & ~$excludedErrors;
        set_error_handler([$this, 'handleError'], $this->errorHandlerErrors);
    }

https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.7/typo3/sysext/core/Classes/Core/Bootstrap.php#L669

        $errorHandlerErrors = $GLOBALS['TYPO3_CONF_VARS']['SYS']['errorHandlerErrors'];
...

            $errorHandler = GeneralUtility::makeInstance($errorHandlerClassName, $errorHandlerErrors);

Using the server defined error_reporting level of php.ini could be achieved by e.g. allowing a value like server_error_reporting for ['SYS']['errorHandlerErrors'] which would lead to the value of error_reporting() being used in

https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.7/typo3/sysext/core/Classes/Core/Bootstrap.php#L701

            $errorHandler = GeneralUtility::makeInstance($errorHandlerClassName, $errorHandlerErrors);

Related issues 1 (0 open — 1 closed)

Actions

Copy link

Updated by Stephan Großberndt almost 5 years ago

Subject changed from TYPO3 bootstrap resets error_reporting level of php.ini to TYPO3 error handler ignores error_reporting level of php.ini
Description updated (diff)

Updated issue to reflect findings that the problem stems from ErrorHandler itself instead of SystemEnvironmentBuilder

Actions

Copy link

Updated by Stephan Großberndt almost 5 years ago

There is a drawback to the proposed solution to disable deprecation logs on production, which needs to be considered:

TYPO3 sets the error_reporting itself in a hardcoded way:

https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.7/typo3/sysext/core/Classes/Core/SystemEnvironmentBuilder.php#L328

    /**
     * Initialize basic error reporting.
     *
     * There are a lot of extensions that have no strict / notice / deprecated free
     * ext_localconf or ext_tables. Since the final error reporting must be set up
     * after those extension files are read, a default configuration is needed to
     * suppress error reporting meanwhile during further bootstrap.
     */
    protected static function initializeBasicErrorReporting()
    {
        // Core should be notice free at least until this point ...
        error_reporting(E_ALL & ~(E_STRICT | E_NOTICE | E_DEPRECATED));
    }

which does not exclude E_USER_DEPRECATED.

Even if ['SYS']['errorHandlerErrors'] is configured not to contain E_USER_DEPRECATED - SystemEnvironmentBuilder will configure error_reporting for E_USER_DEPRECATED to be done: this leads to the default error handler of PHP being used. If you call TYPO3 CLI via typo3/sysext/core/bin/typo3 the deprecations of TYPO3 will be reported to stdout prefixed with PHP Deprecated:

user@host:~# ./typo3/sysext/core/bin/typo3
PHP Deprecated:  The old condition syntax will be removed in TYPO3 v10.0, use the new expression language. Used condition: [userFunc = TYPO3\CMS\Core\Utility\ExtensionManagementUtility::isLoaded('form')]. in typo3_src-9.5.7/typo3/sysext/backend/Classes/Configuration/TypoScript/ConditionMatching/ConditionMatcher.php on line 80

Actions

Copy link

Updated by Josua Vogel almost 5 years ago

Stephan Großberndt wrote:

There is a drawback to the proposed solution to disable deprecation logs on production, which needs to be considered:

TYPO3 sets the error_reporting itself in a hardcoded way:

https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.7/typo3/sysext/core/Classes/Core/SystemEnvironmentBuilder.php#L328

[...]

which does not exclude E_USER_DEPRECATED.

Even if ['SYS']['errorHandlerErrors'] is configured not to contain E_USER_DEPRECATED - SystemEnvironmentBuilder will configure error_reporting for E_USER_DEPRECATED to be done: this leads to the default error handler of PHP being used. If you call TYPO3 CLI via typo3/sysext/core/bin/typo3 the deprecations of TYPO3 will be reported to stdout prefixed with PHP Deprecated:

[...]

Well, I noticed that my TYPO3 reports deprecations of the Core (e.g. Deprecatedin typo3/sysext/extbase/Classes/Core/Bootstrap.php on line 127) and no configuration is able to prevent that. The PHP error.log is getting bigger and bigger. If I add E_USER_DEPRECATED inside the SystemEnvironmentBuilder, no deprecation message is logged. I wonder, if this would be the right way.

Actions

Copy link

Updated by Benni Mack almost 5 years ago

I don't see a point of having the basic error reporting disabled in SystemEnvironmentBuilder anymore since TYPO3 v9 got some good refactorings about error handling.

In addition, I propose to add E_USER_DEPRECATED for the "Production" settings within Install Tool for errorHandlerErrors.

Actions

Copy link

Updated by Stephan Großberndt almost 5 years ago

Thanks Benni.

And as a second addition: the core should not contain any code itself leading to deprecation messages. @Josua: Could you please create issues at https://forge.typo3.org/projects/typo3cms-core/issues for the deprecation messages of the core?

Actions

Copy link

Updated by Gerrit Code Review over 4 years ago

Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62993

Actions

Copy link

Updated by Gerrit Code Review over 4 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62993

Actions

Copy link

Updated by Gerrit Code Review over 4 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62993

Actions

Copy link

Updated by Benni Mack over 4 years ago

Related to Bug #89934: E_UER_DEPRECATED errors should be handled by the basic error reporting added

Actions

Copy link

#10

Updated by Helmut Hummel almost 4 years ago

Even if ['SYS']['errorHandlerErrors'] is configured not to contain E_USER_DEPRECATED

By all means for all systems and environments errorHandlerErrors MUST be configured to contain E_USER_DEPRECATED
Otherwise our depreaction logging strategy fails.

I propose to add E_USER_DEPRECATED for the "Production" settings within Install Tool for errorHandlerErrors.

E_USER_DEPRECATED must be enforced for production AND development settings

Actions

Copy link

#11

Updated by Gerrit Code Review almost 4 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#12

Updated by Helmut Hummel almost 4 years ago

Related to deleted (Bug #89934: E_UER_DEPRECATED errors should be handled by the basic error reporting)

Actions

Copy link

#13

Updated by Helmut Hummel almost 4 years ago

Is duplicate of Bug #89934: E_UER_DEPRECATED errors should be handled by the basic error reporting added

Actions

Copy link

#14

Updated by Helmut Hummel almost 4 years ago

I'm closing this as duplicate

Actions

Copy link

#15

Updated by Helmut Hummel almost 4 years ago

Status changed from Under Review to Closed

Actions

Copy link

#16

Updated by Gerrit Code Review almost 4 years ago

Status changed from Closed to Under Review

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#17

Updated by Gerrit Code Review almost 4 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#18

Updated by Gerrit Code Review almost 4 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#19

Updated by Gerrit Code Review almost 4 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#20

Updated by Gerrit Code Review almost 4 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#21

Updated by Gerrit Code Review almost 4 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/56077

Actions

Copy link

#22

Updated by Gerrit Code Review almost 4 years ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64400

Actions

Copy link

#23

Updated by Helmut Hummel almost 4 years ago

Status changed from Under Review to Resolved

Applied in changeset b0080cba90f2b3634ace7de0cc3f3a1d307709c9.

Actions

Copy link

#24

Updated by Benni Mack almost 4 years ago

Status changed from Resolved to Closed

Actions

Copy link

Also available in: Atom PDF

Project

General

Profile

TYPO3 Core

Custom queries

Watchers (2)

Bug #88444

TYPO3 error handler ignores error_reporting level of php.ini

Updated by Stephan Großberndt almost 5 years ago

Updated by Stephan Großberndt almost 5 years ago

Updated by Josua Vogel almost 5 years ago

Updated by Benni Mack almost 5 years ago

Updated by Stephan Großberndt almost 5 years ago

Updated by Gerrit Code Review over 4 years ago

Updated by Gerrit Code Review over 4 years ago

Updated by Gerrit Code Review over 4 years ago

Updated by Benni Mack over 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Gerrit Code Review almost 4 years ago

Updated by Helmut Hummel almost 4 years ago

Updated by Benni Mack almost 4 years ago