Skip to content
New issue

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

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

Already on GitHub? # to your account

handle whitespace for file uploads (attempt 2) #567

Merged
merged 11 commits into from
Aug 16, 2017
3 changes: 1 addition & 2 deletions app/assets/javascripts/file_upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ function bgUpload(elem) {
progressBar.removeClass('active');
// extract key and generate URL from response
var key = $(data.jqXHR.responseXML).find("Key").text();
var url = 'https://' + form.data('host') + '/' + key;

// create hidden field
var input = $("<input />", { type:'hidden', name: fileInput.attr('name'), value: url, class: 's3-file' });
var input = $("<input />", { type:'hidden', name: fileInput.attr('name'), value: key, class: 's3-file' });
container.append(input);
},
fail: function(e, data) {
Expand Down
9 changes: 5 additions & 4 deletions app/controllers/concerns/file_handling_for_datasets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ def process_files

if [ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile].include?(f["file"].class)
Rails.logger.info "file is an Http::UploadedFile (non javascript?)"
storage_object = FileStorageService.create_and_upload_public_object(f["file"].original_filename, f["file"].read)
key = URI.encode(f["file"].original_filename)
storage_object = FileStorageService.create_and_upload_public_object(key, f["file"].read)

f["storage_key"] = storage_object.key(f["file"].original_filename)
f["storage_key"] = storage_object.key(key)
f["file"] = storage_object.public_url
else
Rails.logger.info "file is not an http uploaded file, it's a URL"
f["storage_key"] = URI(f["file"]).path.gsub(/^\//, '') unless f["file"].nil?
Rails.logger.info "file is not an http uploaded file, it's an s3 storage key"
f["storage_key"] = f["file"] unless f["file"].nil?
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/dataset_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def self.file_from_url(file)
end

def self.file_from_url_with_storage_key(file, storage_key)
Rails.logger.info "DatasetFile: In file_from_url_with_storage_key"
Rails.logger.info "DatasetFile: In file_from_url_with_storage_key - '#{storage_key}'"

fs_file = FileStorageService.get_string_io(storage_key)
ActionDispatch::Http::UploadedFile.new filename: File.basename(file),
Expand Down
29 changes: 29 additions & 0 deletions spec/controllers/datasets/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,35 @@ def creation_assertions(publishing_method = :github_public)
expect(@user.datasets.first.owner).to eq @user.github_username
end

it 'handles whitespace in filenames' do
expect(GitData).to receive(:create).with(@user.github_username, @name, restricted: false, client: a_kind_of(Octokit::Client)) {
@repo
}

filename = "file with whitespace.csv"
storage_key = "uploads/#{SecureRandom.uuid}/#{filename}"
files = [{
:title => "file with whitespace",
:description => "description",
:file => url_with_stubbed_get_for_storage_key(storage_key, filename),
:storage_key => storage_key
}]

request = post :create, params: { dataset: {
name: dataset_name,
description: description,
publisher_name: publisher_name,
publisher_url: publisher_url,
license: license,
frequency: frequency,
publishing_method: :github_public,
owner: controller.send(:current_user).github_username
}, files: files }

creation_assertions
expect(@user.datasets.first.owner).to eq @user.github_username
end

it 'creates a restricted dataset' do
expect(GitData).to receive(:create).with(@user.github_username, @name, restricted: true, client: a_kind_of(Octokit::Client)) {
@repo
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/datasets/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@
{
title: "New file",
description: "New file description",
file: "http://example.com/new-file.csv",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come example.com is removed in this spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the file key in the S3 uploader is no longer a URL, just the storage key. So I've removed the protocol and port which would have been added before.

file: "new-file.csv",
storage_key: 'new-file.csv'
},
{
Expand All @@ -322,7 +322,7 @@
{
"title" => "New file",
"description" => "New file description",
"file" => "http://example.com/new-file.csv",
"file" => "new-file.csv",
"storage_key" => "new-file.csv"
}
], channel_id: nil
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/file with whitespace.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
hat,size
panama,59.5
stetson,big
2 changes: 1 addition & 1 deletion spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def get_json_from_url(url)

def get_string_io_from_fixture_file(storage_key)
unless storage_key.nil?
filename = storage_key.split('/').last
filename = URI.decode(storage_key).split('/').last
StringIO.new(read_fixture_file(filename))
end
end
Expand Down