diff options
| -rw-r--r-- | Gemfile | 4 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | app/models/reshare.rb | 33 | ||||
| -rw-r--r-- | app/workers/deferred_dispatch.rb | 1 | ||||
| -rw-r--r-- | app/workers/process_photo.rb | 1 | ||||
| -rw-r--r-- | app/workers/receive_local_batch.rb | 1 | ||||
| -rw-r--r-- | config/initializers/faraday.rb | 7 | ||||
| -rw-r--r-- | lib/diaspora/exceptions.rb | 7 | ||||
| -rw-r--r-- | lib/diaspora/federated/relayable_retraction.rb | 4 | ||||
| -rw-r--r-- | lib/diaspora/fetcher.rb | 1 | ||||
| -rw-r--r-- | lib/diaspora/fetcher/single.rb | 41 | ||||
| -rw-r--r-- | lib/diaspora/relayable.rb | 17 | ||||
| -rw-r--r-- | lib/federated/relayable.rb | 6 | ||||
| -rw-r--r-- | lib/postzord/receiver/public.rb | 19 | ||||
| -rw-r--r-- | lib/webfinger.rb | 4 | ||||
| -rw-r--r-- | spec/lib/diaspora/federated/relayable_retraction_spec.rb | 7 | ||||
| -rw-r--r-- | spec/lib/diaspora/fetcher/public_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/webfinger_spec.rb | 13 | ||||
| -rw-r--r-- | spec/models/comment_spec.rb | 7 | ||||
| -rw-r--r-- | spec/models/reshare_spec.rb | 13 | ||||
| -rw-r--r-- | spec/workers/deferred_dispatch_spec.rb | 9 | ||||
| -rw-r--r-- | spec/workers/process_photo_spec.rb | 8 |
22 files changed, 162 insertions, 47 deletions
@@ -188,6 +188,10 @@ group :development do gem 'guard-spork', '1.5.1' gem 'spork', '1.0.0rc4' + + # Debugging + gem 'pry' + gem 'pry-debundle' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 939351764..2eeb0e039 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -317,6 +317,8 @@ GEM coderay (~> 1.1.0) method_source (~> 0.8.1) slop (~> 3.4) + pry-debundle (0.8) + pry rack (1.5.2) rack-cors (0.2.9) rack-google-analytics (1.2.0) @@ -551,6 +553,8 @@ DEPENDENCIES omniauth-twitter (= 1.0.1) omniauth-wordpress (= 0.2.1) opengraph_parser (= 0.2.3) + pry + pry-debundle rack-cors (= 0.2.9) rack-google-analytics (= 1.2.0) rack-piwik (= 0.3.0) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 5bb5326d5..869d73351 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -62,7 +62,7 @@ class Reshare < Post end def notification_type(user, person) - Notifications::Reshared if root.author == user.person + Notifications::Reshared if root.try(:author) == user.person end def absolute_root @@ -74,35 +74,16 @@ class Reshare < Post private def after_parse - root_author = Webfinger.new(@root_diaspora_id).fetch - root_author.save! unless root_author.persisted? - - return if Post.exists?(:guid => self.root_guid) - - fetched_post = self.class.fetch_post(root_author, self.root_guid) - - if fetched_post - #Why are we checking for this? - if root_author.diaspora_handle != fetched_post.diaspora_handle - raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" + if root.blank? + self.root = Diaspora::Fetcher::Single.find_or_fetch_from_remote root_guid, @root_diaspora_id do |fetched_post, author| + # why do we check this? + if fetched_post.diaspora_handle != author.diaspora_handle + raise Diaspora::PostNotFetchable, "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{author.diaspora_handle}) specified in the reshare!" + end end - - fetched_post.save! end end - # Fetch a remote public post, used for receiving reshares of unknown posts - # @param [Person] author the remote post's author - # @param [String] guid the remote post's guid - # @return [Post] an unsaved remote post or false if the post was not found - def self.fetch_post author, guid - url = author.url + "/p/#{guid}.xml" - response = Faraday.get(url) - return false if response.status == 404 # Old pod, friendika - raise "Failed to get #{url}" unless response.success? # Other error, N/A for example - Diaspora::Parser.from_xml(response.body) - end - def root_must_be_public if self.root && !self.root.public errors[:base] << "Only posts which are public may be reshared." diff --git a/app/workers/deferred_dispatch.rb b/app/workers/deferred_dispatch.rb index cbb87581a..46fa894c3 100644 --- a/app/workers/deferred_dispatch.rb +++ b/app/workers/deferred_dispatch.rb @@ -17,6 +17,7 @@ module Workers end Postzord::Dispatcher.build(user, object, opts).post + rescue ActiveRecord::RecordNotFound # The target got deleted before the job was run end end end diff --git a/app/workers/process_photo.rb b/app/workers/process_photo.rb index 00d959962..2fcdd3943 100644 --- a/app/workers/process_photo.rb +++ b/app/workers/process_photo.rb @@ -16,6 +16,7 @@ module Workers photo.processed_image.store!(unprocessed_image) photo.save! + rescue ActiveRecord::RecordNotFound # Deleted before the job was run end end end diff --git a/app/workers/receive_local_batch.rb b/app/workers/receive_local_batch.rb index 81d8a9ecf..adcf312aa 100644 --- a/app/workers/receive_local_batch.rb +++ b/app/workers/receive_local_batch.rb @@ -10,6 +10,7 @@ module Workers object = object_class_string.constantize.find(object_id) receiver = Postzord::Receiver::LocalBatch.new(object, recipient_user_ids) receiver.perform! + rescue ActiveRecord::NotFound # Already deleted before the job could run end end end diff --git a/config/initializers/faraday.rb b/config/initializers/faraday.rb index ad1384f6d..d22e92e35 100644 --- a/config/initializers/faraday.rb +++ b/config/initializers/faraday.rb @@ -1,6 +1,13 @@ # Copyright (c) 2010-2011, Diaspora Inc. This file is # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. + +# Use net_http in test, that's better supported by webmock +unless Rails.env.test? + require 'typhoeus/adapters/faraday' + Faraday.default_adapter = :typhoeus +end + options = { request: { timeout: 25 diff --git a/lib/diaspora/exceptions.rb b/lib/diaspora/exceptions.rb index eba8366e0..2a9da47a2 100644 --- a/lib/diaspora/exceptions.rb +++ b/lib/diaspora/exceptions.rb @@ -16,7 +16,6 @@ module Diaspora # that prevents further execution class NotMine < StandardError end - # Received a message without having a contact class ContactRequiredUnlessRequest < StandardError @@ -30,4 +29,10 @@ module Diaspora # original XML message class AuthorXMLAuthorMismatch < StandardError end + + # Tried to fetch a post but it was deleted, not valid + # or the remote end doesn't support post fetching + class PostNotFetchable < StandardError + end + end diff --git a/lib/diaspora/federated/relayable_retraction.rb b/lib/diaspora/federated/relayable_retraction.rb index ab7be6528..3d48f6763 100644 --- a/lib/diaspora/federated/relayable_retraction.rb +++ b/lib/diaspora/federated/relayable_retraction.rb @@ -60,4 +60,8 @@ class RelayableRetraction < SignedRetraction def parent_author_signature_valid? verify_signature(self.parent_author_signature, self.parent.author) end + + def parent_diaspora_handle + target.author.diaspora_handle + end end diff --git a/lib/diaspora/fetcher.rb b/lib/diaspora/fetcher.rb index adb82e552..a27831e2f 100644 --- a/lib/diaspora/fetcher.rb +++ b/lib/diaspora/fetcher.rb @@ -1,5 +1,6 @@ module Diaspora module Fetcher require 'diaspora/fetcher/public' + require 'diaspora/fetcher/single' end end diff --git a/lib/diaspora/fetcher/single.rb b/lib/diaspora/fetcher/single.rb new file mode 100644 index 000000000..5dfeb7f73 --- /dev/null +++ b/lib/diaspora/fetcher/single.rb @@ -0,0 +1,41 @@ +module Diaspora + module Fetcher + module Single + module_function + + # Fetch and store a remote public post + # @param [String] guid the remote posts guid + # @param [String] author_id Diaspora ID of a user known to have the post, + # preferably the author + # @yield [Post, Person] If a block is given it is yielded the post + # and the author prior save + # @return a saved post + def find_or_fetch_from_remote guid, author_id + post = Post.where(guid: guid).first + return post if post + + post_author = Webfinger.new(author_id).fetch + post_author.save! unless post_author.persisted? + + if fetched_post = fetch_post(post_author, guid) + yield fetched_post, post_author if block_given? + raise Diaspora::PostNotFetchable unless fetched_post.save + end + + fetched_post + end + + # Fetch a remote public post, used for receiving of unknown public posts + # @param [Person] author the remote post's author + # @param [String] guid the remote post's guid + # @return [Post] an unsaved remote post or false if the post was not found + def fetch_post author, guid + url = author.url + "/p/#{guid}.xml" + response = Faraday.get(url) + raise Diaspora::PostNotFetchable if response.status == 404 # Old pod, Friendika, deleted + raise "Failed to get #{url}" unless response.success? # Other error, N/A for example + Diaspora::Parser.from_xml(response.body) + end + end + end +end diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb index d0db312b7..4f43ed402 100644 --- a/lib/diaspora/relayable.rb +++ b/lib/diaspora/relayable.rb @@ -51,7 +51,8 @@ module Diaspora end def parent_guid= new_parent_guid - self.parent = parent_class.where(:guid => new_parent_guid).first + @parent_guid = new_parent_guid + self.parent = parent_class.where(guid: new_parent_guid).first end # @return [Array<Person>] @@ -66,7 +67,7 @@ module Diaspora end def receive(user, person=nil) - comment_or_like = self.class.where(:guid => self.guid).first || self + comment_or_like = self.class.where(guid: self.guid).first || self # Check to make sure the signature of the comment or like comes from the person claiming to author it unless comment_or_like.parent_author == user.person || comment_or_like.verify_parent_author_signature @@ -134,5 +135,17 @@ module Diaspora def parent= parent raise NotImplementedError.new('you must override parent= in order to enable relayable on this model') end + + # ROXML hook ensuring our own hooks are called + def after_parse + if @parent_guid + self.parent ||= fetch_parent(@parent_guid) + end + end + + # Childs should override this to support fetching a missing parent + # @param guid the parents guid + def fetch_parent guid + end end end diff --git a/lib/federated/relayable.rb b/lib/federated/relayable.rb index d852a4dcd..a6cf6bf83 100644 --- a/lib/federated/relayable.rb +++ b/lib/federated/relayable.rb @@ -38,5 +38,9 @@ module Federated def parent= parent self.target = parent end + + def fetch_parent guid + Diaspora::Fetcher::Single.find_or_fetch_from_remote guid, diaspora_handle + end end -end
\ No newline at end of file +end diff --git a/lib/postzord/receiver/public.rb b/lib/postzord/receiver/public.rb index 686e9a5b3..d3f27be77 100644 --- a/lib/postzord/receiver/public.rb +++ b/lib/postzord/receiver/public.rb @@ -26,15 +26,15 @@ class Postzord::Receiver::Public < Postzord::Receiver return false unless save_object FEDERATION_LOGGER.info("received a #{@object.inspect}") - if @object.respond_to?(:relayable?) - receive_relayable - elsif @object.is_a?(AccountDeletion) - #nothing - elsif @object.is_a?(SignedRetraction) # feels like a hack + if @object.is_a?(SignedRetraction) # feels like a hack self.recipient_user_ids.each do |user_id| user = User.where(id: user_id).first @object.perform user if user end + elsif @object.respond_to?(:relayable?) + receive_relayable + elsif @object.is_a?(AccountDeletion) + #nothing else Workers::ReceiveLocalBatch.perform_async(@object.class.to_s, @object.id, self.recipient_user_ids) true @@ -57,6 +57,7 @@ class Postzord::Receiver::Public < Postzord::Receiver def save_object @object = Diaspora::Parser::from_xml(@salmon.parsed_data) raise "Object is not public" 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 @@ -88,4 +89,12 @@ class Postzord::Receiver::Public < Postzord::Receiver def object_can_be_public_and_it_is_not? @object.respond_to?(:public) && !@object.public? end + + def object_must_have_parent_and_does_not? + if @object.respond_to?(:relayable?) # comment, like + @object.parent.nil? + else + false + end + end end diff --git a/lib/webfinger.rb b/lib/webfinger.rb index 93e2cf5f6..385c2b40b 100644 --- a/lib/webfinger.rb +++ b/lib/webfinger.rb @@ -31,7 +31,9 @@ class Webfinger Rails.logger.info("Getting: #{url} for #{account}") begin res = Faraday.get(url) - return false if res.status == 404 + unless res.success? + raise "Failed to fetch #{url}: #{res.status}" + end res.body rescue OpenSSL::SSL::SSLError => e Rails.logger.info "Failed to fetch #{url}: SSL setup invalid" diff --git a/spec/lib/diaspora/federated/relayable_retraction_spec.rb b/spec/lib/diaspora/federated/relayable_retraction_spec.rb index baf88a6eb..6e8635a24 100644 --- a/spec/lib/diaspora/federated/relayable_retraction_spec.rb +++ b/spec/lib/diaspora/federated/relayable_retraction_spec.rb @@ -98,6 +98,13 @@ describe RelayableRetraction do expect(Postzord::Dispatcher).not_to receive(:build) @retraction.receive(@recipient, @remote_raphael) end + + it 'performs through postzord' do + xml = Salmon::Slap.create_by_user_and_activity(@local_luke, @retraction.to_diaspora_xml).xml_for(nil) + expect { + Postzord::Receiver::Public.new(xml).perform! + }.to change(Comment, :count).by(-1) + end end end diff --git a/spec/lib/diaspora/fetcher/public_spec.rb b/spec/lib/diaspora/fetcher/public_spec.rb index 43f249cab..28726b2e4 100644 --- a/spec/lib/diaspora/fetcher/public_spec.rb +++ b/spec/lib/diaspora/fetcher/public_spec.rb @@ -19,7 +19,7 @@ describe Diaspora::Fetcher::Public do stub_request(:get, /remote-testpod.net\/people\/.*/) .with(headers: { - 'Accept'=>'application/json', + 'Accept'=>'application/json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'User-Agent'=>'diaspora-fetcher' }).to_return(:body => @fixture) diff --git a/spec/lib/webfinger_spec.rb b/spec/lib/webfinger_spec.rb index 25104bb37..c3515bebe 100644 --- a/spec/lib/webfinger_spec.rb +++ b/spec/lib/webfinger_spec.rb @@ -36,7 +36,7 @@ describe Webfinger do Webfinger.in_background(account) end end - + describe '#fetch' do it 'works' do finger = Webfinger.new(account_in_fixtures) @@ -72,14 +72,15 @@ describe Webfinger do expect(a_request(:get, redirect_url)).to have_been_made end - - it 'returns false on 404' do + + it 'raises on 404' do url ="https://bar.com/.well-known/host-meta" stub_request(:get, url). to_return(:status => 404, :body => nil) - expect(finger.get(url)).not_to eq(nil) - expect(finger.get(url)).to eq(false) + expect { + expect(finger.get(url)).to eq(false) + }.to raise_error end end @@ -190,7 +191,7 @@ describe Webfinger do expect(Person).to receive(:create_from_webfinger).with("webfinger_profile", "hcard") finger.make_person_from_webfinger end - + it 'with an false xrd it does not call Person.create_from_webfinger' do allow(finger).to receive(:webfinger_profile_xrd).and_return(false) expect(Person).not_to receive(:create_from_webfinger) diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index a377d142a..efc7eacdc 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -96,6 +96,13 @@ describe Comment, :type => :model do it 'marshals the post' do expect(@marshalled_comment.post).to eq(@post) end + + it 'tries to fetch a missing parent' do + guid = @post.guid + @post.destroy + expect_any_instance_of(Comment).to receive(:fetch_parent).with(guid).and_return(nil) + Comment.from_xml(@xml) + end end end diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index 89dfd85f6..929ded2b9 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -75,6 +75,13 @@ describe Reshare, :type => :model do it 'returns "Reshared" for the original post author' do expect(@reshare.notification_type(alice, @reshare.author)).to eq(Notifications::Reshared) end + + it 'does not error out if the root was deleted' do + @reshare.root = nil + expect { + @reshare.notification_type(alice, @reshare.author) + }.to_not raise_error + end end describe '#absolute_root' do @@ -83,7 +90,7 @@ describe Reshare, :type => :model do rs1 = FactoryGirl.build(:reshare, :root=>@sm) rs2 = FactoryGirl.build(:reshare, :root=>rs1) @rs3 = FactoryGirl.build(:reshare, :root=>rs2) - + sm = FactoryGirl.create(:status_message, :author => alice.person, :public => true) rs1 = FactoryGirl.create(:reshare, :root => sm) @of_deleted = FactoryGirl.build(:reshare, :root => rs1) @@ -194,13 +201,13 @@ describe Reshare, :type => :model do end context "fetching post" do - it "doesn't error out if the post is not found" do + it "raises if the post is not found" do allow(@response).to receive(:status).and_return(404) expect(Faraday.default_connection).to receive(:get).and_return(@response) expect { Reshare.from_xml(@xml) - }.to_not raise_error + }.to raise_error(Diaspora::PostNotFetchable) end it "raises if there's another error receiving the post" do diff --git a/spec/workers/deferred_dispatch_spec.rb b/spec/workers/deferred_dispatch_spec.rb new file mode 100644 index 000000000..f0a20d768 --- /dev/null +++ b/spec/workers/deferred_dispatch_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe Workers::DeferredDispatch do + it "handles non existing records gracefully" do + expect { + described_class.new.perform(alice.id, 'Comment', 0, {}) + }.to_not raise_error + end +end diff --git a/spec/workers/process_photo_spec.rb b/spec/workers/process_photo_spec.rb index 8294902ca..8e28fba74 100644 --- a/spec/workers/process_photo_spec.rb +++ b/spec/workers/process_photo_spec.rb @@ -61,6 +61,12 @@ describe Workers::ProcessPhoto do expect{ result = Workers::ProcessPhoto.new.perform(p.id) }.to_not raise_error - + + end + + it 'handles already deleted photos gracefully' do + expect { + Workers::ProcessPhoto.new.perform(0) + }.to_not raise_error end end |