Allow reports with long comments from remote instances, but truncate (#25028)
This commit is contained in:
parent
8de0f7e198
commit
dcffd6b3d7
6 changed files with 61 additions and 8 deletions
|
@ -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
|
||||||
|
|
|
@ -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
|
||||||
|
|
||||||
|
|
|
@ -38,7 +38,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
|
||||||
|
@ -49,10 +52,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
|
||||||
|
|
||||||
def object_type
|
def object_type
|
||||||
|
|
|
@ -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) }
|
||||||
|
|
||||||
|
|
|
@ -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
|
||||||
|
|
|
@ -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') }
|
||||||
|
|
Loading…
Reference in a new issue