Allow reports with long comments from remote instances, but truncate (#25028)

This commit is contained in:
Emelia Smith 2023-05-22 13:15:21 +02:00 committed by Claire
parent ea7fa048f3
commit cf80d54cba
6 changed files with 61 additions and 8 deletions

View File

@ -16,7 +16,7 @@ class ActivityPub::Activity::Flag < ActivityPub::Activity
@account, @account,
target_account, target_account,
status_ids: target_statuses.nil? ? [] : target_statuses.map(&:id), status_ids: target_statuses.nil? ? [] : target_statuses.map(&:id),
comment: @json['content'] || '', comment: report_comment,
uri: report_uri uri: report_uri
) )
end end
@ -35,4 +35,8 @@ class ActivityPub::Activity::Flag < ActivityPub::Activity
def report_uri def report_uri
@json['id'] unless @json['id'].nil? || invalid_origin?(@json['id']) @json['id'] unless @json['id'].nil? || invalid_origin?(@json['id'])
end end
def report_comment
(@json['content'] || '')[0...5000]
end
end end

View File

@ -27,6 +27,8 @@ class ActivityPub::TagManager
when :note, :comment, :activity when :note, :comment, :activity
return activity_account_status_url(target.account, target) if target.reblog? return activity_account_status_url(target.account, target) if target.reblog?
short_account_status_url(target.account, target) short_account_status_url(target.account, target)
when :flag
target.uri
end end
end end
@ -41,6 +43,8 @@ class ActivityPub::TagManager
account_status_url(target.account, target) account_status_url(target.account, target)
when :emoji when :emoji
emoji_url(target) emoji_url(target)
when :flag
target.uri
end end
end end

View File

@ -39,7 +39,10 @@ class Report < ApplicationRecord
scope :resolved, -> { where.not(action_taken_at: nil) } scope :resolved, -> { where.not(action_taken_at: nil) }
scope :with_accounts, -> { includes([:account, :target_account, :action_taken_by_account, :assigned_account].index_with({ user: [:invite_request, :invite] })) } scope :with_accounts, -> { includes([:account, :target_account, :action_taken_by_account, :assigned_account].index_with({ user: [:invite_request, :invite] })) }
validates :comment, length: { maximum: 1_000 } # A report is considered local if the reporter is local
delegate :local?, to: :account
validates :comment, length: { maximum: 1_000 }, if: :local?
validates :rule_ids, absence: true, unless: :violation? validates :rule_ids, absence: true, unless: :violation?
validate :validate_rule_ids validate :validate_rule_ids
@ -50,10 +53,6 @@ class Report < ApplicationRecord
violation: 2_000, violation: 2_000,
} }
def local?
false # Force uri_for to use uri attribute
end
before_validation :set_uri, only: :create before_validation :set_uri, only: :create
after_create_commit :trigger_webhooks after_create_commit :trigger_webhooks

View File

@ -37,6 +37,37 @@ RSpec.describe ActivityPub::Activity::Flag do
end end
end end
context 'when the report comment is excessively long' do
subject do
described_class.new({
'@context': 'https://www.w3.org/ns/activitystreams',
id: flag_id,
type: 'Flag',
content: long_comment,
actor: ActivityPub::TagManager.instance.uri_for(sender),
object: [
ActivityPub::TagManager.instance.uri_for(flagged),
ActivityPub::TagManager.instance.uri_for(status),
],
}.with_indifferent_access, sender)
end
let(:long_comment) { Faker::Lorem.characters(number: 6000) }
before do
subject.perform
end
it 'creates a report but with a truncated comment' do
report = Report.find_by(account: sender, target_account: flagged)
expect(report).to_not be_nil
expect(report.comment.length).to eq 5000
expect(report.comment).to eq long_comment[0...5000]
expect(report.status_ids).to eq [status.id]
end
end
context 'when the reported status is private and should not be visible to the remote server' do context 'when the reported status is private and should not be visible to the remote server' do
let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) } let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) }

View File

@ -125,10 +125,17 @@ describe Report do
expect(report).to be_valid expect(report).to be_valid
end end
it 'is invalid if comment is longer than 1000 characters' do let(:remote_account) { Fabricate(:account, domain: 'example.com', protocol: :activitypub, inbox_url: 'http://example.com/inbox') }
it 'is invalid if comment is longer than 1000 characters only if reporter is local' do
report = Fabricate.build(:report, comment: Faker::Lorem.characters(number: 1001)) report = Fabricate.build(:report, comment: Faker::Lorem.characters(number: 1001))
report.valid? expect(report.valid?).to be false
expect(report).to model_have_error_on_field(:comment) expect(report).to model_have_error_on_field(:comment)
end end
it 'is valid if comment is longer than 1000 characters and reporter is not local' do
report = Fabricate.build(:report, account: remote_account, comment: Faker::Lorem.characters(number: 1001))
expect(report.valid?).to be true
end
end end
end end

View File

@ -4,6 +4,14 @@ RSpec.describe ReportService, type: :service do
subject { described_class.new } subject { described_class.new }
let(:source_account) { Fabricate(:account) } let(:source_account) { Fabricate(:account) }
let(:target_account) { Fabricate(:account) }
context 'with a local account' do
it 'has a uri' do
report = subject.call(source_account, target_account)
expect(report.uri).to_not be_nil
end
end
context 'for a remote account' do context 'for a remote account' do
let(:remote_account) { Fabricate(:account, domain: 'example.com', protocol: :activitypub, inbox_url: 'http://example.com/inbox') } let(:remote_account) { Fabricate(:account, domain: 'example.com', protocol: :activitypub, inbox_url: 'http://example.com/inbox') }