Skip to content

Handled storage of absolute image urls#10025

Merged
kevinansfield merged 7 commits intoTryGhost:masterfrom
rshbhgrg:absolute-image-to-relative
Oct 18, 2018
Merged

Handled storage of absolute image urls#10025
kevinansfield merged 7 commits intoTryGhost:masterfrom
rshbhgrg:absolute-image-to-relative

Conversation

@rshbhgrg
Copy link
Copy Markdown
Contributor

@rshbhgrg rshbhgrg commented Oct 17, 2018

refs #10024

  • Updated input serializers for posts/tags/users to handle absolute urls conversion

  1. Ghost stores images without domain
  2. API V2 returns images with absolute urls
  3. Ghost-Admin sends absolute urls back on any save e.g. update user

Current behavior: This will override the relative image path in db to absolute, which in turn won't get updated in future if domain or protocol changes for e.g.
Fix: On save/update, input serializers converts any absolute image url paths back to relative if the base URL from image fields matches the configured URL

@rshbhgrg rshbhgrg force-pushed the absolute-image-to-relative branch from b5d94ef to 8ba51cf Compare October 17, 2018 20:57
refs TryGhost#10024

- Updated input serializers for posts/tags/users to handle absolute urls conversion
@rshbhgrg rshbhgrg force-pushed the absolute-image-to-relative branch from 8ba51cf to c455872 Compare October 17, 2018 20:59
}
}

frame.data.posts[0] = url.forPost(Object.assign({}, frame.data.posts[0]), frame.options);

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.


function handleImageUrl(imageUrl) {
let blogUrl = getBlogUrl();
if (url.parse(imageUrl).host === url.parse(blogUrl).host) {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@kirrg001
Copy link
Copy Markdown
Contributor

@gargol Could you pls review and do a functional test (including subdirectory)? I'll do a final review asap 👍 Thank you :)

@rshbhgrg rshbhgrg force-pushed the absolute-image-to-relative branch from e90255e to d472734 Compare October 18, 2018 07:21
const url = require('./utils/url');

module.exports = {
edit(apiConfig, frame) {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

const url = require('./utils/url');

module.exports = {
edit(apiConfig, frame) {

This comment was marked as abuse.

const {absoluteToRelative, getBlogUrl, STATIC_IMAGE_URL_PREFIX} = require('../../../../../../services/url/utils');

function handleImageUrl(imageUrl) {
let blogUrl = getBlogUrl().replace(/^http(s?):\/\//, '').replace(/\/$/, '');

This comment was marked as abuse.

This comment was marked as abuse.

@@ -0,0 +1,67 @@
const {absoluteToRelative, getBlogUrl, STATIC_IMAGE_URL_PREFIX} = require('../../../../../../services/url/utils');

function handleImageUrl(imageUrl) {

This comment was marked as abuse.

Comment thread core/server/api/v2/utils/serializers/input/utils/url.js Outdated
@naz
Copy link
Copy Markdown
Contributor

naz commented Oct 18, 2018

Found an issue during testing, image url is not properly handled when moving blog to a subdir: "url": "http://ghost.local", -> "url": "http://ghost.local/blog/". In the latter example, the image is still being served from: /content/images/2018/10/DltA_pyXgAA-Lw_.jpg-large.jpeg. Probably should be tackled with this PR, but maybe would make sense to break things out a bit if it's critical to release a partial fix to just a domain change issue 🤔 ?

@rshbhgrg
Copy link
Copy Markdown
Contributor Author

Discussed #10025 (comment) : Its not caused by changes here, there are known problems in changing domains/blog url right now since content is not re-written, so will not be patched in this PR.

@kevinansfield
Copy link
Copy Markdown
Member

problems in changing domains/blog url right now since content is not re-written

To clarify, changing subdomains is a manual process because we don't know which stored URLs are meant to be relative ghost root and which are meant to be relative to the domain root. We'll open separate issues and PRs to handle that side of things if it's something we want to try automating in the future.

@kevinansfield kevinansfield merged commit 915d561 into TryGhost:master Oct 18, 2018
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.

4 participants