Skip to content

Commit

Permalink
LIBITD-1135. Updated Paperclip gem to v5.3.0
Browse files Browse the repository at this point in the history
Updated Paperclip gem from v5.0.0 to v5.3.0 to address gem
vulnerabilities.

Ran "bundle update paperclip" to update related gems.

Fixed a test that began failing due to better file content detection
in the new Rails version (see
thoughtbot/paperclip#2270). Prior to the
upgrade, the test was actually providing a PDF file, but the file was
being rejected because the provided MIME type was given as "text/html".
After the upgrade, the provided MIME type is not trusted, and the file
itself is examined. Since the file provided in the test is actually a
PDF file, the file is correctly identified as a PDF, and is accepted, so
the test failed. Updated the test to use a non-PDF file.

Also removed "before_validation" code in the
"student-applications/app/models/resume.rb" file, as the default
content type detector in Paperclip now performs a similar check
(see https://github.com/thoughtbot/paperclip/blob/v5.3.0/lib/paperclip/content_type_detector.rb).

https://issues.umd.edu/browse/LIBITD-1135
  • Loading branch information
dsteelma-umd committed Aug 20, 2018
1 parent cda6d3b commit ee833a7
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 25 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ gem 'country_select'
gem 'simple_form'
gem 'will_paginate-bootstrap'

gem 'paperclip', '~> 5.0.0'
gem 'paperclip', '~> 5.3.0'

gem 'daemons'
gem 'delayed_job_active_record'
Expand Down
12 changes: 6 additions & 6 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,8 @@ GEM
capybara-screenshot (1.0.14)
capybara (>= 1.0, < 3)
launchy
climate_control (0.1.0)
climate_control (0.2.0)
cliver (0.3.2)
cocaine (0.5.8)
climate_control (>= 0.0.3, < 1.0)
cocoon (1.2.9)
coderay (1.1.1)
concurrent-ruby (1.0.5)
Expand Down Expand Up @@ -200,12 +198,12 @@ GEM
notiffany (0.1.1)
nenv (~> 0.1)
shellany (~> 0.0)
paperclip (5.0.0)
paperclip (5.3.0)
activemodel (>= 4.2.0)
activesupport (>= 4.2.0)
cocaine (~> 0.5.5)
mime-types
mimemagic (~> 0.3.0)
terrapin (~> 0.6.0)
parser (2.3.3.1)
ast (~> 2.2)
pg (0.19.0)
Expand Down Expand Up @@ -319,6 +317,8 @@ GEM
net-scp (>= 1.1.2)
net-ssh (>= 2.8.0)
terminal-notifier-guard (1.7.0)
terrapin (0.6.0)
climate_control (>= 0.0.3, < 1.0)
test_after_commit (1.1.0)
activerecord (>= 3.2)
thor (0.20.0)
Expand Down Expand Up @@ -373,7 +373,7 @@ DEPENDENCIES
minitest-rails-capybara
minitest-reporters
mocha
paperclip (~> 5.0.0)
paperclip (~> 5.3.0)
pg
poltergeist
pry-rails
Expand Down
12 changes: 0 additions & 12 deletions app/models/resume.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,6 @@
class Resume < ActiveRecord::Base
has_attached_file :file

# fix up the MIME type using server-side detection, to overcome browsers
# sometimes sending incorrect Content-Type headers with PDF files
# adapted from http://stackoverflow.com/a/7000208/5124907
GENERIC_CONTENT_TYPES = ['application/force-download', 'application/octet-stream', 'application/x-download'].freeze

before_validation(on: [:create, :update]) do |resume|
if GENERIC_CONTENT_TYPES.include?(resume.file_content_type)
mime_type = MIME::Types.type_for(resume.file_file_name)
resume.file_content_type = mime_type.first.content_type if mime_type.first
end
end

validates_attachment_presence :file
validates_attachment :file, content_type: { content_type: 'application/pdf' }

Expand Down
12 changes: 6 additions & 6 deletions test/controllers/resume_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ResumesControllerTest < ActionController::TestCase
end

test "should not create a new resume if its not a pdf" do
file = fixture_file_upload( "resume.pdf", 'text/html' )
file = fixture_file_upload( "resume.notpdf", 'text/html' )
refute_difference( 'Resume.count' ) do
post :create, resume: { file: file }
end
Expand All @@ -60,25 +60,25 @@ class ResumesControllerTest < ActionController::TestCase
get :show, id: resume.id
assert_response(403)
end

test 'should not allow anyone not logged in to view a submitted resume' do
prospect = prospects(:all_valid)
prospect = prospects(:all_valid)
prospect.build_resume( file: File.new('test/fixtures/resume.pdf', 'r'))
prospect.save

get :show, id: prospect.resume_id
assert_response(403)
end

test 'should allow authed users to view any resume' do
prospect = prospects(:all_valid)
prospect = prospects(:all_valid)
prospect.build_resume( file: File.new('test/fixtures/resume.pdf', 'r'))
prospect.save

session[:cas] = { user: "admin" }
get :show, id: prospect.resume_id
assert_response :success

resume = Resume.create(file: File.new('test/fixtures/resume.pdf', 'r'))
get :show, id: resume.id
assert_response :success
Expand Down

0 comments on commit ee833a7

Please sign in to comment.