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

[5.x] Private asset container url method should return null #10769

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

jasonvarga
Copy link
Member

@jasonvarga jasonvarga commented Sep 5, 2024

This was always the intention, but it didn't.

Doing $asset->url() on a private asset would already return null, it's just calling on the container itself that had the issue.

This was discovered by doing Asset::findByUrl($url) and having a private S3 container.

protected function resolveContainerFromUrl($url)
{
return AssetContainer::all()->sortByDesc(function ($container) {
return strlen($container->url());
})->first(function ($container, $id) use ($url) {
return Str::startsWith($url, $container->url())
|| Str::startsWith(URL::makeAbsolute($url), $container->url());
});
}

It would loop through all containers, get their urls, and figure out a matching container. When you had a private s3 container, getting its URL here would throw an exception. Now since it returns null the exception is avoided. The private container is skipped in findByUrl as expected, since they don't have URLs.

@jasonvarga jasonvarga merged commit 03ee12b into 5.x Sep 5, 2024
19 checks passed
@jasonvarga jasonvarga deleted the private-asset-container-url branch September 5, 2024 21:01
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.

1 participant