Skip to content

Commit

Permalink
Merge pull request #104 from CDLUC3/i724
Browse files Browse the repository at this point in the history
disable file download in Merritt UI
  • Loading branch information
terrywbrady committed Sep 27, 2021
2 parents 12e2490 + 71bb049 commit 058a786
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 101 deletions.
17 changes: 7 additions & 10 deletions app/controllers/file_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,10 @@ class FileController < ApplicationController
before_action :check_version, only: %i[download presign]

def download
if @file.exceeds_download_size?
render file: "#{Rails.root}/public/403.html", status: 403, layout: false
else
stream_response(@file.bytestream_uri,
'inline',
File.basename(@file.pathname),
@file.mime_type,
@file.full_size)
end
# deprecate file download in favor of presigned retrieval
url = request.url.gsub(%r[/d/], "/api/presign-file/")
response.headers['Location'] = url
render status: 308, plain: ''
end

# API Call to redirect to presign URL for a file.
Expand Down Expand Up @@ -149,14 +144,16 @@ def storage_key_do
def fix_filename
# determine if the filename has already been decoded (by looking for the slash after producer or system)
fname = params[:file]
return fname if fname.match(/(producer|system)\//)
return fname if fname.match(%r{(producer|system)/})

# retrieve decoded version of the parameter
fname = params_u(:file)
return fname if fname.valid_encoding?

# :nocov:
log.warn("invalid encoding for filname #{fname}")
fname
# :nocov:
end

def fix_params
Expand Down
2 changes: 2 additions & 0 deletions app/models/inv_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ def external_bytestream_uri
end

def exceeds_download_size?
# :nocov:
full_size > APP_CONFIG['max_download_size']
# :nocov:
end
end
95 changes: 4 additions & 91 deletions spec/controllers/file_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,102 +63,15 @@
expect(response.status).to eq(401)
end

it 'prevents download when download size exceeded' do
it 'download file deprecated' do
mock_permissions_all(user_id, collection_id)

size_too_large = 1 + APP_CONFIG['max_download_size']
inv_file.full_size = size_too_large
inv_file.save!

request.session.merge!({ uid: user_id })
get(:download, params: params)
expect(response.status).to eq(403)
end

it 'streams the file' do
mock_permissions_all(user_id, collection_id)

size_ok = APP_CONFIG['max_download_size'] - 1
inv_file.full_size = size_ok
inv_file.save!

streamer = double(Streamer)
expected_url = inv_file.bytestream_uri
allow(Streamer).to receive(:new).with(expected_url).and_return(streamer)

request.session.merge!({ uid: user_id })
get(:download, params: params)

expect(response.status).to eq(200)

expected_headers = {
'Content-Type' => inv_file.mime_type,
'Content-Disposition' => "inline; filename=\"#{basename}\"",
'Content-Length' => inv_file.full_size.to_s
}
response_headers = response.headers
expected_headers.each do |header, value|
expect(response_headers[header]).to eq(value)
end
end

it 'handles filenames with spaces' do
pathname = 'producer/Caltrans EHE Tests.pdf'
mock_permissions_all(user_id, collection_id)

size_ok = APP_CONFIG['max_download_size'] - 1
inv_file.full_size = size_ok
inv_file.pathname = pathname
inv_file.save!

streamer = double(Streamer)
expected_url = inv_file.bytestream_uri
allow(Streamer).to receive(:new).with(expected_url).and_return(streamer)

params[:file] = pathname
request.session.merge!({ uid: user_id })
get(:download, params: params)
expect(response.status).to eq(200)
end

# Note that percent signs in filenames will generate invalid download links
skip it 'handles filenames with percent sign' do
pathname = 'producer/Test %25BF.pdf'
mock_permissions_all(user_id, collection_id)

size_ok = APP_CONFIG['max_download_size'] - 1
inv_file.full_size = size_ok
inv_file.pathname = pathname
inv_file.save!

streamer = double(Streamer)
expected_url = inv_file.bytestream_uri
allow(Streamer).to receive(:new).with(expected_url).and_return(streamer)

params[:file] = pathname
request.session.merge!({ uid: user_id })
get(:download, params: params)
expect(response.status).to eq(200)
expect(response.status).to eq(308)
expect(response.body).to eq('')
expect(response.headers).to have_key('Location')
end

it 'handles filenames with spaces and pipes' do
pathname = 'producer/AIP/Subseries 1.1/Objects/Evolution book/Tate Collection |landscape2'
mock_permissions_all(user_id, collection_id)

size_ok = APP_CONFIG['max_download_size'] - 1
inv_file.full_size = size_ok
inv_file.pathname = pathname
inv_file.save!

streamer = double(Streamer)
expected_url = inv_file.bytestream_uri
allow(Streamer).to receive(:new).with(expected_url).and_return(streamer)

params[:file] = pathname
request.session.merge!({ uid: user_id })
get(:download, params: params)
expect(response.status).to eq(200)
end
end

describe ':presign' do
Expand Down

0 comments on commit 058a786

Please sign in to comment.