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

Test calling and disallowing a function when its disallowedParam is array|string|null #165

Merged
merged 1 commit into from Mar 19, 2023

Conversation

mad-briller
Copy link
Contributor

When trying to disallow the usage of a particular configuration key in Laravel, this false positive was discovered.

the Laravel config() helper can take a string or array as it's first argument, strings are used to read and arrays are used to update the configuration.

when a disallowParams was set to disallow the use of a constant string as the first argument, calls that provided an array were falsey flagged as disallowed.

this rectifies that issue, however i'm not convinced this is the best way to go about this due to unfamiliarity with this codebase, but it resolves the cases i had to hand, so feel free to let me know if there's a better way

thanks for your time

@mad-briller mad-briller marked this pull request as draft January 20, 2023 12:11
@mad-briller mad-briller marked this pull request as ready for review January 20, 2023 12:13
src/Params/DisallowedCallParamValueExcept.php Outdated Show resolved Hide resolved
tests/libs/Functions.php Outdated Show resolved Hide resolved
tests/src/disallowed/functionCalls.php Outdated Show resolved Hide resolved
tests/src/disallowed/functionCalls.php Show resolved Hide resolved
@spaze
Copy link
Owner

spaze commented Jan 20, 2023

Hi, thanks for the PR. This is unfortunately a bit more difficult to handle as some users may disallow params with arrays and this change would miss such params.

@spaze
Copy link
Owner

spaze commented Mar 19, 2023

I believe the changes in 2.12.0 (#174 or even earlier #167) has also resolved this issue so there's no need to short-circuit array type in DisallowedCallParamValueExcept. Can you please check?

I'll then add the tests to make sure it's doing what it should and would prohibit arrays in config.

spaze added a commit that referenced this pull request Mar 19, 2023
spaze added a commit that referenced this pull request Mar 19, 2023
spaze added a commit that referenced this pull request Mar 19, 2023
spaze added a commit that referenced this pull request Mar 19, 2023
@spaze spaze changed the title Fixed issue where calling a function with an array was always considered disallowed when using disallowParams Test calling and disallowing a function when its disallowedParam is array|string|null Mar 19, 2023
@spaze spaze merged commit cbdf5c9 into spaze:main Mar 19, 2023
spaze added a commit that referenced this pull request Mar 19, 2023
spaze added a commit that referenced this pull request Mar 19, 2023
A non-scalar param config value would throw `ShouldNotHappenException`. This may change in the future.

Ref #165
@mad-briller
Copy link
Contributor Author

this has fixed it! thanks for getting it over the line and sorry i couldn't take it further, i had a few tries but couldn't make much progress and ran out of time

@spaze
Copy link
Owner

spaze commented Mar 20, 2023

Thanks and no problem! :-) I'll publish a release soon.

@spaze
Copy link
Owner

spaze commented Mar 20, 2023

Released in 2.12.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants