Permission check for 'Not strict check' is wrong AFAIK

[sunnysideup]

Module version(s) affected

5.x-dev

Description

Have a look here:

https://github.com/silverstripe/silverstripe-framework/blob/5/src/Security/Permission.php#L152-L153

And compare to: https://github.com/silverstripe/silverstripe-framework/blob/5/src/Security/Permission.php#L259-L275

What I am seeing is that the comment is right, but the code is wrong. It should return TRUE if the permission code is not found at all.

How to reproduce

I am just reading the code here. It says "returns TRUE", but the actual return statement is FALSE

Possible Solution

Since this has never been picked up, I would recommend removing the strict at all.

Additional Context

No response

Validations

• [X] Check that there isn't already an issue that reports the same bug
• [X] Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

[michalkleiner]

It's definitely the safer way how it currently is though I agree the code may not match the comment/the intended use.

[GuySartorelli]

Please avoid using images when referencing code - just the link to the code is sufficient, or use codeblocks if you really want to include the code in the issue itself.

[GuySartorelli]

Looks like in this commit (back in the SilverStripe 2 days) a refactor accidentally missed out the true from the return statement, and then later in this commit (very early Silverstripe 4) another refactor added false which probably seemed like the correct value to return without checking the PHPDoc.

Given it's been wrong since Silverstripe 2 I'm inclined to agree that the strict check should just be removed. It can be removed in 5.x without any BC changes since it ultimately returns false either way once you get to that part of the code. The parameter itself should be deprecated, to be removed in 6.

[sunnysideup]

will you make the changes?

[GuySartorelli]

There are over 2000 issues across the supported modules, and three total people on the CMS Squad. I can't commit to saying I or anyone on the CMS Squad will make changes for any given issue unless it has a high or critical impact.