Fix crash when serializing quotes of deleted posts for ActivityPub (#36381)

app/lib/activitypub/activity/create.rb

@@ -218,11 +218,11 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
 
   def process_quote
     @quote_uri = @status_parser.quote_uri
-    return if @quote_uri.blank?
+    return unless @status_parser.quote?
 
     approval_uri = @status_parser.quote_approval_uri
     approval_uri = nil if unsupported_uri_scheme?(approval_uri) || TagManager.instance.local_url?(approval_uri)
-    @quote = Quote.new(account: @account, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?)
+    @quote = Quote.new(account: @account, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending)
   end
 
   def process_hashtag(tag)

app/lib/activitypub/parser/status_parser.rb

@@ -118,6 +118,14 @@ class ActivityPub::Parser::StatusParser
     flags
   end
 
+  def quote?
+    %w(quote _misskey_quote quoteUrl quoteUri).any? { |key| @object[key].present? }
+  end
+
+  def deleted_quote?
+    @object['quote'].is_a?(Hash) && @object['quote']['type'] == 'Tombstone'
+  end
+
   def quote_uri
     %w(quote _misskey_quote quoteUrl quoteUri).filter_map do |key|
       value_or_id(as_array(@object[key]).first)

app/models/quote.rb

@@ -25,7 +25,7 @@ class Quote < ApplicationRecord
   REFRESH_DEADLINE = 6.hours
 
   enum :state,
-       { pending: 0, accepted: 1, rejected: 2, revoked: 3 },
+       { pending: 0, accepted: 1, rejected: 2, revoked: 3, deleted: 4 },
        validate: true
 
   belongs_to :status

app/serializers/activitypub/note_serializer.rb

@@ -32,8 +32,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
   attribute :voters_count, if: :poll_and_voters_count?
 
   attribute :quote, if: :quote?
-  attribute :quote, key: :_misskey_quote, if: :quote?
+  attribute :quote, key: :_misskey_quote, if: :serializable_quote?
-  attribute :quote, key: :quote_uri, if: :quote?
+  attribute :quote, key: :quote_uri, if: :serializable_quote?
   attribute :quote_authorization, if: :quote_authorization?
 
   attribute :interaction_policy
@@ -214,13 +214,17 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
     object.quote&.present?
   end
 
+  def serializable_quote?
+    object.quote&.quoted_status&.present?
+  end
+
   def quote_authorization?
     object.quote.present? && ActivityPub::TagManager.instance.approval_uri_for(object.quote).present?
   end
 
   def quote
     # TODO: handle inlining self-quotes
-    ActivityPub::TagManager.instance.uri_for(object.quote.quoted_status)
+    object.quote.quoted_status.present? ? ActivityPub::TagManager.instance.uri_for(object.quote.quoted_status) : { type: 'Tombstone' }
   end
 
   def quote_authorization

app/services/activitypub/process_status_update_service.rb

@@ -300,7 +300,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
   def update_quote!
     quote_uri = @status_parser.quote_uri
 
-    if quote_uri.present?
+    if @status_parser.quote?
       approval_uri = @status_parser.quote_approval_uri
       approval_uri = nil if unsupported_uri_scheme?(approval_uri) || TagManager.instance.local_url?(approval_uri)
 
@@ -310,7 +310,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
           # Revoke the quote while we get a chance… maybe this should be a `before_destroy` hook?
           RevokeQuoteService.new.call(@status.quote) if @status.quote.quoted_account&.local? && @status.quote.accepted?
           @status.quote.destroy
-          quote = Quote.create(status: @status, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?)
+          quote = Quote.create(status: @status, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending)
           @quote_changed = true
         else
           quote = @status.quote

spec/lib/activitypub/activity/create_spec.rb

@@ -938,6 +938,30 @@ RSpec.describe ActivityPub::Activity::Create do
         end
       end
 
+      context 'with an unverifiable quote of a dead post' do
+        let(:quoted_status) { Fabricate(:status) }
+
+        let(:object_json) do
+          build_object(
+            type: 'Note',
+            content: 'woah what she said is amazing',
+            quote: { type: 'Tombstone' }
+          )
+        end
+
+        it 'creates a status with an unverified quote' do
+          expect { subject.perform }.to change(sender.statuses, :count).by(1)
+
+          status = sender.statuses.first
+          expect(status).to_not be_nil
+          expect(status.quote).to_not be_nil
+          expect(status.quote).to have_attributes(
+            state: 'deleted',
+            approval_uri: nil
+          )
+        end
+      end
+
       context 'with an unverifiable unknown post' do
         let(:unknown_post_uri) { 'https://unavailable.example.com/unavailable-post' }
 

spec/serializers/activitypub/note_serializer_spec.rb

@@ -58,6 +58,21 @@ RSpec.describe ActivityPub::NoteSerializer do
     end
   end
 
+  context 'with a deleted quote' do
+    let(:quoted_status) { Fabricate(:status) }
+
+    before do
+      Fabricate(:quote, status: parent, quoted_status: nil, state: :accepted)
+    end
+
+    it 'has the expected shape' do
+      expect(subject).to include({
+        'type' => 'Note',
+        'quote' => { 'type' => 'Tombstone' },
+      })
+    end
+  end
+
   context 'with a quote policy' do
     let(:parent) { Fabricate(:status, quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:followers] << 16) }
 

spec/services/activitypub/process_status_update_service_spec.rb

@@ -1053,6 +1053,44 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
+  context 'when the status swaps a verified quote with an ID-less Tombstone through an explicit update' do
+    let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
+    let(:quoted_status) { Fabricate(:status, account: quoted_account) }
+    let(:second_quoted_status) { Fabricate(:status, account: quoted_account) }
+    let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) }
+    let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
+
+    let(:payload) do
+      {
+        '@context': [
+          'https://www.w3.org/ns/activitystreams',
+          {
+            '@id': 'https://w3id.org/fep/044f#quote',
+            '@type': '@id',
+          },
+          {
+            '@id': 'https://w3id.org/fep/044f#quoteAuthorization',
+            '@type': '@id',
+          },
+        ],
+        id: 'foo',
+        type: 'Note',
+        summary: 'Show more',
+        content: 'Hello universe',
+        updated: '2021-09-08T22:39:25Z',
+        quote: { type: 'Tombstone' },
+      }
+    end
+
+    it 'updates the URI and unverifies the quote' do
+      expect { subject.call(status, json, json) }
+        .to change { status.quote.quoted_status }.from(quoted_status).to(nil)
+        .and change { status.quote.state }.from('accepted').to('deleted')
+
+      expect { quote.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+
   context 'when the status swaps a verified quote with another verifiable quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:second_quoted_account) { Fabricate(:account, domain: 'second-quoted.example.com') }