From e46051ca94690782ca24741a570f2051296cd2d5 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 4 Jan 2020 01:54:07 +0100 Subject: [PATCH] port tootsuite#12748 to monsterfork: Fix base64-encoded file uploads not being possible Fix #3804, Fix #5776 --- .../admin/custom_emojis_controller.rb | 3 -- app/controllers/api/v1/media_controller.rb | 3 -- .../concerns/obfuscate_filename.rb | 16 ---------- .../settings/profiles_controller.rb | 6 ++-- app/models/concerns/attachmentable.rb | 19 ++++++++++++ app/models/media_attachment.rb | 3 ++ config/initializers/paperclip.rb | 2 ++ .../controllers/api/proofs_controller_spec.rb | 5 +--- .../concerns/obfuscate_filename_spec.rb | 30 ------------------- spec/models/account_spec.rb | 1 + spec/models/media_attachment_spec.rb | 18 +++++++++++ .../models/concerns/account_avatar.rb | 20 +++++++++++++ .../models/concerns/account_header.rb | 23 ++++++++++++++ 13 files changed, 89 insertions(+), 60 deletions(-) delete mode 100644 app/controllers/concerns/obfuscate_filename.rb delete mode 100644 spec/controllers/concerns/obfuscate_filename_spec.rb create mode 100644 spec/support/examples/models/concerns/account_header.rb diff --git a/app/controllers/admin/custom_emojis_controller.rb b/app/controllers/admin/custom_emojis_controller.rb index f77699166769fd..d61bafdf02d7d9 100644 --- a/app/controllers/admin/custom_emojis_controller.rb +++ b/app/controllers/admin/custom_emojis_controller.rb @@ -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]) diff --git a/app/controllers/api/v1/media_controller.rb b/app/controllers/api/v1/media_controller.rb index 97b213cae4fd98..2f89a973e909ef 100644 --- a/app/controllers/api/v1/media_controller.rb +++ b/app/controllers/api/v1/media_controller.rb @@ -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 diff --git a/app/controllers/concerns/obfuscate_filename.rb b/app/controllers/concerns/obfuscate_filename.rb deleted file mode 100644 index 22736ec3abf968..00000000000000 --- a/app/controllers/concerns/obfuscate_filename.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module ObfuscateFilename - extend ActiveSupport::Concern - - class_methods do - def obfuscate_filename(path) - before_action do - file = params.dig(*path) - next if file.nil? - - file.original_filename = SecureRandom.hex(8) + File.extname(file.original_filename) - end - end - end -end diff --git a/app/controllers/settings/profiles_controller.rb b/app/controllers/settings/profiles_controller.rb index 6b3f0d311a1e6a..8e6dd9bd86294b 100644 --- a/app/controllers/settings/profiles_controller.rb +++ b/app/controllers/settings/profiles_controller.rb @@ -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 diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index de4cf87757715e..dd11e081406320 100644 --- a/app/models/concerns/attachmentable.rb +++ b/app/models/concerns/attachmentable.rb @@ -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 @@ -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 diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 4f0a097846d73d..d2db899ab24a10 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -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 diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 096f7a11e0f973..9ebb41ec2a8b81 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +Paperclip::DataUriAdapter.register + Paperclip.interpolates :filename do |attachment, style| if style == :original attachment.original_filename diff --git a/spec/controllers/api/proofs_controller_spec.rb b/spec/controllers/api/proofs_controller_spec.rb index dbde4927f1484d..2fe6150052cff3 100644 --- a/spec/controllers/api/proofs_controller_spec.rb +++ b/spec/controllers/api/proofs_controller_spec.rb @@ -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 diff --git a/spec/controllers/concerns/obfuscate_filename_spec.rb b/spec/controllers/concerns/obfuscate_filename_spec.rb deleted file mode 100644 index e06d53c038557e..00000000000000 --- a/spec/controllers/concerns/obfuscate_filename_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe ApplicationController, type: :controller do - controller do - include ObfuscateFilename - - obfuscate_filename :file - - def file - render plain: params[:file]&.original_filename - end - end - - before do - routes.draw { get 'file' => 'anonymous#file' } - end - - it 'obfusticates filename if the given parameter is specified' do - file = fixture_file_upload('files/imports.txt', 'text/plain') - post 'file', params: { file: file } - expect(response.body).to end_with '.txt' - expect(response.body).not_to include 'imports' - end - - it 'does nothing if the given parameter is not specified' do - post 'file' - end -end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index f46ce6a52cd278..023b715fc0984f 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -755,4 +755,5 @@ end include_examples 'AccountAvatar', :account + include_examples 'AccountHeader', :account end diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 0333c56d83ab0b..af22a9812b0e69 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -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 diff --git a/spec/support/examples/models/concerns/account_avatar.rb b/spec/support/examples/models/concerns/account_avatar.rb index f2a8a2459c57fa..2180f52739fb5a 100644 --- a/spec/support/examples/models/concerns/account_avatar.rb +++ b/spec/support/examples/models/concerns/account_avatar.rb @@ -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 diff --git a/spec/support/examples/models/concerns/account_header.rb b/spec/support/examples/models/concerns/account_header.rb new file mode 100644 index 00000000000000..77ee0e62901380 --- /dev/null +++ b/spec/support/examples/models/concerns/account_header.rb @@ -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