From b47ffd392af9ead973297deee170b0d6225a42a3 Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Mon, 10 Mar 2008 03:31:19 -0400 Subject: [PATCH] HEAD support and Static as GET/HEAD only event. Implement HEAD by delegating to GET handlers (when no HEAD handler registered) and removing response body. This fixes a problem where Sinatra sent a 404 response to HEAD requests due to HEAD not having any registered handlers. While here, make Static handlers respond only to GET and HEAD requests instead of using GET semantics for PUT/POST/DELETE. Makes it possible for Events to register PUT/POST/DELETE handlers for static file URLs. For instance, assuming a file exists, `public/foo.xml`, and the following event: put '/foo.xml' do File.open('public/foo.xml', 'wb') do |io| io.write(request.body.read) end '' end get '/foo.xml' do "THIS NEVER HAPPENS ... as long as /foo.xml exists on disk." end The built-in Static handler hits on GET and the dynamic event hits on PUT. An important note here is that the Static handler is now registered at the head of the events[:get] array (see Application#load_default_events! and Application#lookup), where it was previously a special case in the lookup method. --- lib/sinatra.rb | 35 ++++++++++++++++++--------- lib/sinatra/test/methods.rb | 6 +++++ test/app_test.rb | 15 ++++++++++-- test/streaming_test.rb | 47 +++++++++++++++++++++++++++++++++++-- 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/lib/sinatra.rb b/lib/sinatra.rb index b9872200..7f4a0b69 100644 --- a/lib/sinatra.rb +++ b/lib/sinatra.rb @@ -586,7 +586,14 @@ module Sinatra op.on('-e env') { |env| default_options[:env] = env } end.parse!(ARGV.dup.select { |o| o !~ /--name/ }) end - + + # Called immediately after the application is initialized or reloaded to + # register default events. Events added here have dibs on requests since + # they appear first in the list. + def load_default_events! + events[:get] << Static.new + end + def initialize @clearables = [ @events = Hash.new { |hash, key| hash[key] = [] }, @@ -595,8 +602,9 @@ module Sinatra @layouts = Hash.new ] load_options! + load_default_events! end - + def define_event(method, path, options = {}, &b) events[method] << event = Event.new(path, options, &b) event @@ -613,17 +621,20 @@ module Sinatra def define_filter(type, &b) filters[:before] << b end - - def static - @static ||= Static.new - end - + + # Visits and invokes each handler registered for the +request_method+ in + # definition order until a Result response is produced. If no handler + # responds with a Result, the NotFound error handler is invoked. + # + # When the request_method is "HEAD" and no valid Result is produced by + # the set of handlers registered for HEAD requests, an attempt is made to + # invoke the GET handlers to generate the response before resorting to the + # default error handler. def lookup(request) method = request.request_method.downcase.to_sym - e = static.invoke(request) - e ||= events[method].eject(&[:invoke, request]) - e ||= (errors[NotFound]).invoke(request) - e + events[method].eject(&[:invoke, request]) || + (events[:get].eject(&[:invoke, request]) if method == :head) || + errors[NotFound].invoke(request) end def options @@ -637,6 +648,7 @@ module Sinatra def reload! @reloading = true clearables.each(&:clear) + load_default_events! Kernel.load $0 @reloading = false Environment.setup! @@ -669,6 +681,7 @@ module Sinatra body = returned.to_result(context) end body = '' unless body.respond_to?(:each) + body = '' if request.request_method.upcase == 'HEAD' context.body = body.kind_of?(String) ? [*body] : body context.finish end diff --git a/lib/sinatra/test/methods.rb b/lib/sinatra/test/methods.rb index 11fea42f..d9aca301 100644 --- a/lib/sinatra/test/methods.rb +++ b/lib/sinatra/test/methods.rb @@ -20,6 +20,12 @@ module Sinatra @response = @request.get(path, :input => params.to_params, :agent => agent) end + def head_it(path, params = {}) + agent = params.delete(:agent) + @request = Rack::MockRequest.new(Sinatra.application) + @response = @request.request('HEAD', path, :input => params.to_params, :agent => agent) + end + def post_it(path, params = {}) agent = params.delete(:agent) @request = Rack::MockRequest.new(Sinatra.application) diff --git a/test/app_test.rb b/test/app_test.rb index f7a704ef..cfac09be 100644 --- a/test/app_test.rb +++ b/test/app_test.rb @@ -72,6 +72,18 @@ context "Sinatra" do body.should.equal 'Hello!' end + + specify "delegates HEAD requests to GET handlers" do + get '/invisible' do + "I am invisible to the world" + end + + head_it '/invisible' + should.be.ok + body.should.not.equal "I am invisible to the world" + body.should.equal '' + end + specify "put'n with POST" do put '/' do @@ -99,8 +111,7 @@ context "Sinatra" do body.should.equal 'puted' end - # We want to make sure we ignore any _method parameters specified in GET - # requests or on the query string in POST requests. + # Ignore any _method parameters specified in GET requests or on the query string in POST requests. specify "not put'n with GET" do get '/' do 'getted' diff --git a/test/streaming_test.rb b/test/streaming_test.rb index 324bee9c..325eb645 100644 --- a/test/streaming_test.rb +++ b/test/streaming_test.rb @@ -2,15 +2,58 @@ require File.dirname(__FILE__) + '/helper' context "Static files (by default)" do - specify "are served from root/public" do + setup do + Sinatra.application = nil Sinatra.application.options.public = File.dirname(__FILE__) + '/public' + end + + specify "are served from root/public" do get_it '/foo.xml' should.be.ok headers['Content-Length'].should.equal '12' headers['Content-Type'].should.equal 'application/xml' body.should.equal "\n" end - + + specify "are not served when verb is not GET or HEAD" do + post_it '/foo.xml' + # these should actually be giving back a 405 Method Not Allowed but that + # complicates the routing logic quite a bit. + should.be.not_found + status.should.equal 404 + end + + specify "are served when verb is HEAD but missing a body" do + head_it '/foo.xml' + should.be.ok + headers['Content-Length'].should.equal '12' + headers['Content-Type'].should.equal 'application/xml' + body.should.equal "" + end + + # static files override dynamic/internal events and ... + specify "are served when conflicting events exists" do + get '/foo.xml' do + 'this is not foo.xml!' + end + get_it '/foo.xml' + should.be.ok + body.should.equal "\n" + end + + specify "are irrelevant when request_method is not GET/HEAD" do + put '/foo.xml' do + 'putted!' + end + put_it '/foo.xml' + should.be.ok + body.should.equal 'putted!' + + get_it '/foo.xml' + should.be.ok + body.should.equal "\n" + end + end context "SendData" do