Fixed RPC pings (TryGhost#9816)

closes TryGhost#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.
lasharela · Aug 23, 2018 · 2376c61 · 2376c61
1 parent fcc2ee4
commit 2376c61
Show file tree

Hide file tree

Showing 3 changed files with 133 additions and 23 deletions.
diff --git 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
@@ -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 = /<member>[\s]*<name>flerror<\/name>[\s]*<value>[\s]*<boolean>0<\/boolean><\/value><\/member>/;
+        const errorMessage = /<name>(?:faultString|message)<\/name>[\s]*<value>[\s]*<string>([^<]+)/;
+
         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
@@ -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,54 +89,159 @@ 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';
 
             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}});
 
             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,
+                `<?xml version="1.0"?>
+                 <methodResponse>
+                   <params>
+                     <param>
+                       <value>
+                         <struct>
+                           <member>
+                             <name>flerror</name>
+                             <value>
+                               <boolean>1</boolean>
+                             </value>
+                           </member>
+                           <member>
+                             <name>message</name>
+                             <value>
+                              <string>Uh oh. A wee lil error.</string>
+                             </value>
+                           </member>
+                         </struct>
+                       </value>
+                     </param>
+                   </params>
+                 </methodResponse>`),
+                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,
+                (`<?xml version="1.0"?>
+                 <methodResponse>
+                   <params>
+                     <param>
+                       <value>
+                         <struct>
+                           <member>
+                             <name>flerror</name>
+                             <value>
+                               <boolean>1</boolean>
+                             </value>
+                           </member>
+                           <member>
+                             <name>message</name>
+                             <value>
+                              <string>Uh oh. A wee lil error.</string>
+                             </value>
+                           </member>
+                         </struct>
+                       </value>
+                     </param>
+                   </params>
+                 </methodResponse>`).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,
+                `<?xml version="1.0"?>
+                    <methodResponse>
+                      <params>
+                        <param>
+                          <value>
+                            <struct>
+                              <member><name>flerror</name><value><boolean>0</boolean></value></member>
+                              <member><name>message</name><value><string>Pings being forwarded to 9 services!</string></value></member>
+                    </struct>
+                          </value>
+                        </param>
+                      </params>
+                    </methodResponse>`),
+                loggingStub = sandbox.stub(common.logging, 'error');
+
+            ping(testPost);
+
+            (function retry() {
+                if (ping1.isDone()) {
+                    loggingStub.calledOnce.should.eql(false);
                     return done();
                 }