Improve BadRequest logic. Partially reverts 8f8df53, fixes #1211

This commit is contained in:
Mike Pastore 2016-12-01 11:24:39 -06:00
parent 63e81bc539
commit b4fdb7221f
5 changed files with 67 additions and 68 deletions

View File

@ -603,11 +603,6 @@ module Sinatra
status.between? 500, 599
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
def not_found?
status == 404
@ -1143,9 +1138,6 @@ module Sinatra
elsif not_found?
headers['X-Cascade'] = 'pass' if settings.x_cascade?
body '<h1>Not Found</h1>'
elsif bad_request?
dump_errors! boom if settings.dump_errors?
halt status
end
res = error_block!(boom.class, boom) || error_block!(status, boom)

View File

@ -49,6 +49,10 @@ module Sinatra
private
def bad_request?(e)
Sinatra::BadRequest === e
end
def prefers_plain_text?(env)
!(Request.new(env).preferred_type("text/plain","text/html") == "text/html") &&
[/curl/].index { |item| item =~ env["HTTP_USER_AGENT"] }
@ -210,8 +214,10 @@ TEMPLATE = ERB.new <<-HTML # :nodoc:
<p><a href="#" id="expando"
onclick="toggleBacktrace(); return false">(expand)</a></p>
<p id="nav"><strong>JUMP TO:</strong>
<a href="#get-info">GET</a>
<a href="#post-info">POST</a>
<% unless bad_request?(exception) %>
<a href="#get-info">GET</a>
<a href="#post-info">POST</a>
<% end %>
<a href="#cookie-info">COOKIES</a>
<a href="#env-info">ENV</a>
</p>
@ -264,47 +270,49 @@ TEMPLATE = ERB.new <<-HTML # :nodoc:
</ul>
</div> <!-- /BACKTRACE -->
<div id="get">
<h3 id="get-info">GET</h3>
<% if req.GET and not req.GET.empty? %>
<table class="req">
<tr>
<th>Variable</th>
<th>Value</th>
</tr>
<% req.GET.sort_by { |k, v| k.to_s }.each { |key, val| %>
<tr>
<td><%=h key %></td>
<td class="code"><div><%=h val.inspect %></div></td>
</tr>
<% } %>
</table>
<% else %>
<p class="no-data">No GET data.</p>
<% end %>
<div class="clear"></div>
</div> <!-- /GET -->
<% unless bad_request?(exception) %>
<div id="get">
<h3 id="get-info">GET</h3>
<% if req.GET and not req.GET.empty? %>
<table class="req">
<tr>
<th>Variable</th>
<th>Value</th>
</tr>
<% req.GET.sort_by { |k, v| k.to_s }.each { |key, val| %>
<tr>
<td><%=h key %></td>
<td class="code"><div><%=h val.inspect %></div></td>
</tr>
<% } %>
</table>
<% else %>
<p class="no-data">No GET data.</p>
<% end %>
<div class="clear"></div>
</div> <!-- /GET -->
<div id="post">
<h3 id="post-info">POST</h3>
<% if req.POST and not req.POST.empty? %>
<table class="req">
<tr>
<th>Variable</th>
<th>Value</th>
</tr>
<% req.POST.sort_by { |k, v| k.to_s }.each { |key, val| %>
<tr>
<td><%=h key %></td>
<td class="code"><div><%=h val.inspect %></div></td>
</tr>
<% } %>
</table>
<% else %>
<p class="no-data">No POST data.</p>
<% end %>
<div class="clear"></div>
</div> <!-- /POST -->
<div id="post">
<h3 id="post-info">POST</h3>
<% if req.POST and not req.POST.empty? %>
<table class="req">
<tr>
<th>Variable</th>
<th>Value</th>
</tr>
<% req.POST.sort_by { |k, v| k.to_s }.each { |key, val| %>
<tr>
<td><%=h key %></td>
<td class="code"><div><%=h val.inspect %></div></td>
</tr>
<% } %>
</table>
<% else %>
<p class="no-data">No POST data.</p>
<% end %>
<div class="clear"></div>
</div> <!-- /POST -->
<% end %>
<div id="cookies">
<h3 id="cookie-info">COOKIES</h3>

View File

@ -26,23 +26,6 @@ class HelpersTest < Minitest::Test
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
it 'is true for status == 404' do
status_app(404) { not_found? }

View File

@ -54,7 +54,7 @@ class RoutingTest < Minitest::Test
request = Rack::MockRequest.new(@app)
response = request.request('GET', '/foo?bar=&bar[]=', {})
assert response.bad_request?
assert_equal 400, response.status
end
it "404s when no route satisfies the request" do

View File

@ -247,6 +247,22 @@ class SettingsTest < Minitest::Test
assert body.include?("<code>show_exceptions</code> setting")
end
it 'does not attempt to show unparseable query parameters' do
klass = Sinatra.new(Sinatra::Application)
mock_app(klass) {
enable :show_exceptions
get '/' do
raise Sinatra::BadRequest
end
}
get '/'
assert_equal 400, status
refute body.include?('<div id="get">')
refute body.include?('<div id="post">')
end
it 'does not override app-specified error handling when set to :after_handler' do
ran = false
mock_app do