about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2020-12-18 09:18:31 +0100
committerGitHub <noreply@github.com>2020-12-18 09:18:31 +0100
commiteb35be0431b2cdd2bbf3339beb9c5a0839e1088b (patch)
tree6e95bf9c68f7ce9b18c4a5ae404b484ef670655c
parent941ff04b03a8a3e3f03e95c108f0cfa621226fb1 (diff)
Fix follow limit preventing re-following of a moved account (#14207)
-rw-r--r--app/models/follow.rb2
-rw-r--r--app/models/follow_request.rb2
-rw-r--r--app/models/import.rb1
-rw-r--r--app/services/import_service.rb7
-rw-r--r--app/validators/import_validator.rb44
-rw-r--r--config/locales/en.yml2
-rw-r--r--spec/models/follow_spec.rb2
-rw-r--r--spec/models/import_spec.rb10
8 files changed, 62 insertions, 8 deletions
diff --git a/app/models/follow.rb b/app/models/follow.rb
index 55a9da792..69a1722b3 100644
--- a/app/models/follow.rb
+++ b/app/models/follow.rb
@@ -26,7 +26,7 @@ class Follow < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create
+  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   scope :recent, -> { reorder(id: :desc) }
 
diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb
index c1f19149b..2d2a77b59 100644
--- a/app/models/follow_request.rb
+++ b/app/models/follow_request.rb
@@ -26,7 +26,7 @@ class FollowRequest < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create
+  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   def authorize!
     account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri)
diff --git a/app/models/import.rb b/app/models/import.rb
index 702453289..00a54892e 100644
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -27,6 +27,7 @@ class Import < ApplicationRecord
   enum type: [:following, :blocking, :muting, :domain_blocking, :bookmarks]
 
   validates :type, presence: true
+  validates_with ImportValidator, on: :create
 
   has_attached_file :data
   validates_attachment_content_type :data, content_type: FILE_TYPES
diff --git a/app/services/import_service.rb b/app/services/import_service.rb
index 288e47f1e..0c6ef2238 100644
--- a/app/services/import_service.rb
+++ b/app/services/import_service.rb
@@ -27,7 +27,7 @@ class ImportService < BaseService
 
   def import_follows!
     parse_import_data!(['Account address'])
-    import_relationships!('follow', 'unfollow', @account.following, follow_limit, reblogs: { header: 'Show boosts', default: true })
+    import_relationships!('follow', 'unfollow', @account.following, ROWS_PROCESSING_LIMIT, reblogs: { header: 'Show boosts', default: true })
   end
 
   def import_blocks!
@@ -85,6 +85,7 @@ class ImportService < BaseService
 
     head_items = items.uniq { |acct, _| acct.split('@')[1] }
     tail_items = items - head_items
+
     Import::RelationshipWorker.push_bulk(head_items + tail_items) do |acct, extra|
       [@account.id, acct, action, extra]
     end
@@ -133,10 +134,6 @@ class ImportService < BaseService
     Paperclip.io_adapters.for(@import.data).read
   end
 
-  def follow_limit
-    FollowLimitValidator.limit_for_account(@account)
-  end
-
   def relations_map_for_account(account, account_ids)
     {
       blocking: {},
diff --git a/app/validators/import_validator.rb b/app/validators/import_validator.rb
new file mode 100644
index 000000000..a182abfa5
--- /dev/null
+++ b/app/validators/import_validator.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+class ImportValidator < ActiveModel::Validator
+  KNOWN_HEADERS = [
+    'Account address',
+    '#domain',
+    '#uri',
+  ].freeze
+
+  def validate(import)
+    return if import.type.blank? || import.data.blank?
+
+    # We parse because newlines could be part of individual rows. This
+    # runs on create so we should be reading the local file here before
+    # it is uploaded to object storage or moved anywhere...
+    csv_data = CSV.parse(import.data.queued_for_write[:original].read)
+
+    row_count  = csv_data.size
+    row_count -= 1 if KNOWN_HEADERS.include?(csv_data.first&.first)
+
+    import.errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: ImportService::ROWS_PROCESSING_LIMIT)) if row_count > ImportService::ROWS_PROCESSING_LIMIT
+
+    case import.type
+    when 'following'
+      validate_following_import(import, row_count)
+    end
+  end
+
+  private
+
+  def validate_following_import(import, row_count)
+    base_limit = FollowLimitValidator.limit_for_account(import.account)
+
+    limit = begin
+      if import.overwrite?
+        base_limit
+      else
+        base_limit - import.account.following_count
+      end
+    end
+
+    import.errors.add(:data, I18n.t('users.follow_limit_reached', limit: base_limit)) if row_count > limit
+  end
+end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e91f4344f..18a207f5e 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -923,6 +923,8 @@ en:
     status: Verification status
     view_proof: View proof
   imports:
+    errors:
+      over_rows_processing_limit: contains more than %{count} rows
     modes:
       merge: Merge
       merge_long: Keep existing records and add new ones
diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb
index 0c84e5e7b..e723a1ef2 100644
--- a/spec/models/follow_spec.rb
+++ b/spec/models/follow_spec.rb
@@ -5,7 +5,7 @@ RSpec.describe Follow, type: :model do
   let(:bob)   { Fabricate(:account, username: 'bob') }
 
   describe 'validations' do
-    subject { Follow.new(account: alice, target_account: bob) }
+    subject { Follow.new(account: alice, target_account: bob, rate_limit: true) }
 
     it 'has a valid fabricator' do
       follow = Fabricate.build(:follow)
diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb
index 321761166..a5eec1722 100644
--- a/spec/models/import_spec.rb
+++ b/spec/models/import_spec.rb
@@ -20,5 +20,15 @@ RSpec.describe Import, type: :model do
       import = Import.create(account: account, type: type)
       expect(import).to model_have_error_on_field(:data)
     end
+
+    it 'is invalid with too many rows in data' do
+      import = Import.create(account: account, type: type, data: StringIO.new("foo@bar.com\n" * (ImportService::ROWS_PROCESSING_LIMIT + 10)))
+      expect(import).to model_have_error_on_field(:data)
+    end
+
+    it 'is invalid when there are more rows when following limit' do
+      import = Import.create(account: account, type: type, data: StringIO.new("foo@bar.com\n" * (FollowLimitValidator.limit_for_account(account) + 10)))
+      expect(import).to model_have_error_on_field(:data)
+    end
   end
 end