From 2376c614b3b6913224bc78054da9757ab88b3f03 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 23 Aug 2018 21:20:29 +0800 Subject: [PATCH] Fixed RPC pings (#9816) closes #9644 - Removed google blogsearch from pingList, because it's dead (https://plus.google.com/+GoogleWebmasters/posts/46W7ZwVrwqg) - Updated pingomatic method weblogUpdate -> weblogUpdates - Updated RPC response handler to check for errors, log improvement - the pingomatic service sends back a 200 even when it errors, so we check the xml response to see if it's good, it throws and passes of to the catch handler already in place - Avoid multiline XML error message strings, but includes a catch in case our regex stops working with a fallback to multiline XML error message - we check for arbitrary whitespace between different XML tags, which I haven't seen in any of the responses - but it could change I guess. I haven't added support of whitespace between the tags, as I believe that would be a different value with XML spec. And we wanna match on exact values here. --- PRIVACY.md | 3 +- core/server/services/xmlrpc.js | 15 ++- core/test/unit/services/xmlrpc_spec.js | 138 ++++++++++++++++++++++--- 3 files changed, 133 insertions(+), 23 deletions(-) diff --git a/PRIVACY.md b/PRIVACY.md index aca6f93e12..0e57cd069d 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -28,9 +28,8 @@ To automatically populate your profile picture, Ghost pings [Gravatar](http://gr ### RPC Pings -When you publish a new post, Ghost sends out an RPC ping to let third party services know that new content is available on your blog. This enables search engines and other services to discover and index content on your blog more quickly. At present Ghost sends an RPC ping to the following services when you publish a new post: +When you publish a new post, Ghost sends out an RPC ping to let third party services know that new content is available on your blog. This enables search engines and other services to discover and index content on your blog more quickly. At present Ghost sends an RPC ping to the following service when you publish a new post: -- http://blogsearch.google.com - http://rpc.pingomatic.com RPC pings only happen when Ghost is running in the `production` environment. diff --git a/core/server/services/xmlrpc.js b/core/server/services/xmlrpc.js index 49a857fde0..71b7dffae4 100644 --- a/core/server/services/xmlrpc.js +++ b/core/server/services/xmlrpc.js @@ -17,9 +17,6 @@ var _ = require('lodash'), ], // ToDo: Make this configurable pingList = [ - { - url: 'blogsearch.google.com/ping/RPC2' - }, { url: 'rpc.pingomatic.com' } @@ -45,7 +42,7 @@ function ping(post) { // Build XML object. pingXML = xml({ methodCall: [{ - methodName: 'weblogUpdate.ping' + methodName: 'weblogUpdates.ping' }, { params: [{ param: [{ @@ -70,7 +67,17 @@ function ping(post) { timeout: 2 * 1000 }; + const goodResponse = /[\s]*flerror<\/name>[\s]*[\s]*0<\/boolean><\/value><\/member>/; + const errorMessage = /(?:faultString|message)<\/name>[\s]*[\s]*([^<]+)/; + request(pingHost.url, options) + .then(function (res) { + if (!goodResponse.test(res.body)) { + const matches = res.body.match(errorMessage); + const message = matches ? matches[1] : res.body; + throw new Error(message); + } + }) .catch(function (err) { common.logging.error(new common.errors.GhostError({ err: err, diff --git a/core/test/unit/services/xmlrpc_spec.js b/core/test/unit/services/xmlrpc_spec.js index d8fb147253..b9d058c2c7 100644 --- a/core/test/unit/services/xmlrpc_spec.js +++ b/core/test/unit/services/xmlrpc_spec.js @@ -74,14 +74,13 @@ describe('XMLRPC', function () { var ping = xmlrpc.__get__('ping'); it('with a post should execute two pings', function (done) { - var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200), - ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200), + var ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200), testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); ping(testPost); (function retry() { - if (ping1.isDone() && ping2.isDone()) { + if (ping1.isDone()) { return done(); } @@ -90,8 +89,7 @@ describe('XMLRPC', function () { }); it('with default post should not execute pings', function () { - var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200), - ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200), + var ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200), testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); testPost.slug = 'welcome'; @@ -99,23 +97,19 @@ describe('XMLRPC', function () { ping(testPost); ping1.isDone().should.be.false(); - ping2.isDone().should.be.false(); }); it('with a page should not execute pings', function () { - var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200), - ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200), + var ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200), testPage = _.clone(testUtils.DataGenerator.Content.posts[5]); ping(testPage); ping1.isDone().should.be.false(); - ping2.isDone().should.be.false(); }); it('when privacy.useRpcPing is false should not execute pings', function () { - var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200), - ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200), + var ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200), testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); configUtils.set({privacy: {useRpcPing: false}}); @@ -123,21 +117,131 @@ describe('XMLRPC', function () { ping(testPost); ping1.isDone().should.be.false(); - ping2.isDone().should.be.false(); }); it('captures && logs errors from requests', function (done) { var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]), - ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(500), - ping2 = nock('http://rpc.pingomatic.com').post('/').reply(400), + ping1 = nock('http://rpc.pingomatic.com').post('/').reply(400), loggingStub = sandbox.stub(common.logging, 'error'); ping(testPost); (function retry() { - if (ping1.isDone() && ping2.isDone()) { - loggingStub.calledTwice.should.eql(true); - loggingStub.args[0][0].message.should.containEql('Response code 500'); + if (ping1.isDone()) { + loggingStub.calledOnce.should.eql(true); + loggingStub.args[0][0].message.should.containEql('Response code 400'); + return done(); + } + + setTimeout(retry, 100); + }()); + }); + + it('captures && logs XML errors from requests with newlines between tags', function (done) { + var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]), + ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200, + ` + + + + + + + flerror + + 1 + + + + message + + Uh oh. A wee lil error. + + + + + + + `), + loggingStub = sandbox.stub(common.logging, 'error'); + + ping(testPost); + + (function retry() { + if (ping1.isDone()) { + loggingStub.calledOnce.should.eql(true); + loggingStub.args[0][0].message.should.equal('Uh oh. A wee lil error.'); + return done(); + } + + setTimeout(retry, 100); + }()); + }); + + it('captures && logs XML errors from requests without newlines between tags', function (done) { + var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]), + ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200, + (` + + + + + + + flerror + + 1 + + + + message + + Uh oh. A wee lil error. + + + + + + + `).replace('\n', '')), + loggingStub = sandbox.stub(common.logging, 'error'); + + ping(testPost); + + (function retry() { + if (ping1.isDone()) { + loggingStub.calledOnce.should.eql(true); + loggingStub.args[0][0].message.should.equal('Uh oh. A wee lil error.'); + return done(); + } + + setTimeout(retry, 100); + }()); + }); + + it('does not error with responses that have 0 as flerror value', function (done) { + var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]), + ping1 = nock('http://rpc.pingomatic.com').post('/').reply(200, + ` + + + + + + flerror0 + messagePings being forwarded to 9 services! + + + + + `), + loggingStub = sandbox.stub(common.logging, 'error'); + + ping(testPost); + + (function retry() { + if (ping1.isDone()) { + loggingStub.calledOnce.should.eql(false); return done(); }