Skip to content

Customize maximum line length constraint #41

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

Closed
fdisperati-invoca opened this issue Jan 16, 2025 · 5 comments
Closed

Customize maximum line length constraint #41

fdisperati-invoca opened this issue Jan 16, 2025 · 5 comments

Comments

@fdisperati-invoca
Copy link

fdisperati-invoca commented Jan 16, 2025

Hi there!

We have a server running on Falcon that uses a GET endpoint to handle large JSON payloads (due to some old integrations, switching to a POST endpoint isn't an option for now).

After updating to protocol-http1 v0.28.0 (via async-http v0.82.0), we started noticing that some requests were being dropped with the following error:

 4.34s     warn: Async::Task: Reading HTTP/1.1 requests for Async::HTTP::Protocol::HTTP1::Server. [oid=0x2580] [ec=0x2594] [pid=88108] [2025-01-15 22:35:13 -0800]
               | Task may have ended with unhandled exception.
               |   Protocol::HTTP1::LineLengthError: Line too long! (Protocol::HTTP1::LineLengthError)

We understand the security considerations behind this constraint, but we need to accept requests longer than 8192 bytes (at least up to 16kb).

For now, we’ve reverted to protocol-http1 v0.27.0 to remove the request size constraint, but it would be great if this constraint was customizable.

One possible solution could be introducing an environment variable, like PROTOCOL_HTTP1_MAXIMUM_LINE_LENGTH, to override the default:

DEFAULT_MAXIMUM_LINE_LENGTH = ENV.fetch('PROTOCOL_HTTP1_MAXIMUM_LINE_LENGTH', 8192)

I'd be happy to put together a PR if this sounds like a reasonable approach.

@ioquatix
Copy link
Member

ioquatix commented Jan 16, 2025

That would be an acceptable PR. Another option might be increase the default to 16KiB? Maybe 8KiB is too conservative?

You may also be able to write this:

require "protocol/http1/connection"
Protocol::HTTP1::DEFAULT_MAXIMUM_LINE_LENGTH = 16384

@fdisperati-invoca
Copy link
Author

fdisperati-invoca commented Jan 16, 2025

Another option might be increase the default to 16KiB? Maybe 8KiB is too conservative?

16KiB would definitely work for us, more than that our load balancer just rejects the request with a "414 Request-URI Too Large". But I understand that our use case is very peculiar, I'm not sure if it's worth changing the default for everyone.

Also, re-assigning Protocol::HTTP1::DEFAULT_MAXIMUM_LINE_LENGTH in our falcon.rb works great. And it's not any worse than using an env variable: if the implementation of Protocol::HTTP1 changes both solutions will silently stop working. Not a big deal though. We have a regression test for the request length.

@fdisperati-invoca
Copy link
Author

@ioquatix thanks for the help! Feel free to close the issue.

@ioquatix ioquatix moved this to In Progress in Open Source Mar 7, 2025
@ioquatix
Copy link
Member

ioquatix commented Mar 13, 2025

Okay, there is now an official way to do this, using the latest protocol-http1 and async-http:

# falcon.rb

service "hello.localhost" do
  include Falcon::Environment::SelfSignedTLS
  include Falcon::Environment::Rack
	
  protocol {Async::HTTP::Protocol::HTTP1.new(
    maximum_line_length: 1024*16
  )}

  # ... rest of configuration

Similar example code is included here: socketry/falcon@55b8bab

See socketry/async-http#198 for the pull request that addressed this issue.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Open Source Mar 13, 2025
@fdisperati-invoca
Copy link
Author

@ioquatix excellent work, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants