This project is archived and is in readonly mode.

#6698 new
Ryan D Johnson

Rack::Request#content_charset is always blank due to ActionDispatch::MimeNegotation#content_type

Reported by Ryan D Johnson | April 13th, 2011 @ 05:14 AM

It looks like #4109 was intended to fix this in commit 77a2a3d9b3aa461437ced326ea4a70112a8c68ed -- but the content_type method introduced there doesn't do what it needs to. Probably it should be omitted in favor of letting Rack::Request#content_type do its normal thing.

Here's an overview of the problem:

ActionDispatch::Request includes ActionDispatch::Http::MimeNegotiation, which overrides content_type from Rack::Request

Rack::Requset#content_type just dumps env['CONTENT_TYPE'], which will include the charset:

def content_type
  content_type = @env['CONTENT_TYPE']
  content_type.nil? || content_type.empty? ? nil : content_type
end

ActionDispatch::Http::MimeNegotiation does a Mime::Type#lookup as content_mime_type (the Mime::Type used to come back directly from #content_type; that's what 77a2a3 fixed, returning the mime type string instead).

  def content_mime_type
    @env["action_dispatch.request.content_type"] ||= begin
      if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/
        Mime::Type.lookup($1.strip.downcase)
      else
        nil
      end
    end
  end

  def content_type
    content_mime_type && content_mime_type.to_s
  end

Unfortunately, although the new content_type implementation returns a string, it's stripped of the charset. This renders Rack::Request#content_charset (and #media_type_params) useless:

def media_type_params
  return {} if content_type.nil?
  Hash[*content_type.split(/\s*[;,]\s*/)[1..-1].
    collect { |s| s.split('=', 2) }.
    map { |k,v| [k.downcase, v] }.flatten]
end

def content_charset
  media_type_params['charset']
end

It would be really great if request.body could end up with the proper encoding at the end of the day. But even if that isn't going to happen right away, at least request.raw_post.force_encoding( request.content_charset ) should work. As is, I have to reparse the request.env['CONTENT_TYPE'] to get the charset.

Attaching a patch which implements the suggestion. The unit test modification leads to a failure without the lib change and passes with it. All other action pack tests pass with the patch.

Comments and changes to this ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages