Skip to content

Commit

Permalink
Introduce the ability to return 413: payload too large for requests (#…
Browse files Browse the repository at this point in the history
…3040)

* Introduce the ability to return 413: payload too large for requests

When recieving large payload objects, the server can often slowdown
or get fully exhausted if bunch of requests with large payload body size
come in. When request with large payload come, lot of the time is spent
reading it into then, writing it to the IO for rack, before the request
is passed to the rails app for further processing.

While there are some workarounds around limiting large request sizes, like at nginx layer
by setting `client_max_body_size`, which would return a `413` back to the client,
today that is not possible with puma.

This would be a very nice feature to have, especially when there is no reverse
proxy in between client and server.

This approach - allows a user to set `http_content_length_limit_exceeded`
via  a config variable (defaults to `nil`). This value is then compared
against `Content-Length` http header before reading the body into buffer.
If the user value is higher than the header value, the request body is
not loaded and an immediate `413` (`Payload too large`) http response is returned,
from `Puma::Request.handle_request`.

Without having to buffer in the huge request and return the `413`
immediately to the clients that send a `Content-Length` - is a nice feature
and helpful protection to have.

* Compare and limit against body bytesize when no content-length
http header is present.

* Update lib/puma/dsl.rb

Co-authored-by: Nate Berkopec <nate.berkopec@gmail.com>

* Update test

* Reset http_content_length_limit_exceeded

* Add some more specs

* Removed unsued var

* Minor logic DRY up

Co-authored-by: Nate Berkopec <nate.berkopec@gmail.com>
  • Loading branch information
shayonj and nateberkopec committed Jan 2, 2023
1 parent 8831577 commit 1c7804c
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 2 deletions.
27 changes: 25 additions & 2 deletions lib/puma/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ def initialize(io, env=nil)
@requests_served = 0
@hijacked = false

@http_content_length_limit = nil
@http_content_length_limit_exceeded = false

@peerip = nil
@peer_family = nil
@listener = nil
Expand All @@ -98,9 +101,9 @@ def initialize(io, env=nil)
end

attr_reader :env, :to_io, :body, :io, :timeout_at, :ready, :hijacked,
:tempfile, :io_buffer
:tempfile, :io_buffer, :http_content_length_limit_exceeded

attr_writer :peerip
attr_writer :peerip, :http_content_length_limit

attr_accessor :remote_addr_header, :listener

Expand Down Expand Up @@ -151,6 +154,7 @@ def reset(fast_check=true)
@body_remain = 0
@peerip = nil if @remote_addr_header
@in_last_chunk = false
@http_content_length_limit_exceeded = false

if @buffer
return false unless try_to_parse_proxy_protocol
Expand Down Expand Up @@ -210,6 +214,17 @@ def try_to_parse_proxy_protocol
end

def try_to_finish
if env[CONTENT_LENGTH] && above_http_content_limit(env[CONTENT_LENGTH].to_i)
@http_content_length_limit_exceeded = true
end

if @http_content_length_limit_exceeded
@buffer = nil
@body = EmptyBody
set_ready
return true
end

return read_body if in_data_phase

begin
Expand Down Expand Up @@ -239,6 +254,10 @@ def try_to_finish

@parsed_bytes = @parser.execute(@env, @buffer, @parsed_bytes)

if @parser.finished? && above_http_content_limit(@parser.body.bytesize)
@http_content_length_limit_exceeded = true
end

if @parser.finished?
return setup_body
elsif @parsed_bytes >= MAX_HEADER
Expand Down Expand Up @@ -594,5 +613,9 @@ def set_ready
@requests_served += 1
@ready = true
end

def above_http_content_limit(value)
@http_content_length_limit&.< value
end
end
end
1 change: 1 addition & 0 deletions lib/puma/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ class Configuration
worker_shutdown_timeout: 30,
worker_timeout: 60,
workers: 0,
http_content_length_limit: nil
}

def initialize(user_options={}, default_options = {}, &block)
Expand Down
13 changes: 13 additions & 0 deletions lib/puma/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,19 @@ def mutate_stdout_and_stderr_to_sync_on_write(enabled=true)
@options[:mutate_stdout_and_stderr_to_sync_on_write] = enabled
end

# Specify how big the request payload should be, in bytes.
# This limit is compared against Content-Length HTTP header.
# If the payload size (CONTENT_LENGTH) is larger than http_content_length_limit,
# HTTP 413 status code is returned.
#
# When no Content-Length http header is present, it is compared against the
# size of the body of the request.
#
# The default value for http_content_length_limit is nil.
def http_content_length_limit(limit)
@options[:http_content_length_limit] = limit
end

private

# To avoid adding cert_pem and key_pem as URI params, we store them on the
Expand Down
5 changes: 5 additions & 0 deletions lib/puma/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,13 @@ def handle_request(client, requests)
socket = client.io # io may be a MiniSSL::Socket
app_body = nil


return false if closed_socket?(socket)

if client.http_content_length_limit_exceeded
return prepare_response(413, {}, ["Payload Too Large"], requests, client)
end

normalize_env env, client

env[PUMA_SOCKET] = socket
Expand Down
2 changes: 2 additions & 0 deletions lib/puma/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def initialize(app, events = nil, options = {})
@queue_requests = @options[:queue_requests]
@max_fast_inline = @options[:max_fast_inline]
@io_selector_backend = @options[:io_selector_backend]
@http_content_length_limit = @options[:http_content_length_limit]

temp = !!(@options[:environment] =~ /\A(development|test)\z/)
@leak_stack_on_error = @options[:environment] ? temp : true
Expand Down Expand Up @@ -334,6 +335,7 @@ def handle_servers
drain += 1 if shutting_down?
pool << Client.new(io, @binder.env(sock)).tap { |c|
c.listener = sock
c.http_content_length_limit = @http_content_length_limit
c.send(addr_send_name, addr_value) if addr_value
}
end
Expand Down
8 changes: 8 additions & 0 deletions test/test_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,14 @@ def test_silence_single_worker_warning_overwrite
assert_equal true, conf.options[:silence_single_worker_warning]
end

def test_http_content_length_limit
assert_nil Puma::Configuration.new.options.default_options[:http_content_length_limit]

conf = Puma::Configuration.new({ http_content_length_limit: 10000})

assert_equal 10000, conf.final_options[:http_content_length_limit]
end

private

def assert_run_hooks(hook_name, options = {})
Expand Down
22 changes: 22 additions & 0 deletions test/test_puma_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,28 @@ def test_early_hints_is_off_by_default
assert_equal expected_data, data
end

def test_request_payload_too_large
server_run(http_content_length_limit: 10)

sock = send_http "POST / HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\nContent-Length: 19\r\n\r\n"
sock << "hello world foo bar"

data = sock.gets

assert_equal "HTTP/1.1 413 Payload Too Large\r\n", data
end

def test_http_11_keep_alive_with_large_payload
server_run(http_content_length_limit: 10) { [204, {}, []] }

sock = send_http "GET / HTTP/1.1\r\nConnection: Keep-Alive\r\nContent-Length: 17\r\n\r\n"
sock << "hello world foo bar"
h = header sock

assert_equal ["HTTP/1.1 413 Payload Too Large", "Content-Length: 17"], h

end

def test_GET_with_no_body_has_sane_chunking
server_run { [200, {}, []] }

Expand Down

0 comments on commit 1c7804c

Please sign in to comment.