Bug #88444

TYPO3 error handler ignores error_reporting level of php.ini

Added by Stephan Großberndt over 1 year ago. Updated 7 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
System/Bootstrap/Configuration
Target version:
-
Start date:
2019-05-27
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
9
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

Is duplicate of TYPO3 Core - Bug #89934: E_UER_DEPRECATED errors should be handled by the basic error reportingClosed2019-12-13

Actions
#1

Updated by Stephan Großberndt over 1 year 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

#2

Updated by Stephan Großberndt over 1 year 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
#3

Updated by Josua Vogel over 1 year 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.

#4

Updated by Benni Mack over 1 year 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.

#5

Updated by Stephan Großberndt over 1 year 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?

#6

Updated by Gerrit Code Review 10 months 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

#7

Updated by Gerrit Code Review 10 months 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

#8

Updated by Gerrit Code Review 10 months 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

#9

Updated by Benni Mack 10 months ago

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

Updated by Helmut Hummel 7 months 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

#11

Updated by Gerrit Code Review 7 months 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

#12

Updated by Helmut Hummel 7 months ago

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

Updated by Helmut Hummel 7 months ago

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

Updated by Helmut Hummel 7 months ago

I'm closing this as duplicate

#15

Updated by Helmut Hummel 7 months ago

  • Status changed from Under Review to Closed
#16

Updated by Gerrit Code Review 7 months 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

#17

Updated by Gerrit Code Review 7 months 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

#18

Updated by Gerrit Code Review 7 months 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

#19

Updated by Gerrit Code Review 7 months 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

#20

Updated by Gerrit Code Review 7 months 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

#21

Updated by Gerrit Code Review 7 months 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

#22

Updated by Gerrit Code Review 7 months 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

#23

Updated by Helmut Hummel 7 months ago

  • Status changed from Under Review to Resolved
#24

Updated by Benni Mack 7 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF