handle when the diaspora xml parser returns nil

closes #5991

Changelog.md

@@ -17,6 +17,7 @@
 * Ensure posts have an author [#5986](https://github.com/diaspora/diaspora/pull/5986)
 * Improve the logging messages of Sidekiq messages [#5988](https://github.com/diaspora/diaspora/pull/5988)
 * Improve the logging of Eyes output [#5989](https://github.com/diaspora/diaspora/pull/5989)
+* Gracefully handle XML parse errors within federation [#5991](https://github.com/diaspora/diaspora/pull/5991)
 
 ## Bug fixes
 * Disable auto follow back on aspect deletion [#5846](https://github.com/diaspora/diaspora/pull/5846)

app/workers/base.rb

@@ -17,7 +17,8 @@ module Workers
            Diaspora::AuthorXMLAuthorMismatch,
            # We received a private object to our public endpoint, again something
            # Friendica seems to provoke
-           Diaspora::NonPublic => e
+           Diaspora::NonPublic,
+           Diaspora::XMLNotParseable => e
       Rails.logger.info("error on receive: #{e.class}")
     rescue ActiveRecord::RecordInvalid => e
       Rails.logger.info("failed to save received object: #{e.record.errors.full_messages}")

lib/diaspora/exceptions.rb

@@ -35,4 +35,7 @@ module Diaspora
   class PostNotFetchable < StandardError
   end
 
+  # Error while parsing an received message and got nil
+  class XMLNotParseable < StandardError
+  end
 end

lib/diaspora/parser.rb

@@ -5,15 +5,15 @@
 module Diaspora
   module Parser
     def self.from_xml(xml)
-      doc = Nokogiri::XML(xml) { |cfg| cfg.noblanks }
+      doc = Nokogiri::XML(xml) {|cfg| cfg.noblanks }
       return unless body = doc.xpath("/XML/post").children.first
-      class_name = body.name.gsub('-', '/')
+      class_name = body.name.gsub("-", "/")
       begin
         class_name.camelize.constantize.from_xml body.to_s
       rescue NameError => e
         # A pods is trying to federate an object we don't recognize.
-        # i.e. their codebase is different from ours.  Quietly discard
+        # i.e. their codebase is different from ours.
-        # so that no job failure is created
+        ::Logging::Logger[self].warn("Error while parsing the xml: #{e.message}")
         nil
       end
     end

lib/postzord/receiver/public.rb

@@ -55,11 +55,12 @@ class Postzord::Receiver::Public < Postzord::Receiver
 
   # @return [Object]
   def save_object
-    @object = Diaspora::Parser::from_xml(@salmon.parsed_data)
+    @object = Diaspora::Parser.from_xml(@salmon.parsed_data)
+    raise Diaspora::XMLNotParseable if @object.nil?
     raise Diaspora::NonPublic if object_can_be_public_and_it_is_not?
     raise Diaspora::RelayableObjectWithoutParent if object_must_have_parent_and_does_not?
     raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
-    @object.save! if @object && @object.respond_to?(:save!)
+    @object.save! if @object.respond_to?(:save!)
     @object
   end
 
@@ -71,7 +72,7 @@ class Postzord::Receiver::Public < Postzord::Receiver
   def xml_author
     if @object.respond_to?(:relayable?)
       #this is public, so it would only be owners sending us other people comments etc
-       @object.parent_author.local? ? @object.diaspora_handle : @object.parent_diaspora_handle
+      @object.parent_author.local? ? @object.diaspora_handle : @object.parent_diaspora_handle
     else
       @object.diaspora_handle
     end

spec/lib/postzord/receiver/public_spec.rb

@@ -118,4 +118,15 @@ describe Postzord::Receiver::Public do
       @receiver.receive_relayable
     end
   end
+
+  describe "#save_object" do
+    before do
+      @receiver = Postzord::Receiver::Public.new(@xml)
+    end
+
+    it "should raise a Diaspora::XMLNotParseable when the parsed object is nil" do
+      expect(Diaspora::Parser).to receive(:from_xml).and_return(nil)
+      expect { @receiver.save_object }.to raise_error(Diaspora::XMLNotParseable)
+    end
+  end
 end