Internal Sinatra errors now extend Sinatra::Error
By extending Sinatra::Error, an error class can set the http status code on the response to a value other than 500. This commit fixes issues #1204 and #1518 where an error raised by a third party library that responded to http_status could set the status on the response. Any error outside of Sinatra errors will now always return a 500 status. This fixes an issue where an exception could leak sensitive data in the message to the browser. Errors that have http_status code 400 or 404 use the message as the body of the response. This is why it is imperative that these errors extend Sinatra::Error so that this is an explicit decision.
This commit is contained in:
parent
9a924aa894
commit
bda8c29d70
|
@ -257,11 +257,14 @@ module Sinatra
|
|||
end
|
||||
end
|
||||
|
||||
class BadRequest < TypeError #:nodoc:
|
||||
class Error < StandardError #:nodoc:
|
||||
end
|
||||
|
||||
class BadRequest < Error #:nodoc:
|
||||
def http_status; 400 end
|
||||
end
|
||||
|
||||
class NotFound < NameError #:nodoc:
|
||||
class NotFound < Error #:nodoc:
|
||||
def http_status; 404 end
|
||||
end
|
||||
|
||||
|
@ -1151,14 +1154,17 @@ module Sinatra
|
|||
end
|
||||
@env['sinatra.error'] = boom
|
||||
|
||||
if boom.respond_to? :http_status and boom.http_status.between? 400, 599
|
||||
status(boom.http_status)
|
||||
elsif settings.use_code? and boom.respond_to? :code and boom.code.between? 400, 599
|
||||
status(boom.code)
|
||||
else
|
||||
status(500)
|
||||
http_status = if boom.kind_of? Sinatra::Error
|
||||
if boom.respond_to? :http_status
|
||||
boom.http_status
|
||||
elsif settings.use_code? && boom.respond_to?(:code)
|
||||
boom.code
|
||||
end
|
||||
end
|
||||
|
||||
http_status = 500 unless http_status && http_status.between?(400, 599)
|
||||
status(http_status)
|
||||
|
||||
if server_error?
|
||||
dump_errors! boom if settings.dump_errors?
|
||||
raise boom if settings.show_exceptions? and settings.show_exceptions != :after_handler
|
||||
|
|
|
@ -6,15 +6,15 @@ end
|
|||
class FooNotFound < Sinatra::NotFound
|
||||
end
|
||||
|
||||
class FooSpecialError < RuntimeError
|
||||
class FooSpecialError < Sinatra::Error
|
||||
def http_status; 501 end
|
||||
end
|
||||
|
||||
class FooStatusOutOfRangeError < RuntimeError
|
||||
class FooStatusOutOfRangeError < Sinatra::Error
|
||||
def code; 4000 end
|
||||
end
|
||||
|
||||
class FooWithCode < RuntimeError
|
||||
class FooWithCode < Sinatra::Error
|
||||
def code; 419 end
|
||||
end
|
||||
|
||||
|
|
|
@ -1,5 +1,9 @@
|
|||
require File.expand_path('../helper', __FILE__)
|
||||
|
||||
class ThirdPartyError < RuntimeError
|
||||
def http_status; 400 end
|
||||
end
|
||||
|
||||
class ResultTest < Minitest::Test
|
||||
it "sets response.body when result is a String" do
|
||||
mock_app { get('/') { 'Hello World' } }
|
||||
|
@ -73,4 +77,15 @@ class ResultTest < Minitest::Test
|
|||
assert_equal 205, status
|
||||
assert_equal '', body
|
||||
end
|
||||
|
||||
it "sets status to 500 when raised error is not Sinatra::Error" do
|
||||
mock_app do
|
||||
set :raise_errors, false
|
||||
get('/') { raise ThirdPartyError }
|
||||
end
|
||||
|
||||
get '/'
|
||||
assert_equal 500, status
|
||||
assert_equal '<h1>Internal Server Error</h1>', body
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue