Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable file download in Merritt UI #104

Merged
merged 2 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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