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

Use file content_type when file is ActionDispatch::Http::UploadedFile #155

Merged
merged 2 commits into from Mar 11, 2014

Conversation

Projects
None yet
3 participants
Contributor

vkmita commented Mar 10, 2014

@steved555 @grosser @mcodella
https://zendesk.atlassian.net/browse/HC-2715

When uploading attachments using ActionDispatch::Http::UploadedFile, the temp_file path does not resolve to any mime type - i.e. /var/folders/6l/2c1qltgj1p3526rgkkb2vpkr0000gp/T/RackMultipart20140310-86212-16fkt8o - but the ActionDispatch::Http::UploadedFile has the correct content_type.

This should take care of that case. Let me know if this shouldn't be taken care of as a general solution in this gem.

@steved steved commented on an outdated diff Mar 10, 2014

lib/zendesk_api/middleware/request/upload.rb
@@ -49,7 +49,11 @@ def set_file(hash, key, top_level)
hash = hash[key]
end
- mime_type = MIME::Types.type_for(path).first || "application/octet-stream"
+ mime_type = MIME::Types.type_for(path).first
@steved

steved Mar 10, 2014

Contributor

Maybe set mime_type = nil where the file is set, the override it inside the same defined?(ActionDispatch) && file.is_a?(ActionDispatch::Http::UploadedFile) block, while finally adding these two as fallbacks:

file = ...
mime_type = nil

case file
when ...
when ..
else
  if defined?(ActionDispatch) && file.is_a?(ActionDispatch::Http::UploadedFile)
   path = ...
   mime_type = file.content_type
  else
    ...
end

if path
  mime_type ||= MIME::Types || ...
end

Not sure if it's cleaner, but it keeps the ActionDispatch logic in one place.

Contributor

steved commented Mar 10, 2014

Either way, this fixes it so 👍. Do we need a patch release for < 1.0?

Owner

grosser commented Mar 10, 2014

👍

Contributor

vkmita commented Mar 11, 2014

I'm going to try to upgrade HC to the most recent > 1.0, we'll see...

vkmita pushed a commit that referenced this pull request Mar 11, 2014

Merge pull request #155 from zendesk/vkmita/HC-2715-fix-content-type-…
…upload

Use file content_type when file is ActionDispatch::Http::UploadedFile

@vkmita vkmita merged commit 0b0be43 into master Mar 11, 2014

1 check passed

default The Travis CI build passed
Details

@vkmita vkmita deleted the vkmita/HC-2715-fix-content-type-upload branch Mar 11, 2014

Contributor

vkmita commented Mar 11, 2014

Yup, working on a patch release for <1.0

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