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

Lazy adapter can cause infinite recursion #106

Closed
linaori opened this issue Jul 1, 2022 · 1 comment · Fixed by #108
Closed

Lazy adapter can cause infinite recursion #106

linaori opened this issue Jul 1, 2022 · 1 comment · Fixed by #108

Comments

@linaori
Copy link

linaori commented Jul 1, 2022

I made a mistake in my configuration which lead me to infinite recursion (segfaulting). Turns out I had the name of the lazy storage in my environment variable, trying to infinitely create itself through LazyFactory::create()

parameters:
    env(RECURSION_SOURCE): recursion

flysystem:
    storages:
        recursion:
            adapter: 'lazy'
            options:
                source: '%env(RECURSION_SOURCE)%' #also breaks if I just fill in "recursion" here

Due to the nature of being a runtime calculation on which storage to use, I think the factory should probably validate that it's not trying to initialize the lazy adapter as the lazy adapter. If multiple levels of factories should be support (lazy -> lazy -> real) my suggestion won't solve it, but this would be a fix for my scenario:

class LazyFactory
{
    private $storages;

    public function __construct(ContainerInterface $storages)
    {
        $this->storages = $storages;
    }

    public function createStorage(string $source, string $storageName)
    {
        if ($source === $storageName) {
            throw new \InvalidArgumentException('... some message saying you try to initialize the lazy adapter');
        }

        if (!$this->storages->has($source)) {
            throw new \InvalidArgumentException('You have requested a non-existent source storage "'.$source.'" in lazy storage "'.$storageName.'".');
        }

        return $this->storages->get($source);
    }
}
@tgalopin
Copy link
Member

tgalopin commented Jul 3, 2022

This is a good idea indeed, if you're open to create PR I'd gladly merge it, otherwise I'll have a look :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants