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

[HttpKernel] Fix a PHP 8.1 deprecation notice in HttpCache #46769

Merged
merged 1 commit into from
Jun 26, 2022

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jun 24, 2022

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

PHP 8.1 may trigger a deprecation notice PHP Deprecated: abs(): Passing null to parameter #1 ($num) of type int|float is deprecated in .../symfony/http-kernel/HttpCache/HttpCache.php on line 721

The reason is that $entry->getTtl() may return null for cache entries where no freshness information is present.

I think we would err on the safe side by not using stale-while-revalidate behaviour in this case.

@mpdude
Copy link
Contributor Author

mpdude commented Jun 24, 2022

I know @dbu, @Tobion and @aschempp know a lot about caching and the HttpCache – do you agree we should handle it this way?

Alternatively, we could assume the TTL to be zero, which is what abs(null) effectively evaluates to.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

my gut feeling would be to treat null as 0 to not introduce a change.


return abs($entry->getTtl()) < $timeout;
return abs($ttl) < $timeout;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return abs($ttl) < $timeout;
return abs($entry->getTtl() ?: 0) < $timeout;

Copy link
Member

Choose a reason for hiding this comment

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

The suggestion looks good to me.

@carsonbot carsonbot changed the title Fix a PHP 8.1 deprecation notice in HttpCache [HttpKernel] Fix a PHP 8.1 deprecation notice in HttpCache Jun 26, 2022
@nicolas-grekas
Copy link
Member

Thank you @mpdude.

@nicolas-grekas nicolas-grekas merged commit 45db39c into symfony:4.4 Jun 26, 2022
@mpdude mpdude deleted the patch-5 branch June 26, 2022 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants