Commit 062dcdf9 authored by Thong Kuah's avatar Thong Kuah

Merge branch '208852-remove-uploadtypecheck-concern' into 'master'

Resolve "Remove UploadTypeCheck concern"

Closes #208852

See merge request gitlab-org/gitlab!26427
parents 5e816cea e6a71f0c
# frozen_string_literal: true
# Ensure that uploaded files are what they say they are for security and
# handling purposes. The checks are not 100% reliable so we err on the side of
# caution and allow by default, and deny when we're confident of a fail state.
#
# Include this concern, then call `check_upload_type` to check all
# uploads. Attach a `mime_type` or `extensions` parameter to only check
# specific upload types. Both parameters will be normalized to a MIME type and
# checked against the inferred MIME type of the upload content and filename
# extension.
#
# class YourUploader
# include UploadTypeCheck::Concern
# check_upload_type mime_types: ['image/png', /image\/jpe?g/]
#
# # or...
#
# check_upload_type extensions: ['png', 'jpg', 'jpeg']
# end
#
# The mime_types parameter can accept `NilClass`, `String`, `Regexp`,
# `Array[String, Regexp]`. This matches the CarrierWave `extension_whitelist`
# and `content_type_whitelist` family of behavior.
#
# The extensions parameter can accept `NilClass`, `String`, `Array[String]`.
module UploadTypeCheck
module Concern
extend ActiveSupport::Concern
class_methods do
def check_upload_type(mime_types: nil, extensions: nil)
define_method :check_upload_type_callback do |file|
magic_file = MagicFile.new(file.to_file)
# Map file extensions back to mime types.
if extensions
mime_types = Array(mime_types) +
Array(extensions).map { |e| MimeMagic::EXTENSIONS[e] }
end
if mime_types.nil? || magic_file.matches_mime_types?(mime_types)
check_content_matches_extension!(magic_file)
end
end
before :cache, :check_upload_type_callback
end
end
def check_content_matches_extension!(magic_file)
return if magic_file.ambiguous_type?
if magic_file.magic_type != magic_file.ext_type
raise CarrierWave::IntegrityError, 'Content type does not match file extension'
end
end
end
# Convenience class to wrap MagicMime objects.
class MagicFile
attr_reader :file
def initialize(file)
@file = file
end
def magic_type
@magic_type ||= MimeMagic.by_magic(file)
end
def ext_type
@ext_type ||= MimeMagic.by_path(file.path)
end
def magic_type_type
magic_type&.type
end
def ext_type_type
ext_type&.type
end
def matches_mime_types?(mime_types)
Array(mime_types).any? do |mt|
magic_type_type =~ /\A#{mt}\z/ || ext_type_type =~ /\A#{mt}\z/
end
end
# - Both types unknown or text/plain.
# - Ambiguous magic type with text extension. Plain text file.
# - Text magic type with ambiguous extension. TeX file missing extension.
def ambiguous_type?
(ext_type.to_s.blank? && magic_type.to_s.blank?) ||
(magic_type.to_s.blank? && ext_type_type == 'text/plain') ||
(ext_type.to_s.blank? && magic_type_type == 'text/plain')
end
end
end
...@@ -2,37 +2,6 @@ ...@@ -2,37 +2,6 @@
# Construct an `uploader` variable that is configured to `check_upload_type` # Construct an `uploader` variable that is configured to `check_upload_type`
# with `mime_types` and `extensions`. # with `mime_types` and `extensions`.
RSpec.shared_context 'uploader with type check' do
let(:uploader_class) do
Class.new(GitlabUploader) do
include UploadTypeCheck::Concern
storage :file
end
end
let(:mime_types) { nil }
let(:extensions) { nil }
let(:uploader) do
uploader_class.class_exec(mime_types, extensions) do |mime_types, extensions|
check_upload_type mime_types: mime_types, extensions: extensions
end
uploader_class.new(build_stubbed(:user))
end
end
# This works with the UploadTypeCheck::Concern
RSpec.shared_context 'stubbed MimeMagic mime type detection' do
let(:mime_type) { '' }
let(:magic_mime) { mime_type }
let(:ext_mime) { mime_type }
before do
magic_mime_obj = MimeMagic.new(magic_mime)
ext_mime_obj = MimeMagic.new(ext_mime)
allow(MimeMagic).to receive(:by_magic).with(anything).and_return(magic_mime_obj)
allow(MimeMagic).to receive(:by_path).with(anything).and_return(ext_mime_obj)
end
end
# @param uploader [CarrierWave::Uploader::Base] uploader with extension_whitelist method. # @param uploader [CarrierWave::Uploader::Base] uploader with extension_whitelist method.
RSpec.shared_context 'ignore extension whitelist check' do RSpec.shared_context 'ignore extension whitelist check' do
before do before do
......
...@@ -26,67 +26,3 @@ shared_examples 'accepted carrierwave upload' do ...@@ -26,67 +26,3 @@ shared_examples 'accepted carrierwave upload' do
expect { uploader.cache!(fixture_file) }.to change { uploader.file }.from(nil).to(kind_of(CarrierWave::SanitizedFile)) expect { uploader.cache!(fixture_file) }.to change { uploader.file }.from(nil).to(kind_of(CarrierWave::SanitizedFile))
end end
end end
def check_content_matches_extension!(file = double(read: nil, path: ''))
magic_file = UploadTypeCheck::MagicFile.new(file)
uploader.check_content_matches_extension!(magic_file)
end
RSpec.shared_examples 'upload passes content type check' do
it 'does not raise error' do
expect { check_content_matches_extension! }.not_to raise_error
end
end
RSpec.shared_examples 'upload fails content type check' do
it 'raises error' do
expect { check_content_matches_extension! }.to raise_error(CarrierWave::IntegrityError)
end
end
def upload_type_checked_filenames(filenames)
Array(filenames).each do |filename|
# Feed the uploader "some" content.
path = File.join('spec', 'fixtures', 'dk.png')
file = File.new(path, 'r')
# Rename the file with what we want.
allow(file).to receive(:path).and_return(filename)
# Force the content type to match the extension type.
mime_type = MimeMagic.by_path(filename)
allow(MimeMagic).to receive(:by_magic).and_return(mime_type)
uploaded_file = Rack::Test::UploadedFile.new(file, original_filename: filename)
uploader.cache!(uploaded_file)
end
end
def upload_type_checked_fixtures(upload_fixtures)
upload_fixtures = Array(upload_fixtures)
upload_fixtures.each do |upload_fixture|
path = File.join('spec', 'fixtures', upload_fixture)
uploader.cache!(fixture_file_upload(path))
end
end
RSpec.shared_examples 'type checked uploads' do |upload_fixtures = nil, filenames: nil|
it 'check type' do
upload_fixtures = Array(upload_fixtures)
filenames = Array(filenames)
times = upload_fixtures.length + filenames.length
expect(uploader).to receive(:check_content_matches_extension!).exactly(times).times
upload_type_checked_fixtures(upload_fixtures) unless upload_fixtures.empty?
upload_type_checked_filenames(filenames) unless filenames.empty?
end
end
RSpec.shared_examples 'skipped type checked uploads' do |upload_fixtures = nil, filenames: nil|
it 'skip type check' do
expect(uploader).not_to receive(:check_content_matches_extension!)
upload_type_checked_fixtures(upload_fixtures) if upload_fixtures
upload_type_checked_filenames(filenames) if filenames
end
end
# frozen_string_literal: true
require 'spec_helper'
describe UploadTypeCheck do
include_context 'uploader with type check'
def upload_fixture(filename)
fixture_file_upload(File.join('spec', 'fixtures', filename))
end
describe '#check_content_matches_extension! callback using file upload' do
context 'when extension matches contents' do
it 'not raise error on upload' do
expect { uploader.cache!(upload_fixture('banana_sample.gif')) }.not_to raise_error
end
end
context 'when extension does not match contents' do
it 'raise error' do
expect { uploader.cache!(upload_fixture('not_a_png.png')) }.to raise_error(CarrierWave::IntegrityError)
end
end
end
describe '#check_content_matches_extension! callback using stubs' do
include_context 'stubbed MimeMagic mime type detection'
context 'when no extension and with ambiguous/text content' do
let(:magic_mime) { '' }
let(:ext_mime) { '' }
it_behaves_like 'upload passes content type check'
end
context 'when no extension and with non-text content' do
let(:magic_mime) { 'image/gif' }
let(:ext_mime) { '' }
it_behaves_like 'upload fails content type check'
end
# Most text files will exhibit this behaviour.
context 'when ambiguous content with text extension' do
let(:magic_mime) { '' }
let(:ext_mime) { 'text/plain' }
it_behaves_like 'upload passes content type check'
end
context 'when text content with text extension' do
let(:magic_mime) { 'text/plain' }
let(:ext_mime) { 'text/plain' }
it_behaves_like 'upload passes content type check'
end
context 'when ambiguous content with non-text extension' do
let(:magic_mime) { '' }
let(:ext_mime) { 'application/zip' }
it_behaves_like 'upload fails content type check'
end
# These are the types when uploading a .dmg
context 'when content and extension do not match' do
let(:magic_mime) { 'application/x-bzip' }
let(:ext_mime) { 'application/x-apple-diskimage' }
it_behaves_like 'upload fails content type check'
end
end
describe '#check_content_matches_extension! mime_type filtering' do
context 'without mime types' do
let(:mime_types) { nil }
it_behaves_like 'type checked uploads', %w[doc_sample.txt rails_sample.jpg]
end
context 'with mime types string' do
let(:mime_types) { 'text/plain' }
it_behaves_like 'type checked uploads', %w[doc_sample.txt]
it_behaves_like 'skipped type checked uploads', %w[dk.png]
end
context 'with mime types regex' do
let(:mime_types) { [/image\/(gif|png)/] }
it_behaves_like 'type checked uploads', %w[banana_sample.gif dk.png]
it_behaves_like 'skipped type checked uploads', %w[doc_sample.txt]
end
context 'with mime types array' do
let(:mime_types) { ['text/plain', /image\/png/] }
it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
it_behaves_like 'skipped type checked uploads', %w[audio_sample.wav]
end
end
describe '#check_content_matches_extension! extensions filtering' do
context 'without extensions' do
let(:extensions) { nil }
it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
end
context 'with extensions string' do
let(:extensions) { 'txt' }
it_behaves_like 'type checked uploads', %w[doc_sample.txt]
it_behaves_like 'skipped type checked uploads', %w[rails_sample.jpg]
end
context 'with extensions array of strings' do
let(:extensions) { %w[txt png] }
it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
it_behaves_like 'skipped type checked uploads', %w[audio_sample.wav]
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment