From 0d85006d01afca1b4d7454cde469ca113963ff01 Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Sat, 31 Dec 2011 12:46:41 +0100 Subject: [PATCH] deprecate Exception#code usage, ignore values that are not between 400 and 599. fixes #423. --- CHANGES | 4 ++++ lib/sinatra/base.rb | 17 +++++++++++++++-- test/mapped_error_test.rb | 19 +++++++++++++++++-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index c1055b29..aefd0b61 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,10 @@ * No longer include Sinatra::Delegator in Object, instead extend the main object only. (Konstantin Haase) + * Exception#code is only used when :use_code is enabled and displays a warning. + Moreover, it will be ignored if the value is not between 400 and 599. You + should use Exception#http_status instead. (Konstantin Haase) + = 1.3.2 / 2011-12-30 * Don't automatically add `Rack::CommonLogger` if `Rack::Server` is adding it, diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index 893016ef..9731ef3c 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -88,7 +88,7 @@ module Sinatra end class NotFound < NameError #:nodoc: - def code ; 404 ; end + def http_status; 404 end end # Methods available to routes, before/after filters, and views. @@ -893,7 +893,19 @@ module Sinatra # Error handling during requests. def handle_exception!(boom) @env['sinatra.error'] = boom - status boom.respond_to?(:code) ? Integer(boom.code) : 500 + + if boom.respond_to? :http_status + status(boom.http_status) + elsif settings.use_code? and boom.respond_to? :code and boom.code.between? 400, 599 + status(boom.code) + warn "Using #{boom.class}#code (#{status}) for setting status code. This is deprecated. " \ + "Use #http_status instead. If this happens unintentional, please \`disable :use_code\`" \ + " in your application." + else + status(500) + end + + status(500) unless status.between? 400, 599 if server_error? dump_errors! boom if settings.dump_errors? @@ -1491,6 +1503,7 @@ module Sinatra set :logging, false set :protection, true set :method_override, false + set :use_code, true set :default_encoding, "utf-8" set :add_charset, %w[javascript xml xhtml+xml json].map { |t| "application/#{t}" } settings.add_charset << /^text\// diff --git a/test/mapped_error_test.rb b/test/mapped_error_test.rb index 0a16af73..5a96da03 100644 --- a/test/mapped_error_test.rb +++ b/test/mapped_error_test.rb @@ -7,9 +7,14 @@ class FooNotFound < Sinatra::NotFound end class FooSpecialError < RuntimeError - def code; 501 end + def http_status; 501 end end +class FooStatusOutOfRangeError < RuntimeError + def code; 4000 end +end + + class MappedErrorTest < Test::Unit::TestCase def test_default assert true @@ -182,7 +187,7 @@ class MappedErrorTest < Test::Unit::TestCase assert_equal 'subclass', body end - it 'honors Exception#code if present' do + it 'honors Exception#http_status if present' do mock_app do set :raise_errors, false error(501) { 'Foo!' } @@ -192,6 +197,16 @@ class MappedErrorTest < Test::Unit::TestCase assert_equal 501, status assert_equal 'Foo!', body end + + it 'does not rely on Exception#code for invalid codes' do + mock_app do + set :raise_errors, false + get('/') { raise FooStatusOutOfRangeError } + end + get '/' + assert_equal 500, status + end + end describe 'Custom Error Pages' do