Skip to content

fix(3.x): Catch failure checking S3 file visibility#16940

Open
matdave wants to merge 2 commits intomodxcms:3.xfrom
matdave:3.x_S3visibility
Open

fix(3.x): Catch failure checking S3 file visibility#16940
matdave wants to merge 2 commits intomodxcms:3.xfrom
matdave:3.x_S3visibility

Conversation

@matdave
Copy link
Copy Markdown
Contributor

@matdave matdave commented Apr 1, 2026

What does it do?

Adds a catch around the $this->filesystem->visibility function call in S3 media sources.

Why is it needed?

Visibility can throw a FilesystemException exception if it is not implemented in the user's permissions. https://github.com/thephpleague/flysystem/blob/8aaffb653c5777781b0f7f69a5d937baf7ab6cdb/src/FilesystemAdapter.php#L68

How to test

Ran into this on a site where the user did not have permissions to retrieve object metadata. However, I'm unsure how to replicate that.

@matdave matdave requested review from Mark-H and opengeek as code owners April 1, 2026 17:21
return $this->filesystem->visibility($path);
try {
return $this->filesystem->visibility($path);
} catch (\Exception | FilesystemException $exception) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why the \Exception is also a log level info? In my opinion this means something important is broken right?

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 this pull request may close these issues.

2 participants