diff --git a/lib/puma/request.rb b/lib/puma/request.rb index c9f4c5474c..deea7e4bcb 100644 --- a/lib/puma/request.rb +++ b/lib/puma/request.rb @@ -46,11 +46,7 @@ def handle_request(client, lines, requests) env[HIJACK_P] = true env[HIJACK] = client - body = client.body - - head = env[REQUEST_METHOD] == HEAD - - env[RACK_INPUT] = body + env[RACK_INPUT] = client.body env[RACK_URL_SCHEME] ||= default_server_port(env) == PORT_443 ? HTTPS : HTTP if @early_hints @@ -69,118 +65,132 @@ def handle_request(client, lines, requests) # A rack extension. If the app writes #call'ables to this # array, we will invoke them when the request is done. # - after_reply = env[RACK_AFTER_REPLY] = [] + env[RACK_AFTER_REPLY] = [] begin - begin - status, headers, res_body = @thread_pool.with_force_shutdown do - @app.call(env) - end - - return :async if client.hijacked + status, headers, res_body = @thread_pool.with_force_shutdown do + @app.call(env) + end - status = status.to_i + return :async if client.hijacked - if status == -1 - unless headers.empty? and res_body == [] - raise "async response must have empty headers and body" - end + status = status.to_i - return :async + if status == -1 + unless headers.empty? and res_body == [] + raise "async response must have empty headers and body" end - rescue ThreadPool::ForceShutdown => e - @events.unknown_error e, client, "Rack app" - @events.log "Detected force shutdown of a thread" - status, headers, res_body = lowlevel_error(e, env, 503) - rescue Exception => e - @events.unknown_error e, client, "Rack app" - - status, headers, res_body = lowlevel_error(e, env, 500) + return :async end + rescue ThreadPool::ForceShutdown => e + @events.unknown_error e, client, "Rack app" + @events.log "Detected force shutdown of a thread" - res_info = {} - res_info[:content_length] = nil - res_info[:no_body] = head + status, headers, res_body = lowlevel_error(e, env, 503) + rescue Exception => e + @events.unknown_error e, client, "Rack app" - res_info[:content_length] = if res_body.kind_of? Array and res_body.size == 1 - res_body[0].bytesize - else - nil - end + status, headers, res_body = lowlevel_error(e, env, 500) + end - cork_socket io + write_response(status, headers, res_body, lines, requests, client) + end - str_headers(env, status, headers, res_info, lines, requests, client) + # Does the actual response writing for Request#handle_request and Server#client_error + # + # @param status [Integer] the status returned by the Rack application + # @param headers [Hash] the headers returned by the Rack application + # @param res_body [Array] the body returned by the Rack application + # @param lines [Puma::IOBuffer] modified in place + # @param requests [Integer] number of inline requests handled + # @param client [Puma::Client] + # @return [Boolean,:async] + def write_response(status, headers, res_body, lines, requests, client) + env = client.env + io = client.io - line_ending = LINE_END + head = env[REQUEST_METHOD] == HEAD + after_reply = env[RACK_AFTER_REPLY] ||= [] - content_length = res_info[:content_length] - if res_body && !res_body.respond_to?(:each) - response_hijack = res_body - else - response_hijack = res_info[:response_hijack] - end + res_info = {} + res_info[:no_body] = head - if res_info[:no_body] - if content_length and status != 204 - lines.append CONTENT_LENGTH_S, content_length.to_s, line_ending - end + res_info[:content_length] = res_body.kind_of?(Array) && res_body.size == 1 ? + res_body[0].bytesize : nil - lines << LINE_END - fast_write io, lines.to_s - return res_info[:keep_alive] - end + cork_socket io - if content_length + str_headers(env, status, headers, res_info, lines, requests, client) + + line_ending = LINE_END + + content_length = res_info[:content_length] + + if res_body && !res_body.respond_to?(:each) + response_hijack = res_body + else + response_hijack = res_info[:response_hijack] + end + + if res_info[:no_body] + if content_length and status != 204 lines.append CONTENT_LENGTH_S, content_length.to_s, line_ending - chunked = false - elsif !response_hijack and res_info[:allow_chunked] - lines << TRANSFER_ENCODING_CHUNKED - chunked = true end - lines << line_ending - + lines << LINE_END fast_write io, lines.to_s + return res_info[:keep_alive] + end - if response_hijack - response_hijack.call io - return :async - end + if content_length + lines.append CONTENT_LENGTH_S, content_length.to_s, line_ending + chunked = false + elsif !response_hijack and res_info[:allow_chunked] + lines << TRANSFER_ENCODING_CHUNKED + chunked = true + end - begin - res_body.each do |part| - next if part.bytesize.zero? - if chunked - fast_write io, (part.bytesize.to_s(16) << line_ending) - fast_write io, part # part may have different encoding - fast_write io, line_ending - else - fast_write io, part - end - io.flush - end + lines << line_ending + + fast_write io, lines.to_s + + if response_hijack + response_hijack.call io + return :async + end + begin + res_body.each do |part| + next if part.bytesize.zero? if chunked - fast_write io, CLOSE_CHUNKED - io.flush + fast_write io, (part.bytesize.to_s(16) << line_ending) + fast_write io, part # part may have different encoding + fast_write io, line_ending + else + fast_write io, part end - rescue SystemCallError, IOError - raise ConnectionError, "Connection error detected during write" + io.flush end - ensure - uncork_socket io - - body.close - client.tempfile.unlink if client.tempfile - res_body.close if res_body.respond_to? :close - - after_reply.each { |o| o.call } + if chunked + fast_write io, CLOSE_CHUNKED + io.flush + end + rescue SystemCallError, IOError + raise ConnectionError, "Connection error detected during write" end - res_info[:keep_alive] + + ensure + uncork_socket io + # lines must be reset if an error is raised + lines.reset + client.body.close if client.body + client.tempfile.unlink if client.tempfile + res_body.close if res_body.respond_to? :close + + after_reply.each { |o| o.call } end # @param env [Hash] see Puma::Client#env, from request diff --git a/lib/puma/server.rb b/lib/puma/server.rb index f4aed5dfc5..59ef16fec2 100644 --- a/lib/puma/server.rb +++ b/lib/puma/server.rb @@ -476,7 +476,7 @@ def process_client(client, buffer) end true rescue StandardError => e - client_error(e, client) + client_error(e, client, buffer, requests) # The ensure tries to close +client+ down requests > 0 ensure @@ -504,37 +504,37 @@ 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, buffer = ::Puma::IOBuffer.new, requests = 1) # Swallow, do not log return if [ConnectionError, EOFError].include?(e.class) - lowlevel_error(e, client.env) case e when MiniSSL::SSLError @events.ssl_error e, client.io when HttpParserError - client.write_error(400) + status, headers, res_body = lowlevel_error(e, client.env, 400) + write_response(status, headers, res_body, buffer, requests, client) @events.parse_error e, client else - client.write_error(500) + status, headers, res_body = lowlevel_error(e, client.env) + write_response(status, headers, res_body, buffer, requests, client) @events.unknown_error e, nil, "Read" end end # A fallback rack response if +@app+ raises as exception. # - def lowlevel_error(e, env, status=500) + def lowlevel_error(e, env, status = 500) if handler = @options[:lowlevel_error_handler] if handler.arity == 1 - return handler.call(e) + handler_status, headers, res_body = handler.call(e) elsif handler.arity == 2 - return handler.call(e, env) + handler_status, headers, res_body = handler.call(e, env) else - return handler.call(e, env, status) + handler_status, headers, res_body = handler.call(e, env, status) end - end - - if @leak_stack_on_error + [handler_status || status, headers || {}, res_body || []] + elsif @leak_stack_on_error backtrace = e.backtrace.nil? ? '' : e.backtrace.join("\n") [status, {}, ["Puma caught this error: #{e.message} (#{e.class})\n#{backtrace}"]] else diff --git a/test/test_puma_server.rb b/test/test_puma_server.rb index 4909f2681f..5575e0e6b0 100644 --- a/test/test_puma_server.rb +++ b/test/test_puma_server.rb @@ -383,6 +383,17 @@ def test_lowlevel_error_message assert (data.size > 0), "Expected response message to be not empty" end + def test_lowlevel_error_handler_custom_response + options = { lowlevel_error_handler: ->(_err) { [500, {}, ["error page"]] } } + # setting the headers argument to nil will trigger exception inside Puma + broken_app = ->(_env) { [200, nil, []] } + server_run(**options, &broken_app) + + data = send_http_and_read "GET / HTTP/1.0\r\n\r\n" + + assert_match %r{\AHTTP/1.0 500 Internal Server Error\r\nContent-Length: 10\r\n\r\nerror page}, data + end + def test_force_shutdown_error_default server_run(force_shutdown_after: 2) do @server.stop diff --git a/test/test_response_header.rb b/test/test_response_header.rb index 508bbcd78c..03fb43ca54 100644 --- a/test/test_response_header.rb +++ b/test/test_response_header.rb @@ -45,7 +45,7 @@ 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(/HTTP\/1.0 500 Internal Server Error/, data) end # The header must respond to each @@ -53,7 +53,7 @@ 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(/HTTP\/1.0 500 Internal Server Error/, data) end # The values of the header must be Strings