Skip to content

Commit

Permalink
🐛 Fixed bookmark card hot linking icons and thumbnails (#20923)
Browse files Browse the repository at this point in the history 
Ref:
https://linear.app/tryghost/issue/ENG-904/bookmark-card-hot-linking-favicons
  • Loading branch information
@vershwal
vershwal authored Sep 5, 2024
1 parent 2faa051 commit 426b1d4
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 14 deletions.
3 changes: 2 additions & 1 deletion ghost/core/core/server/services/oembed/service.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
const config = require('../../../shared/config');
const storage = require('../../adapters/storage');
const externalRequest = require('../../lib/request-external');

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 = new NFT({
Expand Down
65 changes: 59 additions & 6 deletions ghost/core/test/e2e-api/admin/oembed.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const testUtils = require('../../utils/index');
const config = require('../../../core/shared/config/index');
const localUtils = require('./utils');
const {mockManager} = require('../../utils/e2e-framework');
const oembed = require('../../../../core/core/server/services/oembed');

// for sinon stubs
const dnsPromises = require('dns').promises;
Expand All @@ -19,9 +20,18 @@ describe('Oembed API', function () {
await localUtils.doAuth(request);
});

let processImageFromUrlStub;

beforeEach(function () {
// ensure sure we're not network dependent
mockManager.disableNetwork();
processImageFromUrlStub = sinon.stub(oembed, 'processImageFromUrl');
processImageFromUrlStub.callsFake(async function (imageUrl) {
if (imageUrl === 'http://example.com/bad-image') {
throw new Error('Failed to process image');
}
return '/content/images/image-01.png';
});
});

afterEach(function () {
Expand Down Expand Up @@ -228,15 +238,10 @@ describe('Oembed API', function () {
.get('/page-with-icon')
.reply(
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'}
);

// 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 res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`))
.set('Origin', config.get('url'))
Expand All @@ -252,6 +257,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('/content/images/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('/content/images/image-01.png');
});

describe('with unknown provider', function () {
it('fetches url and follows redirects', async function () {
const redirectMock = nock('http://test.com/')
Expand Down
66 changes: 59 additions & 7 deletions ghost/oembed-service/lib/OEmbedService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const logging = require('@tryghost/logging');
const _ = require('lodash');
const charset = require('charset');
const iconv = require('iconv-lite');
const path = require('path');

// 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/)';
Expand Down Expand Up @@ -51,6 +52,11 @@ const findUrlWithProvider = (url) => {
* @prop {(key: string) => string} get
*/

/**
* @typedef {Object} IStorage
* @prop {(feature: string) => Object} getStorage
*/

/**
* @typedef {(url: string, config: Object) => Promise} IExternalRequest
*/
Expand All @@ -66,10 +72,12 @@ class OEmbedService {
*
* @param {Object} dependencies
* @param {IConfig} dependencies.config
* @param {IStorage} dependencies.storage
* @param {IExternalRequest} dependencies.externalRequest
*/
constructor({config, externalRequest}) {
constructor({config, externalRequest, storage}) {
this.config = config;
this.storage = storage;

/** @type {IExternalRequest} */
this.externalRequest = externalRequest;
Expand Down Expand Up @@ -118,6 +126,44 @@ 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);

// Extract file name from URL
const fileName = path.basename(new URL(imageUrl).pathname);

const targetPath = path.join(imageType, fileName);

const imageStoredUrl = await this.storage.getStorage('images').saveRaw(imageBuffer, targetPath);

return imageStoredUrl;
}

/**
* @param {string} url
* @param {Object} options
Expand Down Expand Up @@ -271,14 +317,20 @@ class OEmbedService {
});
}

if (metadata.icon) {
try {
await this.externalRequest.head(metadata.icon);
} catch (err) {
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 {
version: '1.0',
Expand Down

0 comments on commit 426b1d4

Please sign in to comment.