thephpleague / flysystem

Abstraction for local and remote filesystems
https://flysystem.thephpleague.com
MIT License
13.25k stars 824 forks source link

AbstractCache: array_key_exists() expects parameter 2 to be array, null given #307

Closed scarlac closed 9 years ago

scarlac commented 9 years ago

Currently using /GrahamCampbell/Laravel-Flysystem's layer on top of Flysystem. When using anything but a dummy (simple single-request array-based) cache driver I get this error when calling ->has(...):

array_key_exists() expects parameter 2 to be array, null given

Code:

     *
     * @param  string  $dirname
     * @param  boolean $recursive
     * @return boolean
     */
    public function isComplete($dirname, $recursive)
    {
        if ( ! array_key_exists($dirname, $this->complete)) {
            return false;
        }
(...)

It seems $this->complete is simply not defined per default. I am not sure if the error is in the framework or in GrahamCampbell's layer, so let me know if this is way off. For now I simply have to turn of caching entirely to avoid issues. I'd love it if I could use memcached :)

GrahamCampbell commented 9 years ago

The cache driver is in my repo btw.


https://github.com/GrahamCampbell/Laravel-Flysystem/blob/1.0/src/Cache/IlluminateCache.php https://github.com/GrahamCampbell/Laravel-Flysystem/blob/1.0/src/Cache/IlluminateConnector.php

frankdejonge commented 9 years ago

@scarlac which version are you using?

frankdejonge commented 9 years ago

The default is set to array here: https://github.com/thephpleague/flysystem/blob/master/src/Cache/AbstractCache.php#L23

scarlac commented 9 years ago

I believe it's 0.5.10 based on the latest version mentioned in changelog.md.

scarlac commented 9 years ago

Yeah, I investigated and found that too. I have no insight into how caching works so I can't tell how it gets set to null...

frankdejonge commented 9 years ago

From what I can tell, it can only happen when the data-store corrupts it. @GrahamCampbell you might know the laravel internals better. Could there be an edge case with json_encoded stuff?

scarlac commented 9 years ago

Ok, so I have some speculative insights. If I configure it to use "Illuminate" caching and add a cache key, it'll likely call IlluminateCache->load() at some point: https://github.com/GrahamCampbell/Laravel-Flysystem/blob/1.0/src/Cache/IlluminateCache.php on line 75:

if (($contents = $this->client->get($this->key)) !== null) {
    $this->setFromStorage($contents);
}

If cache restoring for $key fails (which I guess it could?), that means that setFromStorage() will call list($cache, $complete) = json_decode(...) and $this->complete = $complete in setFromStorage() using passed JSON data. And since that's empty, it'll be null.

scarlac commented 9 years ago

Using Laravel's cache interface may give you FALSE (depending on backend - for memcached: http://php.net/manual/en/memcache.get.php), not just NULL, which means this check will fail. This could maybe be fixed easily by checking for FALSE and NULL on line 75 of IlluminateCache.php? But perhaps even better: AbstractCache.php:453 ensures $this->complete is always a boolean when decoding json data?

frankdejonge commented 9 years ago

@scarlac this was my hunch too. Although it seems the laravel internals check for the result code. I think then this is something weird about memcached (which is not uncommon) that eventhough a cache entry wasn't available the operation itself was successful. The result, however, would then still be false. I've reproduced the following effects locally:

list($a, $b) = json_decode(false);
var_dump($a, $b);
// NULL
// NULL
frankdejonge commented 9 years ago

@GrahamCampbell the comments above should be interesting to you :)

scarlac commented 9 years ago

By "boolean" i meant array. I have patched my own local install of Flysystem with:

$this->complete = is_array($complete) ? $complete : [];

in AbstractCache.php, line 453. Should ensure that this internal var is not corrupted by outside data. This fixed the issue for me. Would you consider this a fix or is it too dirty, @FrenkyNet ?

frankdejonge commented 9 years ago

@scarlac how about that ^ ?

scarlac commented 9 years ago

@FrenkyNet - that fix makes sense, yet it does not fix this particular error, sorry. json_decode() won't barf when you try to decode FALSE (which is kinda odd):

[1] > echo json_decode(false);
[2] > echo json_last_error();
0[3] > 
(...)
[5] > echo JSON_ERROR_NONE;
0[6] > echo JSON_ERROR_STATE_MISMATCH;
2[7] >
frankdejonge commented 9 years ago

Ok, this should kill this. Heading to bed now! Later guys.

scarlac commented 9 years ago

Seems to be working! Thanks @FrenkyNet :)