Skip to content

Commit

Permalink
FIX: randomize file name when created from fixtures (#9731)
Browse files Browse the repository at this point in the history
* FIX: randomize file name when created from fixtures

When a temporary file is created from fixtures it should have a unique name.
It is to prevent a collision in parallel specs evaluation

* FIX: use /tmp/pid folder to keep fixture files
  • Loading branch information
lis2 committed May 18, 2020
1 parent 16f6240 commit f99f6ca
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
24 changes: 21 additions & 3 deletions spec/rails_helper.rb
Expand Up @@ -237,6 +237,12 @@ class << x.exception
end
end

config.after(:suite) do
if SpecSecureRandom.value
FileUtils.remove_dir(file_from_fixtures_tmp_folder, true)
end
end

config.before :each, &TestSetup.method(:test_setup)

config.before(:each, type: :multisite) do
Expand Down Expand Up @@ -368,9 +374,15 @@ def unfreeze_time
end

def file_from_fixtures(filename, directory = "images")
FileUtils.mkdir_p("#{Rails.root}/tmp/spec") unless Dir.exists?("#{Rails.root}/tmp/spec")
FileUtils.cp("#{Rails.root}/spec/fixtures/#{directory}/#{filename}", "#{Rails.root}/tmp/spec/#{filename}")
File.new("#{Rails.root}/tmp/spec/#{filename}")
SpecSecureRandom.value ||= SecureRandom.hex
FileUtils.mkdir_p(file_from_fixtures_tmp_folder) unless Dir.exists?(file_from_fixtures_tmp_folder)
tmp_file_path = File.join(file_from_fixtures_tmp_folder, SecureRandom.hex << filename)
FileUtils.cp("#{Rails.root}/spec/fixtures/#{directory}/#{filename}", tmp_file_path)
File.new(tmp_file_path)
end

def file_from_fixtures_tmp_folder
File.join(Dir.tmpdir, "rspec_#{Process.pid}_#{SpecSecureRandom.value}")
end

def has_trigger?(trigger_name)
Expand Down Expand Up @@ -410,3 +422,9 @@ def track_log_messages(level: nil)
ensure
Rails.logger = old_logger
end

class SpecSecureRandom
class << self
attr_accessor :value
end
end
8 changes: 5 additions & 3 deletions spec/requests/admin/themes_controller_spec.rb
Expand Up @@ -24,22 +24,24 @@
end

describe '#upload_asset' do
let(:file) { file_from_fixtures("fake.woff2", "woff2") }
let(:filename) { File.basename(file) }
let(:upload) do
Rack::Test::UploadedFile.new(file_from_fixtures("fake.woff2", "woff2"))
Rack::Test::UploadedFile.new(file)
end

it 'can create a theme upload' do
post "/admin/themes/upload_asset.json", params: { file: upload }
expect(response.status).to eq(201)

upload = Upload.find_by(original_filename: "fake.woff2")
upload = Upload.find_by(original_filename: filename)

expect(upload.id).not_to be_nil
expect(response.parsed_body["upload_id"]).to eq(upload.id)
end

context "when trying to upload an existing file" do
let(:uploaded_file) { Upload.find_by(original_filename: "fake.woff2") }
let(:uploaded_file) { Upload.find_by(original_filename: filename) }
let(:response_json) { response.parsed_body }

before do
Expand Down
12 changes: 7 additions & 5 deletions spec/requests/uploads_controller_spec.rb
Expand Up @@ -16,9 +16,11 @@
sign_in(user)
end

let(:logo_file) { file_from_fixtures("logo.png") }
let(:logo) do
Rack::Test::UploadedFile.new(file_from_fixtures("logo.png"))
Rack::Test::UploadedFile.new(logo_file)
end
let(:logo_filename) { File.basename(logo_file) }

let(:fake_jpg) do
Rack::Test::UploadedFile.new(file_from_fixtures("fake.jpg"))
Expand Down Expand Up @@ -160,7 +162,7 @@
upload = Upload.last

expect(upload.id).to eq(id)
expect(upload.original_filename).to eq('logo.png')
expect(upload.original_filename).to eq(logo_filename)
end

it 'respects `authorized_extensions_for_staff` setting when staff upload file' do
Expand Down Expand Up @@ -265,7 +267,7 @@ def upload_file(file, folder = "images")
expect(response.status).to eq(200)

expect(response.headers["Content-Disposition"])
.to eq(%Q|attachment; filename="logo.png"; filename*=UTF-8''logo.png|)
.to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
end

it 'returns 200 when js file' do
Expand All @@ -282,7 +284,7 @@ def upload_file(file, folder = "images")
get "/uploads/#{site}/#{upload.sha1}.json"
expect(response.status).to eq(200)
expect(response.headers["Content-Disposition"])
.to eq(%Q|attachment; filename="image_no_extension.png"; filename*=UTF-8''image_no_extension.png|)
.to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
end

it "handles file without extension" do
Expand All @@ -292,7 +294,7 @@ def upload_file(file, folder = "images")
get "/uploads/#{site}/#{upload.sha1}.json"
expect(response.status).to eq(200)
expect(response.headers["Content-Disposition"])
.to eq(%Q|attachment; filename="not_an_image"; filename*=UTF-8''not_an_image|)
.to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
end

context "prevent anons from downloading files" do
Expand Down

0 comments on commit f99f6ca

Please sign in to comment.