Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Incorrect content type for files. #284

Closed
andoriyu opened this issue Apr 18, 2011 · 27 comments
Closed

Incorrect content type for files. #284

andoriyu opened this issue Apr 18, 2011 · 27 comments

Comments

@andoriyu
Copy link

Every time when I'm trying to upload a file from irb, file has content-type 'binary/octet-stream'. Is there any possible way to force change of content-type to correct one before saving the file?

@trevorturk
Copy link
Contributor

I'm assuming you mean when it's uploading to S3? There's an open issue related to this, actually: #254

I think you can make a new CarrierWave::SanitizedFile to set everything manually. See how that works here: https://github.com/jnicklas/carrierwave/blob/master/lib/carrierwave/uploader/cache.rb#L72

@geemus
Copy link
Contributor

geemus commented Apr 18, 2011

FWIW: In fog if you don't specify a content-type it uses the content-type gem to try and guess one. The guess is based entirely on file extension if I remember correctly. So if you have the proper file extensions it should all 'just work'. Barring that, explicitly passing one ought to work (soo also those other tickets though, sounds like there may be some related bugs).

@andoriyu
Copy link
Author

So if i install 'content_type' it will work?

@geemus
Copy link
Contributor

geemus commented Apr 18, 2011

That part should already be done due to dependency resolution actually. If you change the filenames of the things you are uploading it should probably do the right thing (or at least raw fog ought to, if that value doesn't correctly make its way through carrierwave then we have other issues).

@andoriyu
Copy link
Author

Well, I don't have 'content_type' gem, in fact, it fails to build because I don't have libmagic (so, this extension determinate content-type based on "magic number"). The problem is — there is no .content_type method for File provided...

@geemus
Copy link
Contributor

geemus commented Apr 18, 2011

Oh, sorry I was wrong. Sorry, I mis-spoke it is the 'mime-types' gem that does this. And fog requires it, so if you have fog you should also have mime-types.

@andoriyu
Copy link
Author

I have that gem, but content-type is wrong. I also use fog to upload my jekyll website, and all files has correct content-type. I forget to specified, that I'm using ruby 1.9.2p180.

@trevorturk
Copy link
Contributor

Please see my initial comment. I think you'll have to use CarrierWave::SanitizedFile to set the content_type.

@andoriyu
Copy link
Author

andoriyu commented May 8, 2011

I actually don't get what you suggested ...

@JangoSteve
Copy link
Contributor

I had this problem, so I put this in my PageUploader (i.e. the particular uploader I created):

require 'mime/types'

class PageUploader < CarrierWave::Uploader::Base
  ...
  process :set_content_type

  def set_content_type(*args)
    content_type = file.content_type == 'application/octet-stream' || file.content_type.blank? ? MIME::Types.type_for(original_filename).first : file.content_type

    self.file.instance_variable_set(:@content_type, content_type)
  end
end

Obviously, I'm thinking this would be a little better if the CarrierWave::SanitizedFile class had a content_type= method, so that I didn't need to use the instance_variable_set hack.

However, that aside, now I'm getting this error:

Excon::Errors::Forbidden in ReleasesController#create

Expected(200) <=> Actual(403 Forbidden)
  ...
  response => #<Excon::Response:0x42997e0 @body="<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message>
  ...

EDIT: The good news at least is that I inspected the request sent by Fog, and it does indeed include Content-Type => 'text/html'. So I'm closer. I'm guessing some sort of checksum signature is getting generated for the request before the new content-type is being set, so that once it is set/changed, it no longer matches the checksum. But I am new to Fog/CarrierWave, so I don't know.

@JangoSteve
Copy link
Contributor

Woo, figured it out! It looks like my above solution actually does work, but for one small change. I needed to add .to_s to the following:

MIME::Types.type_for(original_filename).first

To make it:

MIME::Types.type_for(original_filename).first.to_s

And that did the trick. Now my content_types are uploading and getting set properly in my S3 bucket.

@andoriyu
Copy link
Author

I will try that. Thx.

@chrisbloom7
Copy link

@JangoSteve - thanks for the suggestion, but unfortunately that method stops working as soon as you have something like this in in your uploader:

process :convert => 'png'

This is due to the fact that the temp file retains to original file name including the original extension, which the mime-type gem uses to determine the content type. Even if you have a filename() method that renames the file to whatever.png, the file isn't actually renamed until it is copied from the temp folder to the final destination.

@JangoSteve
Copy link
Contributor

@chrisbloom7, I'm confused. If you know you're converting the file to a png, why not just set the content_type to png, instead of using the mime-type gem?

class PageUploader < CarrierWave::Uploader::Base
  ...
  process :set_content_type

  def set_content_type(*args)
    self.file.instance_variable_set(:@content_type, "image/png")
  end
end

@chrisbloom7
Copy link

That is what I ended up doing. I was just pointing out that you can't do what you suggested in cases like that, just in case anyone else should try it and experience the same problem I did.

@trevorturk
Copy link
Contributor

Hrm... perhaps worthwhile adding something to the wiki? Or maybe it's possible to fix in Carrierwave itself...?

@JangoSteve
Copy link
Contributor

I could probably submit a patch to carrierwave to make this much easier, if you'd like.

@JangoSteve
Copy link
Contributor

If anyone is interested, I've submitted a pull request which actually adds set_content_type using the mime-types gem as a processor (so that mime-types doesn't become a hard dependency). See pull request here.

@nickhoffman
Copy link
Contributor

Hey guys. This problem's biting me. Is there a better solution than what @JangoSteve suggested above on May 15? I see that a couple of commits were made to try to fix it, but at least one (#254) was reverted.

@nickhoffman
Copy link
Contributor

After stumbling upon lib/carrierwave/processing/mime_types.rb , I figured out how to get the correct content-type set on files and their versions:

  1. Add require 'carrierwave/processing/mime_types' to an initializer or your uploader(s).
  2. Add include CarrierWave::MimeTypes to your uploader.
  3. Add process :set_content_type to your uploader(s).
  4. Grab a beer.

My question now is: Why is this not done by default?

@JangoSteve
Copy link
Contributor

I meant to write up a quick post on the new MimeTypes processor which I commented about in this thread on June 11, but yeah it's basically as easy as @nickhoffman described.

The reason it's not done by default, and why I built it as a processor, is because it requires a dependency on the mime-types gem in order to provide more in-depth file-type guessing than most people who use carrierwave probably need. The goal is to keep carrierwave small, modular, and having as few required dependencies as possible.

@nickhoffman
Copy link
Contributor

@JangoSteve Ah, that makes sense re: minimizing CarrierWave's dependencies.

I've just submitted pull request #440, which adds a section to the README that explains how to use CarrierWave::MimeTypes to set more-specific content-type values.

@fbjork
Copy link
Contributor

fbjork commented Sep 13, 2011

I cannot get this to work using the latest carrierwave-mongoid. I've set the set_content_type directive, the Content-Type will always be set to the original file rather than the converted format.

@trevorturk
Copy link
Contributor

@fbjork is this a problem with carriewwave or carrierwave-mongoid? Can you make a failing test case and open an issue where appropriate? Thanks!

@tribalvibes
Copy link

@nickhoffman to answer your question, it's because this is a giant hairball with a broken model of one output file plus a bunch of "versions" for each input upload. Now of course you want to upload them all to e.g. s3 with the correct types for each version, and that just doesn't fit well in this model.... to say nothing of the remarkable! coding techniques used. You might find that simply using rack, tempfile, rmagick and fog directly is a much more expedient and less painful way to go.
@JangoSteve thanks to you and Goog, saved another infuriating morning with cw

@bvishny
Copy link

bvishny commented May 24, 2012

I put "process :set_content_type" at the top of the uploader and while it fixes the problem for the main image, it does not solve the problem for the different versions. Here is my code:

Process files as they are uploaded:

process :convert => 'png'
process :set_content_type

version :product do
process :convert => 'png'
process :set_content_type
process :resize_to_fit => [200,230]
end

version :storefront do
process :convert => 'png'
process :set_content_type
process :resize_to_fit => [160,200]
end

@cyrilchampier
Copy link

Like bvishny, I can't make it works for version with different types.
I am forced to do something like this:

version :preprocessed do
process :preprocess
process :set_content_type
def full_filename(for_file)
super.chomp(File.extname(super)) + '.zip'
end
def set_content_type(*args)
self.file.instance_variable_set(:@content_type, "application/zip")
end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants