Skip to content

Conversation

@TysonAndre
Copy link
Contributor

@TysonAndre TysonAndre commented Jan 5, 2019

Q A
Branch? 4.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes (build failure seems unrelated)
Fixed tickets
License MIT
Doc PR

Previously, if this were called, it would throw an ArgumentCountError.
I'm assuming existing code always checks hasItem, so this bug hasn't impacted many people.
This was noticed via static analysis.

The get() method was added to NullAdapter in symfony 4.2

@TysonAndre
Copy link
Contributor Author

code to reproduce:

<?php
require_once 'vendor/autoload.php';
$x = new Symfony\Component\Cache\Adapter\NullAdapter();
var_export($x->get('key', function (...$args) {}));  // would throw ArgumentCountError

Previously, if this were called, it would throw an ArgumentCountError.
I'm assuming existing code always checks hasItem, so this bug hasn't impacted many people.
This was noticed via static analysis.

The get() method was added to NullAdapter in symfony 4.2
@chalasr chalasr added this to the 4.2 milestone Jan 5, 2019
@chalasr chalasr added the Cache label Jan 5, 2019
@nicolas-grekas
Copy link
Member

Good catch, thanks @TysonAndre.

@nicolas-grekas nicolas-grekas merged commit 1976d29 into symfony:4.2 Jan 25, 2019
nicolas-grekas added a commit that referenced this pull request Jan 25, 2019
This PR was merged into the 4.2 branch.

Discussion
----------

Always pass $key to NullAdapter->createCacheItem

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes (build failure seems unrelated)
| Fixed tickets |
| License       | MIT
| Doc PR        |

Previously, if this were called, it would throw an ArgumentCountError.
I'm assuming existing code always checks hasItem, so this bug hasn't impacted many people.
This was noticed via static analysis.

The get() method was added to NullAdapter in symfony 4.2

Commits
-------

1976d29 Always pass $key to NullAdapter->createCacheItem
@fabpot fabpot mentioned this pull request Feb 3, 2019
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.

4 participants