TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-12-01 05:50:35 +03:00
Code Issues Projects Releases Wiki Activity

🐛 Fixed img-url to output relative url by default (#10596)

Browse Source 
closes #10595

* Added breaking test for img-url helper

Input from the content API was absolute, adding this test to verify my fix

* Updated existing test to breaking test for img-url

Had made a dumb assumption when building images sizes, this updates the
test to fail so we can verify the fix

* Refactored img-url to return as early as possible

This makes it a little easier to parse what's going on, and it also
allows us to remove the check for existence of the image url in the
getImageSizes function

* Refactored img-url config parsing to clean up core logic

Superficial refactor to make future changes able to focus on what rather
than how.

* Refactored internal image detection into helper

We're gonna need to know if the image is internal or not, when we force
the path to relative, if we pull this out now, we have access in the top
level logic

* Removed duplicate checks for internal image

Cleaning up and moving "higher-level" logic into the main function of
the module

* Renamed attr -> requestedImageUrl

Superficial refactor, trying to be more explicit about identifiers

* 🐛 Fixed img-url to output relative url by default

Includes a check to isInternalImage as we never want to make external
images relative.

* Returned early if img-url recieves external url

After realising we never want to deal with external urls, we can
continue to return as early as possible, letting us remove checks and
simplify the more complex logic for internal images.

* Cleaned up the internal image logic

Defining the three functions in order helps to see what operations are
going to happen and in which order, we can then return the result of
each operation applied to the next operation.
This commit is contained in:
Fabien O'Carroll 2019-03-11 15:20:05 +01:00 committed by GitHub
parent 812986b215
commit a2aa66ca73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 75 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
core/server/helpers/img_url.js
View File

@ -8,61 +8,96 @@
// `absolute` flag outputs absolute URL, else URL is relative.
const url = require('url');
const _ = require('lodash');
const proxy = require('./proxy');
const urlService = proxy.urlService;
const STATIC_IMAGE_URL_PREFIX = `/${urlService.utils.STATIC_IMAGE_URL_PREFIX}`;
module.exports = function imgUrl(attr, options) {
// CASE: if no attribute is passed, e.g. `{{img_url}}` we show a warning
module.exports = function imgUrl(requestedImageUrl, options) {
// CASE: if no url is passed, e.g. `{{img_url}}` we show a warning
if (arguments.length < 2) {
proxy.logging.warn(proxy.i18n.t('warnings.helpers.img_url.attrIsRequired'));
return;
}
const absolute = options && options.hash && options.hash.absolute && options.hash.absolute !== 'false';
const size = options && options.hash && options.hash.size;
const imageSizes = options && options.data && options.data.config && options.data.config.image_sizes;
const image = getImageWithSize(attr, size, imageSizes);
// CASE: if attribute is passed, but it is undefined, then the attribute was
// CASE: if url is passed, but it is undefined, then the attribute was
// an unknown value, e.g. {{img_url feature_img}} and we also show a warning
if (image === undefined) {
if (requestedImageUrl === undefined) {
proxy.logging.warn(proxy.i18n.t('warnings.helpers.img_url.attrIsRequired'));
return;
}
if (image) {
return urlService.utils.urlFor('image', {image}, absolute);
// CASE: if you pass e.g. cover_image, but it is not set, then requestedImageUrl is null!
// in this case we don't show a warning
if (requestedImageUrl === null) {
return;
}
// CASE: if you pass e.g. cover_image, but it is not set, then attr is null!
// in this case we don't show a warning
// CASE: if you pass an external image, there is nothing we want to do to it!
const isInternalImage = detectInternalImage(requestedImageUrl);
if (!isInternalImage) {
return requestedImageUrl;
}
const {requestedSize, imageSizes} = getImageSizeOptions(options);
const absoluteUrlRequested = getAbsoluteOption(options);
function applyImageSizes(image) {
return getImageWithSize(image, requestedSize, imageSizes);
}
function getImageUrl(image) {
return urlService.utils.urlFor('image', {image}, absoluteUrlRequested);
}
function ensureRelativePath(image) {
return urlService.utils.absoluteToRelative(image);
}
// CASE: only make paths relative if we didn't get a request for an absolute url
const maybeEnsureRelativePath = !absoluteUrlRequested ? ensureRelativePath : _.identity;
return maybeEnsureRelativePath(
getImageUrl(
applyImageSizes(requestedImageUrl)
)
);
};
function getAbsoluteOption(options) {
const absoluteOption = options && options.hash && options.hash.absolute;
return absoluteOption ? !!absoluteOption && absoluteOption !== 'false' : false;
}
function getImageSizeOptions(options) {
const requestedSize = options && options.hash && options.hash.size;
const imageSizes = options && options.data && options.data.config && options.data.config.image_sizes;
return {
requestedSize,
imageSizes
};
}
function detectInternalImage(requestedImageUrl) {
const siteUrl = urlService.utils.getBlogUrl();
const isAbsoluteImage = /https?:\/\//.test(requestedImageUrl);
const isAbsoluteInternalImage = isAbsoluteImage && requestedImageUrl.startsWith(siteUrl);
// CASE: imagePath is a "protocol relative" url e.g. "//www.gravatar.com/ava..."
// by resolving the the imagePath relative to the blog url, we can then
// detect if the imagePath is external, or internal.
const isRelativeInternalImage = !isAbsoluteImage && url.resolve(siteUrl, requestedImageUrl).startsWith(siteUrl);
return isAbsoluteInternalImage || isRelativeInternalImage;
}
function getImageWithSize(imagePath, requestedSize, imageSizes) {
if (!imagePath) {
return imagePath;
}
if (!requestedSize) {
return imagePath;
}
const blogUrl = urlService.utils.getBlogUrl();
if (/https?:\/\//.test(imagePath) && !imagePath.startsWith(blogUrl)) {
return imagePath;
} else {
// CASE: imagePath is a "protocol relative" url e.g. "//www.gravatar.com/ava..."
// by resolving the the imagePath relative to the blog url, we can then
// detect if the imagePath is external, or internal.
const resolvedUrl = url.resolve(blogUrl, imagePath);
if (!resolvedUrl.startsWith(blogUrl)) {
return imagePath;
}
}
if (!imageSizes || !imageSizes[requestedSize]) {
return imagePath;
}

9
core/test/unit/helpers/img_url_spec.js
View File

@ -32,6 +32,13 @@ describe('{{image}} helper', function () {
logWarnStub.called.should.be.false();
});
it('should output relative url of image if the input is absolute', function () {
var rendered = helpers.img_url('http://localhost:82832/content/images/image-relative-url.png', {});
should.exist(rendered);
rendered.should.equal('/content/images/image-relative-url.png');
logWarnStub.called.should.be.false();
});
it('should output absolute url of image if the option is present ', function () {
var rendered = helpers.img_url('/content/images/image-relative-url.png', {hash: {absolute: 'true'}});
should.exist(rendered);
@ -120,7 +127,7 @@ describe('{{image}} helper', function () {
}
});
should.exist(rendered);
rendered.should.equal('http://localhost:82832/content/images/size/w400/my-coole-img.jpg');
rendered.should.equal('/content/images/size/w400/my-coole-img.jpg');
});
it('should output the correct url for protocol relative urls', function () {
var rendered = helpers.img_url('//website.com/whatever/my-coole-img.jpg', {