1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
|
https://github.com/sinatra/sinatra/pull/1519
From 6d34a2a1bee48961c25e1b53edac874a31c42060 Mon Sep 17 00:00:00 2001
From: Jordan Owens <jkowens@gmail.com>
Date: Thu, 31 Jan 2019 22:32:45 -0500
Subject: [PATCH] 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.
---
lib/sinatra/base.rb | 22 ++++++++++++++--------
test/mapped_error_test.rb | 6 +++---
test/result_test.rb | 15 +++++++++++++++
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb
index 6dbb3ae57..4dfc9a8ae 100644
--- a/lib/sinatra/base.rb
+++ b/lib/sinatra/base.rb
@@ -255,11 +255,14 @@ def call(env)
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
@@ -1149,14 +1152,17 @@ def handle_exception!(boom)
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
diff --git a/test/mapped_error_test.rb b/test/mapped_error_test.rb
index cb158a268..562e509dc 100644
--- a/test/mapped_error_test.rb
+++ b/test/mapped_error_test.rb
@@ -6,15 +6,15 @@ class FooError < RuntimeError
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
diff --git a/test/result_test.rb b/test/result_test.rb
index cbb781319..67d163fc4 100644
--- a/test/result_test.rb
+++ b/test/result_test.rb
@@ -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 @@ def res.each ; yield call ; end
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
|
