Merge pull request #1070 from jkowens/fix-1058
Add error handling for requests with invalid params (Fixes #1058)
This commit is contained in:
commit
939ce04c1b
|
@ -73,6 +73,12 @@ module Sinatra
|
||||||
request_method == "UNLINK"
|
request_method == "UNLINK"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def params
|
||||||
|
super
|
||||||
|
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
|
||||||
|
raise BadRequest, "Invalid query parameters: #{e.message}"
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
class AcceptEntry
|
class AcceptEntry
|
||||||
|
@ -225,6 +231,10 @@ module Sinatra
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class BadRequest < TypeError #:nodoc:
|
||||||
|
def http_status; 400 end
|
||||||
|
end
|
||||||
|
|
||||||
class NotFound < NameError #:nodoc:
|
class NotFound < NameError #:nodoc:
|
||||||
def http_status; 404 end
|
def http_status; 404 end
|
||||||
end
|
end
|
||||||
|
@ -593,6 +603,11 @@ module Sinatra
|
||||||
status.between? 500, 599
|
status.between? 500, 599
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# whether or not the status is set to 400
|
||||||
|
def bad_request?
|
||||||
|
status == 400
|
||||||
|
end
|
||||||
|
|
||||||
# whether or not the status is set to 404
|
# whether or not the status is set to 404
|
||||||
def not_found?
|
def not_found?
|
||||||
status == 404
|
status == 404
|
||||||
|
@ -900,9 +915,7 @@ module Sinatra
|
||||||
@env = env
|
@env = env
|
||||||
@request = Request.new(env)
|
@request = Request.new(env)
|
||||||
@response = Response.new
|
@response = Response.new
|
||||||
@params = indifferent_params(@request.params)
|
|
||||||
template_cache.clear if settings.reload_templates
|
template_cache.clear if settings.reload_templates
|
||||||
force_encoding(@params)
|
|
||||||
|
|
||||||
@response['Content-Type'] = nil
|
@response['Content-Type'] = nil
|
||||||
invoke { dispatch! }
|
invoke { dispatch! }
|
||||||
|
@ -1089,6 +1102,9 @@ module Sinatra
|
||||||
|
|
||||||
# Dispatch a request with error handling.
|
# Dispatch a request with error handling.
|
||||||
def dispatch!
|
def dispatch!
|
||||||
|
@params = indifferent_params(@request.params)
|
||||||
|
force_encoding(@params)
|
||||||
|
|
||||||
invoke do
|
invoke do
|
||||||
static! if settings.static? && (request.get? || request.head?)
|
static! if settings.static? && (request.get? || request.head?)
|
||||||
filter! :before
|
filter! :before
|
||||||
|
@ -1124,11 +1140,12 @@ module Sinatra
|
||||||
if server_error?
|
if server_error?
|
||||||
dump_errors! boom if settings.dump_errors?
|
dump_errors! boom if settings.dump_errors?
|
||||||
raise boom if settings.show_exceptions? and settings.show_exceptions != :after_handler
|
raise boom if settings.show_exceptions? and settings.show_exceptions != :after_handler
|
||||||
end
|
elsif not_found?
|
||||||
|
|
||||||
if not_found?
|
|
||||||
headers['X-Cascade'] = 'pass' if settings.x_cascade?
|
headers['X-Cascade'] = 'pass' if settings.x_cascade?
|
||||||
body '<h1>Not Found</h1>'
|
body '<h1>Not Found</h1>'
|
||||||
|
elsif bad_request?
|
||||||
|
dump_errors! boom if settings.dump_errors?
|
||||||
|
halt status
|
||||||
end
|
end
|
||||||
|
|
||||||
res = error_block!(boom.class, boom) || error_block!(status, boom)
|
res = error_block!(boom.class, boom) || error_block!(status, boom)
|
||||||
|
|
|
@ -26,6 +26,23 @@ class HelpersTest < Minitest::Test
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'bad_request?' do
|
||||||
|
it 'is true for status == 400' do
|
||||||
|
status_app(400) { bad_request? }
|
||||||
|
assert_body 'true'
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'is false for status gt 400' do
|
||||||
|
status_app(401) { bad_request? }
|
||||||
|
assert_body 'false'
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'is false for status lt 400' do
|
||||||
|
status_app(399) { bad_request? }
|
||||||
|
assert_body 'false'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe 'not_found?' do
|
describe 'not_found?' do
|
||||||
it 'is true for status == 404' do
|
it 'is true for status == 404' do
|
||||||
status_app(404) { not_found? }
|
status_app(404) { not_found? }
|
||||||
|
|
|
@ -50,6 +50,11 @@ class RequestTest < Minitest::Test
|
||||||
assert_equal({ 'compress' => '0.25' }, request.preferred_type.params)
|
assert_equal({ 'compress' => '0.25' }, request.preferred_type.params)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "raises Sinatra::BadRequest when params contain conflicting types" do
|
||||||
|
request = Sinatra::Request.new 'QUERY_STRING' => 'foo=&foo[]='
|
||||||
|
assert_raises(Sinatra::BadRequest) { request.params }
|
||||||
|
end
|
||||||
|
|
||||||
it "makes accept types behave like strings" do
|
it "makes accept types behave like strings" do
|
||||||
request = Sinatra::Request.new('HTTP_ACCEPT' => 'image/jpeg; compress=0.25')
|
request = Sinatra::Request.new('HTTP_ACCEPT' => 'image/jpeg; compress=0.25')
|
||||||
assert request.accept?('image/jpeg')
|
assert request.accept?('image/jpeg')
|
||||||
|
|
|
@ -47,6 +47,16 @@ class RoutingTest < Minitest::Test
|
||||||
assert_equal '', response.body
|
assert_equal '', response.body
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "400s when request params contain conflicting types" do
|
||||||
|
mock_app {
|
||||||
|
get('/foo') { }
|
||||||
|
}
|
||||||
|
|
||||||
|
request = Rack::MockRequest.new(@app)
|
||||||
|
response = request.request('GET', '/foo?bar=&bar[]=', {})
|
||||||
|
assert response.bad_request?
|
||||||
|
end
|
||||||
|
|
||||||
it "404s when no route satisfies the request" do
|
it "404s when no route satisfies the request" do
|
||||||
mock_app {
|
mock_app {
|
||||||
get('/foo') { }
|
get('/foo') { }
|
||||||
|
|
Loading…
Reference in New Issue