diff --git a/Changelog.md b/Changelog.md index 12f90669dbd6a17b2703dfe42345c0a56f5b868b..2b157e1348306a457ab765ccceba793d52a9381a 100644 --- a/Changelog.md +++ b/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) diff --git a/app/workers/base.rb b/app/workers/base.rb index ab506eb2cd194d61bba8af6622cbf0e81c32bf9f..2a614663e7e1fb84760d22411c2d62c2e3ea7027 100644 --- a/app/workers/base.rb +++ b/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}") diff --git a/lib/diaspora/exceptions.rb b/lib/diaspora/exceptions.rb index 2a9da47a2bfa03c5a5dfa22283c12446d9d52480..5e537594e9bf8a31686b88ea55da1fd7678327d2 100644 --- a/lib/diaspora/exceptions.rb +++ b/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 diff --git a/lib/diaspora/parser.rb b/lib/diaspora/parser.rb index 1d2c5009248908d48388bf3e974c339cb9cee4b4..62420f9b87ef0f1a706870c81e0ae757f764e740 100644 --- a/lib/diaspora/parser.rb +++ b/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 - # so that no job failure is created + # i.e. their codebase is different from ours. + ::Logging::Logger[self].warn("Error while parsing the xml: #{e.message}") nil end end diff --git a/lib/postzord/receiver/public.rb b/lib/postzord/receiver/public.rb index 1851b4b98bd2f2fd4977ecc2649d0009ee803041..8c5d51d1f00b725fe4e50f382b5e8cea18837016 100644 --- a/lib/postzord/receiver/public.rb +++ b/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 diff --git a/spec/lib/postzord/receiver/public_spec.rb b/spec/lib/postzord/receiver/public_spec.rb index a1eaab462a9f90804c5c210a6dbb19ea84815d82..afa005bc5d4264d19031ec9b50a19d508f3dce50 100644 --- a/spec/lib/postzord/receiver/public_spec.rb +++ b/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