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

Adding a MaximumRequestSizeHandler #1916

Merged
merged 6 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions spec/lucky/maximum_request_size_handler_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require "../spec_helper"
require "http/server"

include ContextHelper

describe Lucky::MaximumRequestSizeHandler do
context "when the handler is disabled" do
it "simply serves the request" do
context = build_small_request_context("/path")
Lucky::MaximumRequestSizeHandler.temp_config(enabled: false) do
run_request_size_handler(context)
end
context.response.status.should eq(HTTP::Status::OK)
end
end

context "when the handler is enabled" do
it "with a small request, serve the request" do
context = build_small_request_context("/path")
Lucky::MaximumRequestSizeHandler.temp_config(enabled: true) do
run_request_size_handler(context)
end
context.response.status.should eq(HTTP::Status::OK)
end

it "with a large request, deny the request" do
context = build_large_request_context("/path")
Lucky::MaximumRequestSizeHandler.temp_config(enabled: true) do
run_request_size_handler(context)
end
context.response.status.should eq(HTTP::Status::PAYLOAD_TOO_LARGE)

Check failure on line 31 in spec/lucky/maximum_request_size_handler_spec.cr

View workflow job for this annotation

GitHub Actions / specs (shard.yml, 1.10.0, false)

got: HTTP::Status::OK

Check failure on line 31 in spec/lucky/maximum_request_size_handler_spec.cr

View workflow job for this annotation

GitHub Actions / specs (shard.yml, latest, false)

got: HTTP::Status::OK

Check failure on line 31 in spec/lucky/maximum_request_size_handler_spec.cr

View workflow job for this annotation

GitHub Actions / specs (shard.edge.yml, latest, true)

got: HTTP::Status::OK

Check failure on line 31 in spec/lucky/maximum_request_size_handler_spec.cr

View workflow job for this annotation

GitHub Actions / specs (shard.override.yml, nightly, true)

got: HTTP::Status::OK
Copy link
Member

Choose a reason for hiding this comment

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

seems this failed?

end
end
end

private def run_request_size_handler(context)
handler = Lucky::MaximumRequestSizeHandler.new
handler.next = ->(_ctx : HTTP::Server::Context) {}
handler.call(context)
end

private def build_small_request_context(path : String) : HTTP::Server::Context
build_context(path: path)
end

private def build_large_request_context(path : String) : HTTP::Server::Context
build_context(path: path).tap do |context|
context.request.headers["Content-Length"] = "1000000"
context.request.body = "a" * 1000000
end
end
4 changes: 4 additions & 0 deletions spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,8 @@ Lucky::ForceSSLHandler.configure do |settings|
settings.enabled = true
end

Lucky::MaximumRequestSizeHandler.configure do |settings|
settings.enabled = false
end
jwoertink marked this conversation as resolved.
Show resolved Hide resolved

Habitat.raise_if_missing_settings!
40 changes: 40 additions & 0 deletions src/lucky/maximum_request_size_handler.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
class Lucky::MaximumRequestSizeHandler
include HTTP::Handler

Habitat.create do
setting enabled : Bool = false
setting max_size : Int32 = 1_048_576 # 1MB
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

actually... if you wanted this to be say 10GB (video uploads), would this need to be Int64?

end

def call(context)
return call_next(context) unless settings.enabled

body_size = 0
body = IO::Memory.new

begin
buffer = Bytes.new(8192) # 8KB buffer
while (read_bytes = context.request.body.try(&.read(buffer)))
body_size += read_bytes
body.write(buffer[0, read_bytes])

if body_size > settings.max_size
context.response.status = HTTP::Status::PAYLOAD_TOO_LARGE
context.response.print("Request entity too large")
return context
end

break if read_bytes < buffer.size # End of body
end
rescue IO::Error
context.response.status = HTTP::Status::BAD_REQUEST
context.response.print("Error reading request body")
return context
end

# Reset the request body for downstream handlers
context.request.body = IO::Memory.new(body.to_s)

call_next(context)
end
end
Loading