[Bug]: FlysystemVisibilityPass causing problems with other Flysystem adapters

tomhatzer commented 5 days ago

Pimcore version

11.2.6

Steps to reproduce

Try using flysystem with a provider with non-S3 storage system adapter (eg. PlatformCommunity/flysystem-bunnycdn). Pimcore will error out with this exception as the FlysystemVisibilityPass class is only checking for fully S3 compatible storages that support arguments at specified places for visibility. Adapters that do not support visibility or have different argument placing will not work when specified.

The problem lies within this code piece here https://github.com/pimcore/pimcore/blob/76a0f020262381a91f87c742debf1d69e1c84930/bundles/CoreBundle/src/DependencyInjection/Compiler/FlysystemVisibilityPass.php#L37-L42 inside the FlysystemVisibilityPass which checks for arguments at position 1. This will only work for S3 but not for other storage types.

Exception:

PHP Fatal error:  Uncaught Error: Call to a member function setArgument() on string in /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/bundles/CoreBundle/src/DependencyInjection/Compiler/FlysystemVisibilityPass.php:41
Stack trace:
#0 /home/webserver/pimcore-test.local/vendor/symfony/dependency-injection/Compiler/Compiler.php(80): Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\FlysystemVisibilityPass->process()
#1 /home/webserver/pimcore-test.local/vendor/symfony/dependency-injection/ContainerBuilder.php(767): Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
#2 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(506): Symfony\Component\DependencyInjection\ContainerBuilder->compile()
#3 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Kernel.php(155): Symfony\Component\HttpKernel\Kernel->initializeContainer()
#4 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(763): Pimcore\Kernel->initializeContainer()
#5 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(126): Symfony\Component\HttpKernel\Kernel->preBoot()
#6 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Kernel.php(140): Symfony\Component\HttpKernel\Kernel->boot()
#7 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Bootstrap.php(255): Pimcore\Kernel->boot()
#8 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Bootstrap.php(64): Pimcore\Bootstrap::kernel()
#9 /home/webserver/pimcore-test.local/bin/console(36): Pimcore\Bootstrap::startupCli()
#10 /home/webserver/pimcore-test.local/vendor/autoload_runtime.php(24): {closure}()
#11 /home/webserver/pimcore-test.local/bin/console(20): require_once('...')
#12 {main}
  thrown in /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/bundles/CoreBundle/src/DependencyInjection/Compiler/FlysystemVisibilityPass.php on line 41

Actual Behavior

Throws an exception when not using fully S3 compatible adapters.

Exception:

PHP Fatal error:  Uncaught Error: Call to a member function setArgument() on string in /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/bundles/CoreBundle/src/DependencyInjection/Compiler/FlysystemVisibilityPass.php:41
Stack trace:
#0 /home/webserver/pimcore-test.local/vendor/symfony/dependency-injection/Compiler/Compiler.php(80): Pimcore\Bundle\CoreBundle\DependencyInjection\Compiler\FlysystemVisibilityPass->process()
#1 /home/webserver/pimcore-test.local/vendor/symfony/dependency-injection/ContainerBuilder.php(767): Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
#2 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(506): Symfony\Component\DependencyInjection\ContainerBuilder->compile()
#3 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Kernel.php(155): Symfony\Component\HttpKernel\Kernel->initializeContainer()
#4 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(763): Pimcore\Kernel->initializeContainer()
#5 /home/webserver/pimcore-test.local/vendor/symfony/http-kernel/Kernel.php(126): Symfony\Component\HttpKernel\Kernel->preBoot()
#6 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Kernel.php(140): Symfony\Component\HttpKernel\Kernel->boot()
#7 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Bootstrap.php(255): Pimcore\Kernel->boot()
#8 /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/lib/Bootstrap.php(64): Pimcore\Bootstrap::kernel()
#9 /home/webserver/pimcore-test.local/bin/console(36): Pimcore\Bootstrap::startupCli()
#10 /home/webserver/pimcore-test.local/vendor/autoload_runtime.php(24): {closure}()
#11 /home/webserver/pimcore-test.local/bin/console(20): require_once('...')
#12 {main}
  thrown in /home/webserver/pimcore-test.local/vendor/pimcore/pimcore/bundles/CoreBundle/src/DependencyInjection/Compiler/FlysystemVisibilityPass.php on line 41

Expected Behavior

Skip FlysystemVisibilityPass completely if provider does not have expected arguments at the position we expect them to be or provider is not an S3 compatible provider.

tomhatzer commented 5 days ago

Example configs for bunny.net integration

config/packages/flysystem.yaml

flysystem:
    storages:
        pimcore.asset.storage:
            # Storage for asset source files, directory structure is equal to the asset tree structure
            adapter: 'bunnycdn_adapter'
            visibility: private
            directory_visibility: public
            options:
                client: 'bunnycdn_client'
                pullzone_url: 'https://assets.example-pullzone.com'

services.yaml

services:
    bunnycdn_client:
        class: 'PlatformCommunity\Flysystem\BunnyCDN\BunnyCDNClient'
        arguments:
            -  storage_zone_name: '<replace with your storage zone name>'
               api_key: '<replace with your api key>'
               region: '<replace with your region name>'

    bunnycdn_adapter:
        class: 'PlatformCommunity\Flysystem\BunnyCDN\BunnyCDNAdapter'
        arguments:
            -  client: 'bunnycdn_client'
               pullzone_url: '<replace with your pullzone url>'

tomhatzer commented 22 hours ago

Short update:

According to the comment by @brusch here, the FlysystemVisibilityPass can now be removed as it's not needed anymore as the flysystem-bundle has updated their implementation in https://github.com/thephpleague/flysystem-bundle/pull/153 (respectively https://github.com/thephpleague/flysystem-bundle/pull/139 as mentioned in the comment by him).

I'll open a PR and remove the FlysystemVisibilityPass compiler in it as it should not be needed anymore.

Thank you!

pimcore / pimcore