From 4f35139b594c6a56acff2f8ceb71000642518d6d Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Fri, 25 Mar 2011 13:49:01 +0100 Subject: [PATCH] Improve handling of Accept header: * allow wild cards * respect preferences * treat both text/xml and application/xml as xml * treat both text/javascript and application/javascript as js This impacts mainly the `provides` condition, but should improve the behavior of any code using Request#accept and or Request#preferred_type. Tests included. Fixes #230. --- lib/sinatra/base.rb | 44 ++++++++++++++++++++++----- test/routing_test.rb | 71 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index e29a7cc6..070c6c13 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -13,9 +13,22 @@ module Sinatra class Request < Rack::Request # Returns an array of acceptable media types for the response def accept - @env['HTTP_ACCEPT'].to_s.split(',').map { |a| a.split(';')[0].strip } + @env['sinatra.accept'] ||= begin + entries = @env['HTTP_ACCEPT'].to_s.split(',') + entries.map { |e| accept_entry(e) }.sort_by(&:last).map(&:first) + end end + def preferred_type(*types) + return accept.first if types.empty? + types.flatten! + accept.detect do |pattern| + type = types.detect { |t| File.fnmatch(pattern, t) } + return type if type + end + end + + alias accept? preferred_type alias secure? ssl? def forwarded? @@ -33,6 +46,15 @@ module Sinatra @route = nil super end + + private + + def accept_entry(entry) + type, *options = entry.gsub(/\s/, '').split(';') + quality = 0 # we sort smalles first + options.delete_if { |e| quality = 1 - e[2..-1].to_f if e.start_with? 'q=' } + [type, [quality, type.count('*'), 1 - options.size]] + end end # The response object. See Rack::Response and Rack::ResponseHelpers for @@ -150,7 +172,8 @@ module Sinatra # Set the Content-Type of the response body given a media type or file # extension. - def content_type(type, params={}) + def content_type(type = nil, params={}) + return response['Content-Type'] unless type default = params.delete :default mime_type = mime_type(type) || default fail "Unknown media type: %p" % type if mime_type.nil? @@ -1007,6 +1030,14 @@ module Sinatra Rack::Mime::MIME_TYPES[type] = value end + # provides all mime types matching type, including deprecated types: + # mime_types :html # => ['text/html'] + # mime_types :js # => ['application/javascript', 'text/javascript'] + def mime_types(type) + type = mime_type type + type =~ /^application\/(xml|javascript)$/ ? [type, "text/#$1"] : [type] + end + # Define a before filter; runs before all requests within the same # context as route handlers and may access/modify the request and # response. @@ -1059,12 +1090,11 @@ module Sinatra # Condition for matching mimetypes. Accepts file extensions. def provides(*types) - types.map! { |t| mime_type(t) } - + types.map! { |t| mime_types(t) } + types.flatten! condition do - matching_types = (request.accept & types) - unless matching_types.empty? - content_type matching_types.first + if type = request.preferred_type(types) + content_type(type) true else false diff --git a/test/routing_test.rb b/test/routing_test.rb index c36a6f95..189881d3 100644 --- a/test/routing_test.rb +++ b/test/routing_test.rb @@ -724,6 +724,77 @@ class RoutingTest < Test::Unit::TestCase assert_equal 'default', body end + it 'respects user agent prefferences for the content type' do + mock_app { get('/', :provides => [:png, :html]) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => 'image/png;q=0.5,text/html;q=0.8' } + assert_body 'text/html;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'image/png;q=0.8,text/html;q=0.5' } + assert_body 'image/png' + end + + it 'accepts generic types' do + mock_app do + get('/', :provides => :xml) { content_type } + get('/') { 'no match' } + end + get '/' + assert_body 'no match' + get '/', {}, { 'HTTP_ACCEPT' => 'foo/*' } + assert_body 'no match' + get '/', {}, { 'HTTP_ACCEPT' => 'application/*' } + assert_body 'application/xml;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => '*/*' } + assert_body 'application/xml;charset=utf-8' + end + + it 'prefers concrete over partly generic types' do + mock_app { get('/', :provides => [:png, :html]) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => 'image/*, text/html' } + assert_body 'text/html;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'image/png, text/*' } + assert_body 'image/png' + end + + it 'prefers concrete over fully generic types' do + mock_app { get('/', :provides => [:png, :html]) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => '*/*, text/html' } + assert_body 'text/html;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'image/png, */*' } + assert_body 'image/png' + end + + it 'prefers partly generic over fully generic types' do + mock_app { get('/', :provides => [:png, :html]) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => '*/*, text/*' } + assert_body 'text/html;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'image/*, */*' } + assert_body 'image/png' + end + + it 'respects quality with generic types' do + mock_app { get('/', :provides => [:png, :html]) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => 'image/*;q=1, text/html;q=0' } + assert_body 'image/png' + get '/', {}, { 'HTTP_ACCEPT' => 'image/png;q=0.5, text/*;q=0.7' } + assert_body 'text/html;charset=utf-8' + end + + it 'accepts both text/javascript and application/javascript for js' do + mock_app { get('/', :provides => :js) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => 'application/javascript' } + assert_body 'application/javascript;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'text/javascript' } + assert_body 'text/javascript;charset=utf-8' + end + + it 'accepts both text/xml and application/xml for xml' do + mock_app { get('/', :provides => :xml) { content_type }} + get '/', {}, { 'HTTP_ACCEPT' => 'application/xml' } + assert_body 'application/xml;charset=utf-8' + get '/', {}, { 'HTTP_ACCEPT' => 'text/xml' } + assert_body 'text/xml;charset=utf-8' + end + it 'passes a single url param as block parameters when one param is specified' do mock_app { get '/:foo' do |foo|