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

Issue 56 - Add limit to size of headers #232

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kolbma
Copy link

@kolbma kolbma commented Nov 7, 2022

Hello,
I think it is required to not handle http headers and header fields of any size the clients might provide. (DOS, OOM)
This PR limits the header body to 8192 bytes and the header lines to 2048 bytes. Popular servers (apache, nginx) have similar restrictions.

When a client is going over the limits the HTTP status 431 Request Header Fields Too Large is returned.

I've seen that it has been already the intention in #56 to implement this.
I hope this PR is welcome.

Tests HTTP headers for byte limit.
Complete field may be 2048 Bytes and the complete header
may be 8192 Bytes big.
These are limitations similar used by popular HTTP servers.
Each header field is limited to max. 2048 Bytes.
The complete header is limited to max. 8192 Bytes.

Returns HTTP Status 431 Request Header Fields Too Large when the
header is over size.

Closes tiny-http#56

The read and parse of `ClientConnection` uses stringent `AsciiStr`.
No need for `String` or `&str` during reading.
Return HTTP status "414 URI Too Long" if request line is over
2048 bytes.
@Kijewski
Copy link

IMHO 8192 bytes in total is much too small. If the browser is configured to send the full URL in the Referer header, then you could not send a single cookie if the base URL is quite long.

Apaches defaults are:

  • LimitRequestFields = the number of HTTP request header fields that will be accepted from the client = 100
  • LimitRequestFieldSize = the size of the HTTP request header allowed from the client = 8190
  • LimitRequestLine = the size of the HTTP request line that will be accepted from the client = 8190
  • Total size header of headers = unlimited (or implicitly: LimitRequestFields * LimitRequestFieldSize)

@bradfier
Copy link
Member

I have to agree with @Kijewski here, I've seen no end of questionably designed applications that include kilobytes of payload in either query parameters or really long path-style URLs (cough WebSphere).

Using Apache's default values here works for me, but we do need to consider this a breaking change and gate it behind a .with_default_header_limits() on ServerConfig. I appreciate this makes the change much bigger.

@kolbma
Copy link
Author

kolbma commented Feb 2, 2023

@Kijewski @bradfier Don't you develop the "questionable" application with tiny-http itself and therefor why you want to develop a bad kind of WebSphere JEE app with tiny-http where you possible have base urls with thousands of characters and same for cookies?
Not sure why you think you have to handle unknown/all applications in tiny-http and don't handle the currently for every app existing security problem in soon 3 months.

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

Successfully merging this pull request may close these issues.

None yet

3 participants