From 9cbeb74db09b05ab2515f103218d423ed1fed74e Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 6 Oct 2020 08:44:03 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20broken=20embeds=20cards?= =?UTF-8?q?=20when=20pasting=20links=20to=20Wordpress=20sites=20(#12262)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://github.com/TryGhost/Ghost/issues/12260 - if a card type was not explicitly chosen (i.e. a url was pasted into the editor) then abort fetching the oembed endpoint if we detect it's a `wp-json` oembed and return a bookmark card payload instead - cleaned up an unused argument in the internal `fetchBookmarkData()` method --- core/server/api/canary/oembed.js | 18 ++++++++----- test/api-acceptance/admin/oembed_spec.js | 33 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/core/server/api/canary/oembed.js b/core/server/api/canary/oembed.js index 09605b0d51..9eff037849 100644 --- a/core/server/api/canary/oembed.js +++ b/core/server/api/canary/oembed.js @@ -8,7 +8,7 @@ const config = require('../../../shared/config'); const {i18n} = require('../../lib/common'); const externalRequest = require('../../lib/request-external'); -async function fetchBookmarkData(url, html) { +async function fetchBookmarkData(url) { const metascraper = require('metascraper')([ require('metascraper-url')(), require('metascraper-title')(), @@ -23,11 +23,9 @@ async function fetchBookmarkData(url, html) { let scraperResponse; try { - if (!html) { - const cookieJar = new CookieJar(); - const response = await externalRequest(url, {cookieJar}); - html = response.body; - } + const cookieJar = new CookieJar(); + const response = await externalRequest(url, {cookieJar}); + const html = response.body; scraperResponse = await metascraper({html, url}); } catch (err) { return Promise.reject(err); @@ -118,7 +116,7 @@ function isIpOrLocalhost(url) { } } -function fetchOembedData(_url) { +function fetchOembedData(_url, cardType) { // parse the url then validate the protocol and host to make sure it's // http(s) and not an IP address or localhost to avoid potential access to // internal network endpoints @@ -163,6 +161,12 @@ function fetchOembedData(_url) { return unknownProvider(oembedUrl); } + // for standard WP oembed's we want to insert a bookmark card rather than their blockquote+script + // which breaks in the editor and most Ghost themes. Only fallback if card type was not explicitly chosen + if (!cardType && oembedUrl.match(/wp-json\/oembed/)) { + return; + } + // fetch oembed response from embedded rel="alternate" url return externalRequest(oembedUrl, { method: 'GET', diff --git a/test/api-acceptance/admin/oembed_spec.js b/test/api-acceptance/admin/oembed_spec.js index bcf9db5c71..2d4c4ad491 100644 --- a/test/api-acceptance/admin/oembed_spec.js +++ b/test/api-acceptance/admin/oembed_spec.js @@ -598,5 +598,38 @@ describe('Oembed API', function () { done(); }); }); + + it('falls back to bookmark card for WP oembeds', function (done) { + const pageMock = nock('http://test.com') + .get('/') + .twice() // oembed fetch then bookmark fetch + .reply( + 200, + 'TESTING', + {'content-type': 'text/html'} + ); + + const oembedMock = nock('http://test.com') + .get('/wp-json/oembed/embed') + .reply(200, { + version: '1.0', + type: 'link' + }); + + const url = encodeURIComponent('http://test.com'); + request.get(localUtils.API.getApiQuery(`oembed/?url=${url}`)) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + pageMock.isDone().should.be.true(); + oembedMock.isDone().should.be.false(); + done(); + }); + }); }); });