"Publication icon" does not work well with custom storage adapter

[wade-r]

Issue Summary

"Publication icon" does not work well with custom storage adapter.

Steps to Reproduce

1. Setup a custom storage module, mine is https://github.com/unit2b/ghost-oss-store

2. Upload a "Publication icon"

3. Upload works, but the url of the most top left icon in admin UI is wrong, it's something looks like https://yordomain.com/https://static.yordomain.com/logo.png, site url is prepended by mistake.

4. favicon middleware failed to serve a favicon image, it tries to read the icon file from storage adapter with a full url. In this circumstance, favicon middleware should simply do a redirection.

Technical details:

• Ghost Version: 1.5.2
• Node Version: 6.11.2
• Browser/OS: Chrome 60.0.3112.90 64bit, CentOS 7.3
• Database: MySQL 6.7

I checked the Database, when custom storage adapter is applied, icon option in settings will be a full url, however icon related stuffs are not ready for a full url.

[ErisDS]

I've not verified this bug, as it lives in custom storage adapters and that's not something anyone the core team makes regular use of. However, it sounds very plausible.

A pull request to fix this would be very welcome.

[guoyk93]

@ErisDS I made a small work around #8915 for this issue, but I believe storage adapter need some kind of redesign, which is beyond my capability.

[kirrg001]

I wonder why custom storage adapters return the url from the storage provider when storing the image (e.g. https://res.cloudinary.com/dtvsrihpe/image/upload/my-image.png) and not the Ghost image path /content/images/{unique_id}.{ext}? 🤔 Ghost has no restrictions or rules for that conceptional question.

Because imagine you want to use a different storage adapter, you just need to move your images to the new provider, but the path is still the same, just the underlying storage has changed. By that we wouldn't run into this bug. So right now a storage adapter is able to store a whole external url for images.

cc @ErisDS

[kevinansfield]

In general we want the front-end to make as few hops as possible to load assets, if storage engine urls are stored as /content/images/* then the front-end needs to make two requests for every asset:

1. /content/images/x.png -> 301
2. https://res.cloudinary.com/foo/image/upload/x.png

Changing content storages then gives you problems where images will still fail for some users anyway because the CDN and browsers will have cached the 301. If we send a 302 response then it will constantly re-checked by browsers and CDNs and you lose a lot of the benefits of local caches.

The other option would be Ghost proxying to the storage engine, that's fine for images which are stored on a local file system/server but for remote images Ghost itself will now be making long requests for each asset:

browser -> ghost -> browser

becomes:

browser -> ghost -> remote host -> ghost -> browser

[ErisDS]

Another way to look at it is that locally stored files are actually the special case. Every other custom storage adapter provided by the community deals with full URLs.

[kevinansfield]

Upload works, but the url of the most top left icon in admin UI is wrong, it's something looks like https://yordomain.com/https://static.yordomain.com/logo.png, site url is prepended by mistake.

This was fixed via TryGhost/Admin@7dd2090.

favicon middleware failed to serve a favicon image, it tries to read the icon file from storage adapter with a full url.

This sounds like a duplicate of #8771.

I'm going to close this for now as it appears all issues have been resolved. We can open more specific issues if there are any further issues with favicon.