Set correct attachment type for rejected media (#2599)

In #2110, a new attachment type "unknown" was introduced for
attachments that were rejected due to a domain being blocked using
reject_media. However, the "type" field was never set to "unknown"
because a default value of "0" (image) is set for that column,
causing the `type.blank?` expression to always equal false.

This version uses type_changed? instead, causing the type to be set
to "unknown" unless a type has been explicitly set. This introduces
a small change in behaviour causing the type to be set to unknown
before paperclip calls `before_post_process`. Presumably this
behaviour is more appropriate than the current one because the
attachment type has not been determined by that point.

Included are new tests for `ProcessFeedService` and
`UpdateRemoteProfileService` which now check that remote media is
downloaded for non-blocked domains and is rejected for others.
This commit is contained in:
Patrick Figel 2017-04-29 00:18:32 +02:00 committed by Eugen Rochko
parent a823509b99
commit 8ac7fca5d0
3 changed files with 93 additions and 29 deletions

View File

@ -96,7 +96,7 @@ class MediaAttachment < ApplicationRecord
private private
def set_shortcode def set_shortcode
self.type = :unknown if file.blank? && type.blank? self.type = :unknown if file.blank? && !type_changed?
return unless local? return unless local?

View File

@ -12,7 +12,10 @@ RSpec.describe ProcessFeedService do
stub_request(:get, "http://kickass.zone/system/accounts/avatars/000/000/001/large/eris.png").to_return(request_fixture('avatar.txt')) stub_request(:get, "http://kickass.zone/system/accounts/avatars/000/000/001/large/eris.png").to_return(request_fixture('avatar.txt'))
stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/002/original/morpheus_linux.jpg?1476059910").to_return(request_fixture('attachment1.txt')) stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/002/original/morpheus_linux.jpg?1476059910").to_return(request_fixture('attachment1.txt'))
stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/003/original/gizmo.jpg?1476060065").to_return(request_fixture('attachment2.txt')) stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/003/original/gizmo.jpg?1476060065").to_return(request_fixture('attachment2.txt'))
end
context 'when domain does not reject media' do
before do
subject.call(body, account) subject.call(body, account)
end end
@ -20,6 +23,7 @@ RSpec.describe ProcessFeedService do
account.reload account.reload
expect(account.display_name).to eq '::1' expect(account.display_name).to eq '::1'
expect(account).to have_attached_file(:avatar) expect(account).to have_attached_file(:avatar)
expect(account.avatar_file_name).not_to be_nil
end end
it 'creates posts' do it 'creates posts' do
@ -49,6 +53,41 @@ RSpec.describe ProcessFeedService do
expect(status).to_not be_nil expect(status).to_not be_nil
expect(status.media_attachments.first).to have_attached_file(:file) expect(status.media_attachments.first).to have_attached_file(:file)
expect(status.media_attachments.first.image?).to be true
expect(status.media_attachments.first.file_file_name).not_to be_nil
end
end
context 'when domain is set to reject media' do
let!(:domain_block) { Fabricate(:domain_block, domain: 'kickass.zone', reject_media: true) }
before do
subject.call(body, account)
end
it 'updates remote user\'s account information' do
account.reload
expect(account.display_name).to eq '::1'
end
it 'rejects remote user\'s avatar' do
account.reload
expect(account.display_name).to eq '::1'
expect(account.avatar_file_name).to be_nil
end
it 'creates posts' do
expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=1:objectType=Status')).to_not be_nil
expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Status')).to_not be_nil
end
it 'creates posts with remote-only media' do
status = Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=14:objectType=Status')
expect(status).to_not be_nil
expect(status.media_attachments.first.file_file_name).to be_nil
expect(status.media_attachments.first.unknown?).to be true
end
end end
end end

View File

@ -56,4 +56,29 @@ RSpec.describe UpdateRemoteProfileService do
expect(remote_account.reload.note).to eq 'Software engineer, free time musician and enthusiast. Likes cats. Warning: May contain memes' expect(remote_account.reload.note).to eq 'Software engineer, free time musician and enthusiast. Likes cats. Warning: May contain memes'
end end
end end
context 'with updated details from a domain set to reject media' do
let(:remote_account) { Fabricate(:account, username: 'bob', domain: 'example.com') }
let!(:domain_block) { Fabricate(:domain_block, domain: 'example.com', reject_media: true) }
before do
subject.call(xml, remote_account)
end
it 'does not the avatar remote url' do
expect(remote_account.reload.avatar_remote_url).to be_nil
end
it 'sets display name' do
expect(remote_account.reload.display_name).to eq ' '
end
it 'sets note' do
expect(remote_account.reload.note).to eq 'Software engineer, free time musician and enthusiast. Likes cats. Warning: May contain memes'
end
it 'does not set store the avatar' do
expect(remote_account.reload.avatar_file_name).to be_nil
end
end
end end