Skip to content

Commit

Permalink
Closes #2341 - Make sure fallback Rack response are sent to the client (
Browse files Browse the repository at this point in the history
#3094)

Currently, when Client I/O operations raise any exceptions, we build a fallback rack response (either by lowlevel_error_handler or by default puma's hard-coded response), but doesn't do anything with it.
This commit ensures that this response is sent back to client.
  • Loading branch information
Vuta committed Jul 23, 2023
1 parent db06025 commit 99ae85b
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 9 deletions.
21 changes: 14 additions & 7 deletions lib/puma/server.rb
Expand Up @@ -476,7 +476,7 @@ def process_client(client)
end
true
rescue StandardError => e
client_error(e, client)
client_error(e, client, requests)
# The ensure tries to close +client+ down
requests > 0
ensure
Expand Down Expand Up @@ -504,22 +504,22 @@ def with_force_shutdown(client, &block)
# :nocov:

# Handle various error types thrown by Client I/O operations.
def client_error(e, client)
def client_error(e, client, requests = 1)
# Swallow, do not log
return if [ConnectionError, EOFError].include?(e.class)

lowlevel_error(e, client.env)
case e
when MiniSSL::SSLError
lowlevel_error(e, client.env)
@log_writer.ssl_error e, client.io
when HttpParserError
client.write_error(400)
response_to_error(client, requests, e, 400)
@log_writer.parse_error e, client
when HttpParserError501
client.write_error(501)
response_to_error(client, requests, e, 501)
@log_writer.parse_error e, client
else
client.write_error(500)
response_to_error(client, requests, e, 500)
@log_writer.unknown_error e, nil, "Read"
end
end
Expand All @@ -541,10 +541,17 @@ def lowlevel_error(e, env, status=500)
backtrace = e.backtrace.nil? ? '<no backtrace available>' : e.backtrace.join("\n")
[status, {}, ["Puma caught this error: #{e.message} (#{e.class})\n#{backtrace}"]]
else
[status, {}, ["An unhandled lowlevel error occurred. The application logs may have details.\n"]]
[status, {}, [""]]
end
end

def response_to_error(client, requests, err, status_code)
status, headers, res_body = lowlevel_error(err, client.env, status_code)
prepare_response(status, headers, res_body, requests, client)
client.write_error(status_code)
end
private :response_to_error

# Wait for all outstanding requests to finish.
#
def graceful_shutdown
Expand Down
15 changes: 15 additions & 0 deletions test/test_puma_server.rb
Expand Up @@ -1695,4 +1695,19 @@ def spawn_cmd(env = {}, cmd)
[out_w, err_w].each(&:close)
[out_r, err_r, pid]
end

def test_lowlevel_error_handler_response
options = {
lowlevel_error_handler: ->(_error) do
[500, {}, ["something wrong happened"]]
end
}
broken_app = ->(_env) { [200, nil, []] }

server_run(**options, &broken_app)

data = send_http_and_read "GET / HTTP/1.1\r\n\r\n"

assert_match(/something wrong happened/, data)
end
end
4 changes: 2 additions & 2 deletions test/test_response_header.rb
Expand Up @@ -49,15 +49,15 @@ def test_integer_key
server_run app: ->(env) { [200, { 1 => 'Boo'}, []] }
data = send_http_and_read "GET / HTTP/1.0\r\n\r\n"

assert_match(/HTTP\/1.1 500 Internal Server Error/, data)
assert_match(/Puma caught this error/, data)
end

# The header must respond to each
def test_nil_header
server_run app: ->(env) { [200, nil, []] }
data = send_http_and_read "GET / HTTP/1.0\r\n\r\n"

assert_match(/HTTP\/1.1 500 Internal Server Error/, data)
assert_match(/Puma caught this error/, data)
end

# The values of the header must be Strings
Expand Down

0 comments on commit 99ae85b

Please sign in to comment.