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

Convert variable type to int if it was an integer fixes #5688 #5691

Closed
wants to merge 2 commits into from

Conversation

rfikree92
Copy link

@rfikree92 rfikree92 commented Feb 13, 2022

Fixes #5688

Description
convert any variable that is a number to int type, so errors like #5688 doesn't happen.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline comment. Also we would need a test to cover this - should be a straightforward adjustment of current tests.

system/Config/BaseConfig.php Outdated Show resolved Hide resolved
@kenjis
Copy link
Member

kenjis commented Feb 13, 2022

I think the following test cases are needed:

'02471'
'1337e0'
" 42"
"42 "

https://www.php.net/manual/en/function.is-numeric.php#refsect1-function.is-numeric-examples

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 13, 2022
@paulbalandan paulbalandan added the tests needed Pull requests that need tests label Feb 15, 2022
@kenjis
Copy link
Member

kenjis commented Apr 12, 2022

#5779 was merged.

@kenjis kenjis closed this Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them tests needed Pull requests that need tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Ajax request makes the session expire when app.sessionExpiration is set to 0
4 participants