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

Already on GitHub? Sign in to your account

Fix issue with content-disposition existing with no filename when downloading #1323

Merged
merged 2 commits into from Feb 24, 2014

Conversation

Projects
None yet
2 participants
Owner

bensie commented Feb 18, 2014

Another attempt at fixing #1269

/cc @taavo

@taavo taavo commented on an outdated diff Feb 19, 2014

lib/carrierwave/uploader/download.rb
@@ -48,8 +48,8 @@ def file
def filename_from_header
if file.meta.include? 'content-disposition'
- match = file.meta['content-disposition'].match(/filename=(\"?)(.+)\1/)
- return match[2] unless match.nil?
+ match = file.meta['content-disposition'].match(/filename="?([^"]+)/)
+ return match[ 1 ] if !match.nil? && !match[ 1 ].empty?
@taavo

taavo Feb 19, 2014

Member

It looks good, but without further review I'm not sure why it's better to return nil if the match is empty.

bensie added a commit that referenced this pull request Feb 24, 2014

Merge pull request #1323 from carrierwaveuploader/content-dispo
Fix issue with content-disposition existing with no filename when downloading

@bensie bensie merged commit 565f19e into master Feb 24, 2014

@bensie bensie deleted the content-dispo branch Feb 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment