ProviderRepository::loadManifest assumes services.json is not empty

[creste]

When server disk space fills up it is possible for an empty services.json to be written to disk. Subsequent requests will fail because ProviderRepository::loadManifest assumes that services.json is never empty. The error in the log file will vary, but it usually looks something like this:

[17-May-2016 19:57:12 UTC] PHP Fatal error: Call to undefined method Illuminate\Support\Facades\Auth::user() in /app/laravel/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php on line 207

That is not the real error though, because that error is caused by some application-level error handling code that expects Auth::user() to be defined. It is not defined because laravel's bootstrap process fails before services can be registered. The services aren't registered because of the actual error, which is contained in the inner exception:

[message:protected] => array_merge(): Argument #2 is not an array
[string:Exception:private] =>
[code:protected] => 0
[file:protected] => /app/laravel/vendor/laravel/framework/src/Illuminate/Foundation/ProviderRepository.php
[line:protected] => 174

This is the loadManifest() method around line 174 of ProviderRepository.php:

    public function loadManifest()
    {
        // The service manifest is a file containing a JSON representation of every
        // service provided by the application and whether its provider is using
        // deferred loading or should be eagerly loaded on each request to us.
        if ($this->files->exists($this->manifestPath)) {
            $manifest = json_decode($this->files->get($this->manifestPath), true);

            return array_merge(['when' => []], $manifest);
        }
    }

$manifest, which is passed as argument 2 to array_merge, is not an array because ProviderRepository::loadManifest() doesn't check the return value of json_decode(), which is null when services.json is empty.

To fix this, I changed this code in Illuminate/Foundation/ProviderRepository::loadManifest()

            return array_merge(['when' => []], $manifest);

to:

            if ($manifest) {
                    return array_merge(['when' => []], $manifest);
            }

[GrahamCampbell]

Thanks for the report. I'll fix this shortly. ;)

[GrahamCampbell]

Please confirm #14030 works for you. ;)

[creste]

I don't think that fix will work because I tried something similar previously. The problem with that fix is that if loadManifest() returns an array then ProviderRepository::load() will store the array in $manifest and pass it to ProviderRepository::shouldRecompile(). Inside that function, if $manifest is not null then shouldRecompile() attempts to access $manifest['providers']. That will fail because $manifest doesn't have an array key called "providers". It only has a single array key called "when".

I think the proper fix is to make sure loadManifest returns null when json_decode returns null. That will guarantee that shouldRecompile returns true by short-circuiting the || condition before it tries to access an array key that doesn't exist in $manifest.

[GrahamCampbell]

Ah, ok. Thanks. I'll modify my PR. ;)

[creste]

Looks good now. Thank you!

[GrahamCampbell]

Thanks for reporting. :)

[sk33wiff]

I'm getting a similar error, which seems to happen randomly, when using

• docker
• https://github.com/brianium/paratest or
• https://github.com/liuggio/fastest

The issue is that, for some unclear reason, $manifest might not be an array, so it fails to merge.

    public function loadManifest()
    {
        // The service manifest is a file containing a JSON representation of every
        // service provided by the application and whether its provider is using
        // deferred loading or should be eagerly loaded on each request to us.
        if ($this->files->exists($this->manifestPath)) {
            $manifest = $this->files->getRequire($this->manifestPath);

            if ($manifest) {
                return array_merge(['when' => []], $manifest);
            }
        }
    }

Var values during an error:

$this->manifestPath = '/var/app/current/src/vendor/orchestra/testbench-core/src/Traits/../../laravel/bootstrap/cache/services.php'
$manifest = 1

I'm not sure if anyone else could be facing the same issue, but independently of my scenario, this might be a better check:

if (is_array($manifest)) {

instead of

if ($manifest) {