@sgiehl opened this Pull Request on October 17th 2022 Member

Description:

On PHP 8, the settings if log and report data should be deleted might not be respected correctly.

The reason for this is, that the API converts the setting to a boolean e.g. here:
https://github.com/matomo-org/matomo/blob/6cc723685d3c1c9b399b09e9b691b9f2dba80bce/plugins/PrivacyManager/API.php#L295

But as the value is then stored in an option in the database, it gets converted to a string again.
So the resulting database value is either 1 (string value of true) or an empty string (string value of false).

When the value is later checked in the task this is done this way:
https://github.com/matomo-org/matomo/blob/2ea1411560d1942057fb97c94e334bfbd797a817/plugins/PrivacyManager/PrivacyManager.php#L523

That used to work smoothly for PHP 7. But for PHP 8 the result of that compare changed. The result of comparing an empty string against 0 changed. See https://3v4l.org/v4p2U

That means that removing log and report data is actually always enabled on PHP 8 once the settings in the UI had been saved.

Review

@tsteur commented on October 17th 2022 Member

@sgiehl great find! As it's a critical regression can we add a test for this?

@sgiehl commented on October 18th 2022 Member
This Pull Request was closed on October 17th 2022
Powered by GitHub Issue Mirror