Skip to content

Commit

Permalink
Fail on invalid CSV content type (#651)
Browse files Browse the repository at this point in the history
Co-authored-by: Paulo Barros <[email protected]>
  • Loading branch information
george-ma and promulo authored Jun 16, 2022
1 parent cbb77b7 commit fdaa1bf
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 4 deletions.
13 changes: 13 additions & 0 deletions app/models/maintenance_tasks/run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Run < ApplicationRecord
Task.available_tasks.map(&:to_s)
} }
validate :csv_attachment_presence, on: :create
validate :csv_content_type, on: :create
validate :validate_task_arguments, on: :create

attr_readonly :task_name
Expand Down Expand Up @@ -342,6 +343,18 @@ def csv_attachment_presence
nil
end

# Performs validation on the content type of the :csv_file attachment.
# A Run for a Task that uses CsvCollection must have a present :csv_file
# and a content type of "text/csv" to be valid. The appropriate error is
# added if the Run does not meet the above criteria.
def csv_content_type
if csv_file.present? && csv_file.content_type != "text/csv"
errors.add(:csv_file, "must be a CSV")
end
rescue Task::NotFoundError
nil
end

# Support iterating over ActiveModel::Errors in Rails 6.0 and Rails 6.1+.
# To be removed when Rails 6.0 is no longer supported.
if Rails::VERSION::STRING.match?(/^6.0/)
Expand Down
2 changes: 1 addition & 1 deletion app/views/maintenance_tasks/tasks/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<% if @task.csv_task? %>
<div class="block">
<%= form.label :csv_file %>
<%= form.file_field :csv_file %>
<%= form.file_field :csv_file, accept: "text/csv" %>
</div>
<% end %>
<% parameter_names = @task.parameter_names %>
Expand Down
12 changes: 10 additions & 2 deletions lib/maintenance_tasks/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,17 @@ def csv_file
csv_option = options[:csv]

if csv_option == :stdin
{ io: StringIO.new($stdin.read), filename: "stdin.csv" }
{
io: StringIO.new($stdin.read),
filename: "stdin.csv",
content_type: "text/csv",
}
else
{ io: File.open(csv_option), filename: File.basename(csv_option) }
{
io: File.open(csv_option),
filename: File.basename(csv_option),
content_type: "text/csv",
}
end
rescue Errno::ENOENT
raise ArgumentError, "CSV file not found: #{csv_option}"
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/files/sample.tsv
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
title content
My Title 1 Hello World 1!
My Title 2 Hello World 2!
My Title 3 Hello World 3!
My Title 4 Hello World 4!
My Title 5 Hello World 5!
8 changes: 8 additions & 0 deletions test/models/maintenance_tasks/run_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ class RunTest < ActiveSupport::TestCase
refute_predicate run, :valid?
end

test "invalid if content_type is not text/csv" do
run = Run.new(task_name: "Maintenance::ImportPostsTask")
tsv = Rack::Test::UploadedFile.new(file_fixture("sample.tsv"),
"text/tab-separated-values")
run.csv_file.attach(tsv)
refute_predicate run, :valid?
end

test "invalid if associated Task has parameters and they are invalid" do
run = Run.new(
task_name: "Maintenance::ParamsTask",
Expand Down
2 changes: 1 addition & 1 deletion test/models/maintenance_tasks/runner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class RunnerTest < ActiveSupport::TestCase
private

def csv_io
{ io: File.open(@csv), filename: "sample.csv" }
{ io: File.open(@csv), filename: "sample.csv", content_type: "text/csv" }
end
end
end

0 comments on commit fdaa1bf

Please sign in to comment.