rails / rails

Ruby on Rails
https://rubyonrails.org
MIT License
55.63k stars 21.56k forks source link

Parsing of Accept header in request fails on quoted-strings #33980

Open tkalliom opened 5 years ago

tkalliom commented 5 years ago

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"
gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "rails", github: "rails/rails"
end

require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = __dir__
  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get "/greet" => "test#greet"
  end
end

class TestController < ActionController::Base
  def greet
    respond_to do |format|
      format.csv {render plain: "Hello"}
      format.text {render plain: "Hello"}
    end
  end
end

require "minitest/autorun"
require "rack/test"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_media_type_parameters
    get "/greet", nil, { 'HTTP_ACCEPT' => 'text/html; someparameter="a, \"quoted, string, text/csv, etc", text/plain' }
    assert last_response.ok?
    assert_equal "text/plain; charset=utf-8", last_response.content_type
    assert_equal "Hello", last_response.body
  end

  private
    def app
      Rails.application
    end
end

Expected behavior

As per RFC 7231 sections 5.3.2 and 3.1.1.1, the header sent by the client should be understood as having two media type specifications; the first text/html with the media type parameter someparameter having the value a, "quoted, string, text/csv, etc and the second text/plain with no media type parameters. The content negotiation should thus result in choosing text/plain.

Actual behavior

Failure:
BugTest#test_media_type_parameters [accepts.rb:40]:
Expected: "text/plain; charset=utf-8"
  Actual: "text/csv; charset=utf-8"

The header parsing fails to take quoted-strings into account, and text/csv is mistakenly interpreted as an accepted media type. The response is sent as CSV.

System configuration

Rails version: 6.0.0alpha (master)

Ruby version: 2.4.1p111

brendo commented 5 years ago

Had a look into this. The logic for parsing Accept headers lives in Mime::Type inside the private parse method.

For the specific scenario above, having commas within a quoted string is not handled, and causes the header to split in several chunks:

"text/html; someparameter=\"a"
" \\\"quoted"
" string"
" text/csv"
" etc\""
" text/plain"