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

Prevent error on blank WordPress install caused by a string instead of an integer. Thanks to GitHub user @federicojacobi for the pull request. #533

Merged
merged 1 commit into from
May 18, 2024

Conversation

federicojacobi
Copy link

What does this Pull Request do?

In this particular case, $schedule_number must be a number because in line 393 it is being multiplied. I'm not sure if it is a PHP8 thing, but you cannot multiple an int ($seconds) by a string (empty or otherwise). $schedule_number must be a positive int, just casting isn't enough, absint should cover it.

How do I test this Pull Request?

I only noticed this on a blank WP install, if you turn logging on it'll throw a fatal as no schedules have ever been set.

@jonathanstegall jonathanstegall changed the title Prevent fatal Prevent error on blank WordPress install caused by a string instead of an integer May 17, 2024
@jonathanstegall jonathanstegall added the bug fix Pull request that fixes a bug label May 17, 2024
@jonathanstegall
Copy link
Member

@federicojacobi thanks for the pull request. I'm happy to merge it when I get a bit of time to look over it (nobody is currently working on the plugin, but I try to keep an eye on it when I can).

If I'm reading it correctly, it should be possible to use $schedule_number = filter_var( get_option( $this->option_prefix . 'logs_how_often_number', '' ), FILTER_VALIDATE_INT ); instead of absint, right? I'm asking just because that is much more commonly what this codebase does, so it makes sense to me to try to keep it consistent if possible.

@federicojacobi
Copy link
Author

@jonathanstegall yes, it would almost be the same thing. filter_var in your example would allow negative numbers where absint wouldn't, but it would be difficult to get that option to be negative to begin with. So yeah, it is basically the same thing.

Personally, I find absint more readable, but if you are striving for consistency ... 👍🏽

@jonathanstegall jonathanstegall changed the title Prevent error on blank WordPress install caused by a string instead of an integer Prevent error on blank WordPress install caused by a string instead of an integer. Thanks to GitHub user @federicojacobi for the pull request. May 18, 2024
@jonathanstegall jonathanstegall merged commit f113ea5 into MinnPost:master May 18, 2024
2 of 3 checks passed
@jonathanstegall jonathanstegall added the patch pull request that requires a patch release, ex v2.1.2. This is the default for new releases. label May 18, 2024
@jonathanstegall jonathanstegall added this to the v2.2.9 milestone May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Pull request that fixes a bug patch pull request that requires a patch release, ex v2.1.2. This is the default for new releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants