From 0c5cdbf4d23bc419d1b1f757e6aa3f56fb627076 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 18 Jan 2024 14:42:28 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20embed=20service=20trying?= =?UTF-8?q?=20http=20before=20https=20for=20oembed=20providers=20(#19521)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - issue reported via the forum https://forum.ghost.org/t/video-embed-break-page-on-mobile/44172 - due to historical issues we check against http/https and non-www/www URLs to match an oembed provider in case our library's provider list is out of date. However we checked http first which could match and then update the original URL to be `http` in place of `https` leading to potentially broken oembed fetch requests as was the case with http://odysee.com URLs --- ghost/core/test/e2e-api/admin/oembed.test.js | 25 ++++++++++++++++++++ ghost/oembed-service/lib/OEmbedService.js | 6 ++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ghost/core/test/e2e-api/admin/oembed.test.js b/ghost/core/test/e2e-api/admin/oembed.test.js index 72dbaa6f75..7f0b9920e5 100644 --- a/ghost/core/test/e2e-api/admin/oembed.test.js +++ b/ghost/core/test/e2e-api/admin/oembed.test.js @@ -58,6 +58,31 @@ describe('Oembed API', function () { should.exist(res.body.html); }); + it('does not use http preferentially to https', async function () { + const httpMock = nock('https://odysee.com') + .get('/$/oembed') + .query({url: 'http://odysee.com/@BarnabasNagy:5/At-Last-(Playa):2', format: 'json'}) + .reply(200, 'The URL is invalid or is not associated with any claim.'); + + const httpsMock = nock('https://odysee.com') + .get('/$/oembed') + .query({url: 'https://odysee.com/@BarnabasNagy:5/At-Last-(Playa):2', format: 'json'}) + .reply(200, { + html: '', + type: 'rich', + version: '1.0' + }); + + const res = await request.get(localUtils.API.getApiQuery('oembed/?url=https%3A%2F%2Fodysee.com%2F%40BarnabasNagy%3A5%2FAt-Last-%28Playa%29%3A2')) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private); + + httpMock.isDone().should.be.false(); + httpsMock.isDone().should.be.true(); + should.exist(res.body.html); + }); + it('errors with a useful message when embedding is disabled', async function () { const requestMock = nock('https://www.youtube.com') .get('/oembed') diff --git a/ghost/oembed-service/lib/OEmbedService.js b/ghost/oembed-service/lib/OEmbedService.js index 064069ae18..fedf30f5e2 100644 --- a/ghost/oembed-service/lib/OEmbedService.js +++ b/ghost/oembed-service/lib/OEmbedService.js @@ -26,10 +26,10 @@ const findUrlWithProvider = (url) => { // providers list is not always up to date with scheme or www vs non-www let baseUrl = url.replace(/^\/\/|^https?:\/\/(?:www\.)?/, ''); let testUrls = [ - `http://${baseUrl}`, `https://${baseUrl}`, - `http://www.${baseUrl}`, - `https://www.${baseUrl}` + `https://www.${baseUrl}`, + `http://${baseUrl}`, + `http://www.${baseUrl}` ]; for (let testUrl of testUrls) {