-
-
Notifications
You must be signed in to change notification settings - Fork 607
Description
Bug description
Ever since this feature was released (#5143 (comment)), we have had the oppurtunity to use our custom CDN as cache.
While using a custom CDN (e.g. "glide" which points to a public S3 bucket, as shown in the documentation).
I expected it to actually be served over HTTP instead of being served from local storage.
After some digging, I noticed that shouldServeByHttp() is a reverse of shouldServeDirectly() which is the configuration setting statamic.assets.image_manipulation.cache casted to an boolean.
As you probably already know; a (bool) "string" will always return true and so it will be served directly (from local storage).
How to reproduce
- Create a Statamic project
- Configure Asset Container (e.g. "foo") and add a random image (e.g. "bar.jpg").
- Follow documentation: https://statamic.dev/image-manipulation#custom-disk-cdn
- Use the base64 encoded path
foo/bar.jpgwhile using the Glide endpoint: http://localhost:8000/img/asset/Zm9vL2Jhci5qcGc=`
Logs
No response
Environment
Environment
Application Name: Laravel
Laravel Version: 9.51.0
PHP Version: 8.2.1
Composer Version: 2.5.1
Environment: local
Debug Mode: ENABLED
URL: localhost
Maintenance Mode: OFF
Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED
Drivers
Broadcasting: log
Cache: statamic
Database: mysql
Logs: stack / single
Mail: smtp
Queue: sync
Session: file
Statamic
Addons: 1
Antlers: runtime
Stache Watcher: Disabled
Static Caching: Disabled
Version: 3.4.11 PRO
Statamic Addons
statamic/eloquent-driver: 1.2.0Installation
Existing Laravel app
Antlers Parser
None
Additional details
My implementation to solve this issue would be to simply check, if the URL of the used cache disk starts with "http". However I'm unsure if this matches what shouldServeByHttp() was originally intended for and if this would break anything else:
public function shouldServeByHttp()
{
return ! $this->shouldServeDirectly() || strpos($this::cacheDisk()->url('/'), "http") === 0;
}
One more thing I noticed is that S3 buckets that are used for caching should also have their ACL's enabled. I faced the following error earlier:
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>AccessControlListNotSupported</Code>
<Message>The bucket does not allow ACLs</Message>
<RequestId></RequestId>
<HostId></HostId>
</Error>
After enabling ACL's on Amazon AWS Console, the error message disappeared. I suppose it would be a good idea to add a few pointers in the documentation to also enable ACL's. Simply creating a public bucket was not enough.