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

th-downstream
Emelia Smith 1 year ago committed by GitHub
parent 90466ee43a
commit 8b4d7b22ed

@ -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? || non_matching_uri_hosts?(@account.uri, @json['id']) @json['id'] unless @json['id'].nil? || non_matching_uri_hosts?(@account.uri, @json['id'])
end end
def report_comment
(@json['content'] || '')[0...5000]
end
end end

@ -28,6 +28,8 @@ class ActivityPub::TagManager
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
@ -43,6 +45,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

@ -40,7 +40,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
@ -51,10 +54,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

@ -39,6 +39,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) }

@ -121,10 +121,17 @@ describe Report do
end end
describe 'validations' do describe 'validations' do
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

@ -6,6 +6,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 'with a remote account' do context 'with 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') }
@ -35,7 +43,6 @@ RSpec.describe ReportService, type: :service do
-> { described_class.new.call(source_account, target_account, status_ids: [status.id]) } -> { described_class.new.call(source_account, target_account, status_ids: [status.id]) }
end end
let(:target_account) { Fabricate(:account) }
let(:status) { Fabricate(:status, account: target_account, visibility: :direct) } let(:status) { Fabricate(:status, account: target_account, visibility: :direct) }
context 'when it is addressed to the reporter' do context 'when it is addressed to the reporter' do
@ -91,8 +98,7 @@ RSpec.describe ReportService, type: :service do
-> { described_class.new.call(source_account, target_account) } -> { described_class.new.call(source_account, target_account) }
end end
let!(:target_account) { Fabricate(:account) } let!(:other_report) { Fabricate(:report, target_account: target_account) }
let!(:other_report) { Fabricate(:report, target_account: target_account) }
before do before do
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear

Loading…
Cancel
Save