From 21ae93e6583850cee2ac9581ddcf28cbac9298eb Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Tue, 26 May 2015 04:44:10 +0200
Subject: [PATCH] handle when the diaspora xml parser returns nil
closes #5991
---
Changelog.md | 1 +
app/workers/base.rb | 3 ++-
lib/diaspora/exceptions.rb | 3 +++
lib/diaspora/parser.rb | 8 ++++----
lib/postzord/receiver/public.rb | 7 ++++---
spec/lib/postzord/receiver/public_spec.rb | 11 +++++++++++
6 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/Changelog.md b/Changelog.md
index 12f90669db..2b157e1348 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 ab506eb2cd..2a614663e7 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 2a9da47a2b..5e537594e9 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 1d2c500924..62420f9b87 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 1851b4b98b..8c5d51d1f0 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 a1eaab462a..afa005bc5d 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
--
GitLab