Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Jan 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very important path so we'll need to be careful with anything we add here.

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
shayonj marked this conversation as resolved.
Show resolved Hide resolved
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