Skip to content

Commit

Permalink
🐛 Fixed embed service trying http before https for oembed providers (T…
Browse files Browse the repository at this point in the history
…ryGhost#19521)

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
  • Loading branch information
kevinansfield authored Jan 18, 2024
1 parent 7587415 commit 0c5cdbf
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
25 changes: 25 additions & 0 deletions ghost/core/test/e2e-api/admin/oembed.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<iframe></iframe>',
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')
Expand Down
6 changes: 3 additions & 3 deletions ghost/oembed-service/lib/OEmbedService.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 0c5cdbf

Please sign in to comment.