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

$config in AwsS3V3Adapter copy function is null/empty #1759

Open
srabouin opened this issue Feb 29, 2024 · 4 comments
Open

$config in AwsS3V3Adapter copy function is null/empty #1759

srabouin opened this issue Feb 29, 2024 · 4 comments

Comments

@srabouin
Copy link

Bug Report

Q A
Flysystem Version 3.24.0
Adapter Name AwsS3V3Adapter
Adapter version 3.24.0

Summary

I am using Laravel 9.52.16 and the S3 driver to upload/move files to Cloudflare R2. The upload is in a temporary folder, and then I move it to the final folder. The move function is the issue.

In the config, I have set visibility to 'private' to prevent calling GetObjectAcl that is not supported by R2, but after upgrading to 3.24.0 it gave the error that GetObjectAcl is not supported.

During troubleshooting I just dumped the $config variable when the copy function is called here

$visibility = $config->get(Config::OPTION_VISIBILITY);
and found it was null/empty. Reverting to 3.15.0 and everything is working fine again.

It's possible I missed something?

How to reproduce

R2 config:

'disks' => [
        'r2' => [
            'driver' => 's3',
            'key' => env('CLOUDFLARE_R2_ACCESS_KEY_ID'),
            'secret' => env('CLOUDFLARE_R2_SECRET_ACCESS_KEY'),
            'region' => env('CLOUDFLARE_R2_REGION'),
            'bucket' => env('CLOUDFLARE_R2_BUCKET'),
            'disable_asserts' => true,
            'visibility' => 'private',
            'endpoint' => env('CLOUDFLARE_R2_ENDPOINT'),
            'url' => env('CLOUDFLARE_R2_URL'),
            'domain' => env('CLOUDFLARE_R2_FILE_DOMAIN', ''),
            'domain_original' => env('CLOUDFLARE_R2_ORIGINAL_DOMAIN', ''),
            'throw' => true,
        ],
]

Single line of code:

Storage::disk('r2')->move($from, $to);

Actual error, removed irrelevant lines:

[previous exception] [object] (GuzzleHttp\\Exception\\ServerException(code: 501): Server error: `GET https://bucket.x.r2.cloudflarestorage.com/submissions/tmp/file.zip?acl` resulted in a `501 Not Implemented` response:
<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>NotImplemented</Code><Message>GetObjectAcl not implemented</Message>< (truncated...)
 at /var/www/html/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:113)
[...]
#14 /var/www/html/vendor/league/flysystem-aws-s3-v3/AwsS3V3Adapter.php(440): Aws\\AwsClient->execute(Object(Aws\\Command))
#15 /var/www/html/vendor/league/flysystem-aws-s3-v3/AwsS3V3Adapter.php(410): League\\Flysystem\\AwsS3V3\\AwsS3V3Adapter->visibility('submissions/tmp...')
#16 /var/www/html/vendor/league/flysystem-aws-s3-v3/AwsS3V3Adapter.php(399): League\\Flysystem\\AwsS3V3\\AwsS3V3Adapter->copy('submissions/tmp...', 'submissions/102...', Object(League\\Flysystem\\Config))
#17 /var/www/html/vendor/league/flysystem/src/Filesystem.php(137): League\\Flysystem\\AwsS3V3\\AwsS3V3Adapter->move('submissions/tmp...', 'submissions/102...', Object(League\\Flysystem\\Config))
#18 /var/www/html/vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemAdapter.php(537): League\\Flysystem\\Filesystem->move('submissions/tmp...', 'submissions/102...')
#19 /var/www/html/app/Services/Legacy/SubmissionService.php(710): Illuminate\\Filesystem\\FilesystemAdapter->move('submissions/tmp...', 'submissions/102...')
[...]
@martyf
Copy link

martyf commented Mar 21, 2024

The issue also exists with the copy function when used with R2, with $visibility being null, and throwing League\Flysystem\UnableToRetrieveMetadata, where GetObjectAcl not implemented appears too.

Storage::disk('r2')->copy($sourcePath, $targetPath);

Will run this code:

public function copy(string $source, string $destination, Config $config): void
    {
        try {
            $visibility = $config->get(Config::OPTION_VISIBILITY);

            if ($visibility === null && $config->get(Config::OPTION_RETAIN_VISIBILITY, true)) {
                $visibility = $this->visibility($source)->visibility();
            }
        } catch (Throwable $exception) {
            throw UnableToCopyFile::fromLocationTo(
                $source,
                $destination,
                $exception
            );
        }
    // ...

This is where the exception is caught. $visibility will be null, then the exception thrown.

My disk setup is like yours: visibility set to private.

Downgrading to 3.15 does not resolve it for me though.

@it-can
Copy link

it-can commented May 6, 2024

Did any of you guys find a solution for the problem? Move (and copy) seems not to be working on r2

@it-can
Copy link

it-can commented May 6, 2024

this seems related: #1791

@it-can
Copy link

it-can commented May 6, 2024

it seems the config is not passed down to the copy() function

$config = new Config([
    Config::OPTION_VISIBILITY => 'private',
]);

Storage::disk('r2')->move($orgFile, $archiveFile, $config);

when i debug this with ray(), i see the config object is not the same

Scherm­afbeelding 2024-05-06 om 17 06 38

So it seems the Laravel Disk/storage config is not passed correctly, so it tries to fetch the visibility from the file (GetObjectACL is not supported by Cloudflare R2), so it will throw an Exception.

It seems the the laravel disk config is not passed correctly to AwsS3V3Adapter.

--

UPDATE
when i manually comment out this part of the code:

try {
            $visibility = $config->get(Config::OPTION_VISIBILITY);

            if ($visibility === null && $config->get(Config::OPTION_RETAIN_VISIBILITY, true)) {
                $visibility = $this->visibility($source)->visibility();
            }
        } catch (Throwable $exception) {
            throw UnableToCopyFile::fromLocationTo(
                $source,
                $destination,
                $exception
            );
        }

and then add

$visibility = 'private';

the "moving/copying" of the file works...

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

No branches or pull requests

3 participants