From 98936bfcdf48cfd25968d1314ecf41be7d4596c3 Mon Sep 17 00:00:00 2001
From: Akihiko Odaki <akihiko.odaki.4i@stu.hosei.ac.jp>
Date: Tue, 26 Sep 2017 01:33:11 +0900
Subject: [PATCH] Add missing validations in ActivityPub::Activity::Create
 (#5096)

---
 app/lib/activitypub/activity/create.rb       |  12 ++-
 spec/lib/activitypub/activity/create_spec.rb | 104 +++++++++++++++++++
 2 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
index 0964c9f536..4e19b30964 100644
--- a/app/lib/activitypub/activity/create.rb
+++ b/app/lib/activitypub/activity/create.rb
@@ -68,6 +68,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   end
 
   def process_hashtag(tag, status)
+    return if tag['name'].blank?
+
     hashtag = tag['name'].gsub(/\A#/, '').mb_chars.downcase
     hashtag = Tag.where(name: hashtag).first_or_initialize(name: hashtag)
 
@@ -75,6 +77,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   end
 
   def process_mention(tag, status)
+    return if tag['href'].blank?
+
     account = account_from_uri(tag['href'])
     account = FetchRemoteAccountService.new.call(tag['href']) if account.nil?
     return if account.nil?
@@ -82,6 +86,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   end
 
   def process_emoji(tag, _status)
+    return if tag['name'].blank? || tag['href'].blank?
+
     shortcode = tag['name'].delete(':')
     emoji     = CustomEmoji.find_by(shortcode: shortcode, domain: @account.domain)
 
@@ -96,7 +102,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
     return unless @object['attachment'].is_a?(Array)
 
     @object['attachment'].each do |attachment|
-      next if unsupported_media_type?(attachment['mediaType'])
+      next if unsupported_media_type?(attachment['mediaType']) || attachment['url'].blank?
 
       href             = Addressable::URI.parse(attachment['url']).normalize.to_s
       media_attachment = MediaAttachment.create(status: status, account: status.account, remote_url: href)
@@ -106,6 +112,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
       media_attachment.file_remote_url = href
       media_attachment.save
     end
+  rescue Addressable::URI::InvalidURIError => e
+    Rails.logger.debug e
   end
 
   def resolve_thread(status)
@@ -116,7 +124,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   def conversation_from_uri(uri)
     return nil if uri.nil?
     return Conversation.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, 'Conversation')) if OStatus::TagManager.instance.local_id?(uri)
-    Conversation.find_by(uri: uri) || Conversation.create!(uri: uri)
+    Conversation.find_by(uri: uri) || Conversation.create(uri: uri)
   end
 
   def visibility_from_audience
diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb
index 1a9520f04a..cdd4991505 100644
--- a/spec/lib/activitypub/activity/create_spec.rb
+++ b/spec/lib/activitypub/activity/create_spec.rb
@@ -171,6 +171,26 @@ RSpec.describe ActivityPub::Activity::Create do
       end
     end
 
+    context 'with mentions missing href' do
+      let(:object_json) do
+        {
+          id: 'bar',
+          type: 'Note',
+          content: 'Lorem ipsum',
+          tag: [
+            {
+              type: 'Mention',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+        expect(status).to_not be_nil
+      end
+    end
+
     context 'with media attachments' do
       let(:object_json) do
         {
@@ -195,6 +215,27 @@ RSpec.describe ActivityPub::Activity::Create do
       end
     end
 
+    context 'with media attachments missing url' do
+      let(:object_json) do
+        {
+          id: 'bar',
+          type: 'Note',
+          content: 'Lorem ipsum',
+          attachment: [
+            {
+              type: 'Document',
+              mime_type: 'image/png',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+        expect(status).to_not be_nil
+      end
+    end
+
     context 'with hashtags' do
       let(:object_json) do
         {
@@ -219,6 +260,27 @@ RSpec.describe ActivityPub::Activity::Create do
       end
     end
 
+    context 'with hashtags missing name' do
+      let(:object_json) do
+        {
+          id: 'bar',
+          type: 'Note',
+          content: 'Lorem ipsum',
+          tag: [
+            {
+              type: 'Hashtag',
+              href: 'http://example.com/blah',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+        expect(status).to_not be_nil
+      end
+    end
+
     context 'with emojis' do
       let(:object_json) do
         {
@@ -242,5 +304,47 @@ RSpec.describe ActivityPub::Activity::Create do
         expect(status.emojis.map(&:shortcode)).to include('tinking')
       end
     end
+
+    context 'with emojis missing name' do
+      let(:object_json) do
+        {
+          id: 'bar',
+          type: 'Note',
+          content: 'Lorem ipsum :tinking:',
+          tag: [
+            {
+              type: 'Emoji',
+              href: 'http://example.com/emoji.png',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+        expect(status).to_not be_nil
+      end
+    end
+
+    context 'with emojis missing href' do
+      let(:object_json) do
+        {
+          id: 'bar',
+          type: 'Note',
+          content: 'Lorem ipsum :tinking:',
+          tag: [
+            {
+              type: 'Emoji',
+              name: 'tinking',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+        expect(status).to_not be_nil
+      end
+    end
   end
 end