mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-11-25 19:48:50 +03:00
Not storing icons and thumbnails in case of mention (#21424)
Ref https://linear.app/ghost/issue/ENG-1662/incoming-recommendations-get-deleted-if-the-metadata-icon-fails-to https://linear.app/ghost/issue/ENG-904/bookmark-card-hot-linking-favicons
This commit is contained in:
parent
9f1945cb4d
commit
84473dd094
@ -9,6 +9,7 @@ module.exports = {
|
|||||||
emailFailures: require('./email-failures'),
|
emailFailures: require('./email-failures'),
|
||||||
images: require('./images'),
|
images: require('./images'),
|
||||||
integrations: require('./integrations'),
|
integrations: require('./integrations'),
|
||||||
|
oembed: require('./oembed'),
|
||||||
pages: require('./pages'),
|
pages: require('./pages'),
|
||||||
posts: require('./posts'),
|
posts: require('./posts'),
|
||||||
settings: require('./settings'),
|
settings: require('./settings'),
|
||||||
|
@ -0,0 +1,5 @@
|
|||||||
|
const url = require('../utils/url');
|
||||||
|
|
||||||
|
module.exports = (path) => {
|
||||||
|
return url.forImage(path);
|
||||||
|
};
|
@ -1,8 +1,15 @@
|
|||||||
const debug = require('@tryghost/debug')('api:endpoints:utils:serializers:output:oembed');
|
const debug = require('@tryghost/debug')('api:endpoints:utils:serializers:output:oembed');
|
||||||
|
const mappers = require('./mappers');
|
||||||
|
|
||||||
module.exports = {
|
module.exports = {
|
||||||
all(data, apiConfig, frame) {
|
all(data, apiConfig, frame) {
|
||||||
debug('all');
|
debug('all');
|
||||||
|
if (data?.metadata?.thumbnail) {
|
||||||
|
data.metadata.thumbnail = mappers.oembed(data.metadata.thumbnail);
|
||||||
|
}
|
||||||
|
if (data?.metadata?.icon) {
|
||||||
|
data.metadata.icon = mappers.oembed(data.metadata.icon);
|
||||||
|
}
|
||||||
frame.response = data;
|
frame.response = data;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
@ -1,8 +1,9 @@
|
|||||||
const config = require('../../../shared/config');
|
const config = require('../../../shared/config');
|
||||||
|
const storage = require('../../adapters/storage');
|
||||||
const externalRequest = require('../../lib/request-external');
|
const externalRequest = require('../../lib/request-external');
|
||||||
|
|
||||||
const OEmbed = require('@tryghost/oembed-service');
|
const OEmbed = require('@tryghost/oembed-service');
|
||||||
const oembed = new OEmbed({config, externalRequest});
|
const oembed = new OEmbed({config, externalRequest, storage});
|
||||||
|
|
||||||
const NFT = require('./NFTOEmbedProvider');
|
const NFT = require('./NFTOEmbedProvider');
|
||||||
const nft = new NFT({
|
const nft = new NFT({
|
||||||
|
@ -6,6 +6,8 @@ const testUtils = require('../../utils/index');
|
|||||||
const config = require('../../../core/shared/config/index');
|
const config = require('../../../core/shared/config/index');
|
||||||
const localUtils = require('./utils');
|
const localUtils = require('./utils');
|
||||||
const {mockManager} = require('../../utils/e2e-framework');
|
const {mockManager} = require('../../utils/e2e-framework');
|
||||||
|
const oembed = require('../../../../core/core/server/services/oembed');
|
||||||
|
const urlUtils = require('../../../core/shared/url-utils');
|
||||||
|
|
||||||
// for sinon stubs
|
// for sinon stubs
|
||||||
const dnsPromises = require('dns').promises;
|
const dnsPromises = require('dns').promises;
|
||||||
@ -19,9 +21,18 @@ describe('Oembed API', function () {
|
|||||||
await localUtils.doAuth(request);
|
await localUtils.doAuth(request);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
let processImageFromUrlStub;
|
||||||
|
|
||||||
beforeEach(function () {
|
beforeEach(function () {
|
||||||
// ensure sure we're not network dependent
|
// ensure sure we're not network dependent
|
||||||
mockManager.disableNetwork();
|
mockManager.disableNetwork();
|
||||||
|
processImageFromUrlStub = sinon.stub(oembed, 'processImageFromUrl');
|
||||||
|
processImageFromUrlStub.callsFake(async function (imageUrl, imageType) {
|
||||||
|
if (imageUrl === 'http://example.com/bad-image') {
|
||||||
|
throw new Error('Failed to process image');
|
||||||
|
}
|
||||||
|
return `/content/images/${imageType}/image-01.png`;
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(function () {
|
afterEach(function () {
|
||||||
@ -228,15 +239,10 @@ describe('Oembed API', function () {
|
|||||||
.get('/page-with-icon')
|
.get('/page-with-icon')
|
||||||
.reply(
|
.reply(
|
||||||
200,
|
200,
|
||||||
'<html><head><title>TESTING</title><link rel="icon" href="http://example.com/icon.svg"></head><body></body></html>',
|
'<html><head><title>TESTING</title><link rel="icon" href="http://example.com/bad-image"></head><body></body></html>',
|
||||||
{'content-type': 'text/html'}
|
{'content-type': 'text/html'}
|
||||||
);
|
);
|
||||||
|
|
||||||
// Mock the icon URL to return 404
|
|
||||||
nock('http://example.com/')
|
|
||||||
.head('/icon.svg')
|
|
||||||
.reply(404);
|
|
||||||
|
|
||||||
const url = encodeURIComponent(' http://example.com/page-with-icon\t '); // Whitespaces are to make sure urls are trimmed
|
const url = encodeURIComponent(' http://example.com/page-with-icon\t '); // Whitespaces are to make sure urls are trimmed
|
||||||
const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
|
const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
|
||||||
.set('Origin', config.get('url'))
|
.set('Origin', config.get('url'))
|
||||||
@ -252,6 +258,54 @@ describe('Oembed API', function () {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should fetch and store icons', async function () {
|
||||||
|
// Mock the page to contain a readable icon URL
|
||||||
|
const pageMock = nock('http://example.com')
|
||||||
|
.get('/page-with-icon')
|
||||||
|
.reply(
|
||||||
|
200,
|
||||||
|
'<html><head><title>TESTING</title><link rel="icon" href="http://example.com/icon.svg"></head><body></body></html>',
|
||||||
|
{'content-type': 'text/html'}
|
||||||
|
);
|
||||||
|
|
||||||
|
const url = encodeURIComponent(' http://example.com/page-with-icon\t '); // Whitespaces are to make sure urls are trimmed
|
||||||
|
const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
|
||||||
|
.set('Origin', config.get('url'))
|
||||||
|
.expect('Content-Type', /json/)
|
||||||
|
.expect('Cache-Control', testUtils.cacheRules.private)
|
||||||
|
.expect(200);
|
||||||
|
|
||||||
|
// Check that the icon URL mock was loaded
|
||||||
|
pageMock.isDone().should.be.true();
|
||||||
|
|
||||||
|
// Check that the substitute icon URL is returned in place of the original
|
||||||
|
res.body.metadata.icon.should.eql(`${urlUtils.urlFor('home', true)}content/images/icon/image-01.png`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should fetch and store thumbnails', async function () {
|
||||||
|
// Mock the page to contain a readable icon URL
|
||||||
|
const pageMock = nock('http://example.com')
|
||||||
|
.get('/page-with-thumbnail')
|
||||||
|
.reply(
|
||||||
|
200,
|
||||||
|
'<html><head><title>TESTING</title><link rel="thumbnail" href="http://example.com/thumbnail.svg"></head><body></body></html>',
|
||||||
|
{'content-type': 'text/html'}
|
||||||
|
);
|
||||||
|
|
||||||
|
const url = encodeURIComponent(' http://example.com/page-with-thumbnail\t '); // Whitespaces are to make sure urls are trimmed
|
||||||
|
const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
|
||||||
|
.set('Origin', config.get('url'))
|
||||||
|
.expect('Content-Type', /json/)
|
||||||
|
.expect('Cache-Control', testUtils.cacheRules.private)
|
||||||
|
.expect(200);
|
||||||
|
|
||||||
|
// Check that the thumbnail URL mock was loaded
|
||||||
|
pageMock.isDone().should.be.true();
|
||||||
|
|
||||||
|
// Check that the substitute thumbnail URL is returned in place of the original
|
||||||
|
res.body.metadata.thumbnail.should.eql(`${urlUtils.urlFor('home', true)}content/images/thumbnail/image-01.png`);
|
||||||
|
});
|
||||||
|
|
||||||
describe('with unknown provider', function () {
|
describe('with unknown provider', function () {
|
||||||
it('fetches url and follows redirects', async function () {
|
it('fetches url and follows redirects', async function () {
|
||||||
const redirectMock = nock('http://test.com/')
|
const redirectMock = nock('http://test.com/')
|
||||||
|
@ -4,6 +4,7 @@ const logging = require('@tryghost/logging');
|
|||||||
const _ = require('lodash');
|
const _ = require('lodash');
|
||||||
const charset = require('charset');
|
const charset = require('charset');
|
||||||
const iconv = require('iconv-lite');
|
const iconv = require('iconv-lite');
|
||||||
|
const path = require('path');
|
||||||
|
|
||||||
// Some sites block non-standard user agents so we need to mimic a typical browser
|
// Some sites block non-standard user agents so we need to mimic a typical browser
|
||||||
const USER_AGENT = 'Mozilla/5.0 (compatible; Ghost/5.0; +https://ghost.org/)';
|
const USER_AGENT = 'Mozilla/5.0 (compatible; Ghost/5.0; +https://ghost.org/)';
|
||||||
@ -49,6 +50,12 @@ const findUrlWithProvider = (url) => {
|
|||||||
/**
|
/**
|
||||||
* @typedef {Object} IConfig
|
* @typedef {Object} IConfig
|
||||||
* @prop {(key: string) => string} get
|
* @prop {(key: string) => string} get
|
||||||
|
* @prop {(key: string) => string} getContentPath
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @typedef {Object} IStorage
|
||||||
|
* @prop {(feature: string) => Object} getStorage
|
||||||
*/
|
*/
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -66,10 +73,12 @@ class OEmbedService {
|
|||||||
*
|
*
|
||||||
* @param {Object} dependencies
|
* @param {Object} dependencies
|
||||||
* @param {IConfig} dependencies.config
|
* @param {IConfig} dependencies.config
|
||||||
|
* @param {IStorage} dependencies.storage
|
||||||
* @param {IExternalRequest} dependencies.externalRequest
|
* @param {IExternalRequest} dependencies.externalRequest
|
||||||
*/
|
*/
|
||||||
constructor({config, externalRequest}) {
|
constructor({config, externalRequest, storage}) {
|
||||||
this.config = config;
|
this.config = config;
|
||||||
|
this.storage = storage;
|
||||||
|
|
||||||
/** @type {IExternalRequest} */
|
/** @type {IExternalRequest} */
|
||||||
this.externalRequest = externalRequest;
|
this.externalRequest = externalRequest;
|
||||||
@ -118,6 +127,55 @@ class OEmbedService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetches the image buffer from a URL using fetch
|
||||||
|
* @param {String} imageUrl - URL of the image to fetch
|
||||||
|
* @returns {Promise<Buffer>} - Promise resolving to the image buffer
|
||||||
|
*/
|
||||||
|
async fetchImageBuffer(imageUrl) {
|
||||||
|
const response = await fetch(imageUrl);
|
||||||
|
|
||||||
|
if (!response.ok) {
|
||||||
|
throw Error(`Failed to fetch image: ${response.statusText}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
const arrayBuffer = await response.arrayBuffer();
|
||||||
|
|
||||||
|
const buffer = Buffer.from(arrayBuffer);
|
||||||
|
return buffer;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Process and store image from a URL
|
||||||
|
* @param {String} imageUrl - URL of the image to process
|
||||||
|
* @param {String} imageType - What is the image used for. Example - icon, thumbnail
|
||||||
|
* @returns {Promise<String>} - URL where the image is stored
|
||||||
|
*/
|
||||||
|
async processImageFromUrl(imageUrl, imageType) {
|
||||||
|
// Fetch image buffer from the URL
|
||||||
|
const imageBuffer = await this.fetchImageBuffer(imageUrl);
|
||||||
|
const store = this.storage.getStorage('images');
|
||||||
|
|
||||||
|
// Extract file name from URL
|
||||||
|
const fileName = path.basename(new URL(imageUrl).pathname);
|
||||||
|
let ext = path.extname(fileName);
|
||||||
|
let name;
|
||||||
|
|
||||||
|
if (ext) {
|
||||||
|
name = store.getSanitizedFileName(path.basename(fileName, ext));
|
||||||
|
} else {
|
||||||
|
name = store.getSanitizedFileName(path.basename(fileName));
|
||||||
|
}
|
||||||
|
|
||||||
|
let targetDir = path.join(this.config.getContentPath('images'), imageType);
|
||||||
|
const uniqueFilePath = await store.generateUnique(targetDir, name, ext, 0);
|
||||||
|
const targetPath = path.join(imageType, path.basename(uniqueFilePath));
|
||||||
|
|
||||||
|
const imageStoredUrl = await store.saveRaw(imageBuffer, targetPath);
|
||||||
|
|
||||||
|
return imageStoredUrl;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param {string} url
|
* @param {string} url
|
||||||
* @param {Object} options
|
* @param {Object} options
|
||||||
@ -209,7 +267,7 @@ class OEmbedService {
|
|||||||
*
|
*
|
||||||
* @returns {Promise<Object>}
|
* @returns {Promise<Object>}
|
||||||
*/
|
*/
|
||||||
async fetchBookmarkData(url, html) {
|
async fetchBookmarkData(url, html, type) {
|
||||||
const gotOpts = {
|
const gotOpts = {
|
||||||
headers: {
|
headers: {
|
||||||
'User-Agent': USER_AGENT
|
'User-Agent': USER_AGENT
|
||||||
@ -271,13 +329,29 @@ class OEmbedService {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
if (metadata.icon) {
|
if (type === 'mention') {
|
||||||
try {
|
if (metadata.icon) {
|
||||||
await this.externalRequest.head(metadata.icon);
|
try {
|
||||||
} catch (err) {
|
await this.externalRequest.head(metadata.icon);
|
||||||
metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';
|
} catch (err) {
|
||||||
logging.error(err);
|
metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';
|
||||||
|
logging.error(err);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
await this.processImageFromUrl(metadata.icon, 'icon')
|
||||||
|
.then((processedImageUrl) => {
|
||||||
|
metadata.icon = processedImageUrl;
|
||||||
|
}).catch((err) => {
|
||||||
|
metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';
|
||||||
|
logging.error(err);
|
||||||
|
});
|
||||||
|
await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
|
||||||
|
.then((processedImageUrl) => {
|
||||||
|
metadata.thumbnail = processedImageUrl;
|
||||||
|
}).catch((err) => {
|
||||||
|
logging.error(err);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@ -412,7 +486,7 @@ class OEmbedService {
|
|||||||
|
|
||||||
// fetch only bookmark when explicitly requested
|
// fetch only bookmark when explicitly requested
|
||||||
if (type === 'bookmark') {
|
if (type === 'bookmark') {
|
||||||
return this.fetchBookmarkData(url, body);
|
return this.fetchBookmarkData(url, body, type);
|
||||||
}
|
}
|
||||||
|
|
||||||
// mentions need to return bookmark data (metadata) and body (html) for link verification
|
// mentions need to return bookmark data (metadata) and body (html) for link verification
|
||||||
@ -435,7 +509,7 @@ class OEmbedService {
|
|||||||
};
|
};
|
||||||
return {...bookmark, body};
|
return {...bookmark, body};
|
||||||
}
|
}
|
||||||
const bookmark = await this.fetchBookmarkData(url, body);
|
const bookmark = await this.fetchBookmarkData(url, body, type);
|
||||||
return {...bookmark, body, contentType};
|
return {...bookmark, body, contentType};
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -454,7 +528,7 @@ class OEmbedService {
|
|||||||
|
|
||||||
// fallback to bookmark when we can't get oembed
|
// fallback to bookmark when we can't get oembed
|
||||||
if (!data && !type) {
|
if (!data && !type) {
|
||||||
data = await this.fetchBookmarkData(url, body);
|
data = await this.fetchBookmarkData(url, body, type);
|
||||||
}
|
}
|
||||||
|
|
||||||
// couldn't get anything, throw a validation error
|
// couldn't get anything, throw a validation error
|
||||||
|
Loading…
Reference in New Issue
Block a user