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 GitHub
parent d51464283c
commit 19f9098551
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 62 additions and 11 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? || 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

View file

@ -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

View file

@ -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

View file

@ -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) }

View file

@ -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

View file

@ -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,7 +98,6 @@ 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