Skip to content

Commit

Permalink
port tootsuite#12748 to monsterfork: Fix base64-encoded file uploads …
Browse files Browse the repository at this point in the history
…not being possible

Fix mastodon#3804, Fix mastodon#5776
  • Loading branch information
Gargron authored and multiple creatures committed Feb 21, 2020
1 parent 90f2752 commit e46051c
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 60 deletions.
3 changes: 0 additions & 3 deletions app/controllers/admin/custom_emojis_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ class CustomEmojisController < BaseController
before_action :set_custom_emoji, except: [:index, :new, :create]
before_action :set_filter_params

include ObfuscateFilename
obfuscate_filename [:custom_emoji, :image]

def index
authorize :custom_emoji, :index?
@custom_emojis = filtered_custom_emojis.eager_load(:local_counterpart).page(params[:page])
Expand Down
3 changes: 0 additions & 3 deletions app/controllers/api/v1/media_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ class Api::V1::MediaController < Api::BaseController
before_action -> { doorkeeper_authorize! :write, :'write:media' }
before_action :require_user!

include ObfuscateFilename
obfuscate_filename :file

respond_to :json

def create
Expand Down
16 changes: 0 additions & 16 deletions app/controllers/concerns/obfuscate_filename.rb

This file was deleted.

6 changes: 2 additions & 4 deletions app/controllers/settings/profiles_controller.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# frozen_string_literal: true

class Settings::ProfilesController < Settings::BaseController
include ObfuscateFilename
layout 'admin'

before_action :authenticate_user!
before_action :set_account

obfuscate_filename [:account, :avatar]
obfuscate_filename [:account, :header]

def show
@account.build_fields
end
Expand Down
19 changes: 19 additions & 0 deletions app/models/concerns/attachmentable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module Attachmentable
MAX_MATRIX_LIMIT = 16_777_216 # 4096x4096px or approx. 16MB

included do
before_post_process :obfuscate_file_name
before_post_process :set_file_extensions
before_post_process :check_image_dimensions
end
Expand Down Expand Up @@ -47,4 +48,22 @@ def appropriate_extension(attachment)

extension
end

def calculated_content_type(attachment)
content_type = Paperclip.run('file', '-b --mime :file', file: attachment.queued_for_write[:original].path).split(/[:;\s]+/).first.chomp
content_type = 'video/mp4' if content_type == 'video/x-m4v'
content_type
rescue Terrapin::CommandLineError
''
end

def obfuscate_file_name
self.class.attachment_definitions.each_key do |attachment_name|
attachment = send(attachment_name)

next if attachment.blank?

attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name))
end
end
end
3 changes: 3 additions & 0 deletions app/models/media_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,12 @@ def focus
end

after_commit :reset_parent_cache, on: :update

before_create :prepare_description, unless: :local?
before_create :set_shortcode

before_post_process :set_type_and_extension

before_save :set_meta

class << self
Expand Down
2 changes: 2 additions & 0 deletions config/initializers/paperclip.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

Paperclip::DataUriAdapter.register

Paperclip.interpolates :filename do |attachment, style|
if style == :original
attachment.original_filename
Expand Down
5 changes: 1 addition & 4 deletions spec/controllers/api/proofs_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@
end

it 'has the correct avatar url' do
first_part = 'https://cb6e6126.ngrok.io/system/accounts/avatars/'
last_part = 'original/avatar.gif'

expect(body_as_json[:avatar]).to match /#{Regexp.quote(first_part)}(?:\d{3,5}\/){3}#{Regexp.quote(last_part)}/
expect(body_as_json[:avatar]).to match "https://cb6e6126.ngrok.io#{alice.avatar.url}"
end
end
end
Expand Down
30 changes: 0 additions & 30 deletions spec/controllers/concerns/obfuscate_filename_spec.rb

This file was deleted.

1 change: 1 addition & 0 deletions spec/models/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -755,4 +755,5 @@
end

include_examples 'AccountAvatar', :account
include_examples 'AccountHeader', :account
end
18 changes: 18 additions & 0 deletions spec/models/media_attachment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,24 @@
expect(media.file.meta["small"]["height"]).to eq 327
expect(media.file.meta["small"]["aspect"]).to eq 490.0 / 327
end

it 'gives the file a random name' do
expect(media.file_file_name).to_not eq 'attachment.jpg'
end
end

describe 'base64-encoded jpeg' do
let(:base64_attachment) { "data:image/jpeg;base64,#{Base64.encode64(attachment_fixture('attachment.jpg').read)}" }
let(:media) { MediaAttachment.create(account: Fabricate(:account), file: base64_attachment) }

it 'saves media attachment' do
expect(media.persisted?).to be true
expect(media.file).to_not be_nil
end

it 'gives the file a file name' do
expect(media.file_file_name).to_not be_blank
end
end

describe 'descriptions for remote attachments' do
Expand Down
20 changes: 20 additions & 0 deletions spec/support/examples/models/concerns/account_avatar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,24 @@
end
end
end

describe 'base64-encoded files' do
let(:base64_attachment) { "data:image/jpeg;base64,#{Base64.encode64(attachment_fixture('attachment.jpg').read)}" }
let(:account) { Fabricate(fabricator, avatar: base64_attachment) }

it 'saves avatar' do
expect(account.persisted?).to be true
expect(account.avatar).to_not be_nil
end

it 'gives the avatar a file name' do
expect(account.avatar_file_name).to_not be_blank
end

it 'saves a new avatar under a different file name' do
previous_file_name = account.avatar_file_name
account.update(avatar: base64_attachment)
expect(account.avatar_file_name).to_not eq previous_file_name
end
end
end
23 changes: 23 additions & 0 deletions spec/support/examples/models/concerns/account_header.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

shared_examples 'AccountHeader' do |fabricator|
describe 'base64-encoded files' do
let(:base64_attachment) { "data:image/jpeg;base64,#{Base64.encode64(attachment_fixture('attachment.jpg').read)}" }
let(:account) { Fabricate(fabricator, header: base64_attachment) }

it 'saves header' do
expect(account.persisted?).to be true
expect(account.header).to_not be_nil
end

it 'gives the header a file name' do
expect(account.header_file_name).to_not be_blank
end

it 'saves a new header under a different file name' do
previous_file_name = account.header_file_name
account.update(header: base64_attachment)
expect(account.header_file_name).to_not eq previous_file_name
end
end
end

0 comments on commit e46051c

Please sign in to comment.