Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed incorrect comparison in ACL module skipping logic #472

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

tipue-dev
Copy link
Contributor

@tipue-dev tipue-dev commented Aug 1, 2023

Proposed change

The original code snippet was intended to skip over a module if its ReturnSubType did not match the required ReturnSubType. Due to an error in the condition logic, the module skipping function does not operating as expected.

next MODULENAME if !$Module->{ReturnSubType} eq $Param{ReturnSubType};

!$Module->{ReturnSubType} results in an empty string, that is then checked for equality with $Param{ReturnSubType}.
This code runs only if $Param{ReturnSubType} is given. The result is the same as if a ReturnSubType would not be checked at all and the code runs for every sub type.

Update:
Additionally the ReturnSubType can also given as an array of possible sub types, but the condition checks for a hash reference. If it was intended, that the subtype could be given as hash, then the code inside the condition is wrong.
Either way, currently none of the restrictions are working.

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy passed.(❕)
  • Local UnitTests / Selenium passed.(❕)
  • GitHub workflow CI (UnitTests / Selenium) passed.(❗)

@tipue-dev tipue-dev changed the title Fixed incorrect string comparison in ACL module skipping logic Fixed incorrect comparison in ACL module skipping logic Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants