NotificationsYou must be signed in to change notification settings
Fork137
Star1.1k

Issue 56 - Add limit to size of headers#232

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

kolbma wants to merge4 commits intotiny-http:master

base:master

Choose a base branch

fromkolbma:issue-56

Open

Issue 56 - Add limit to size of headers#232

kolbma wants to merge4 commits intotiny-http:masterfromkolbma:issue-56

Conversation

Copy link

kolbma commentedNov 7, 2022

Hello,
I think it is required tonot 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 status431 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.

kolbma added4 commits

November 7, 2022 20:10

test: too_long_header_field, too_long_header

a5f7aaf

Tests HTTP headers for byte limit.Complete field may be 2048 Bytes and the complete headermay be 8192 Bytes big.These are limitations similar used by popular HTTP servers.

chore: rustfmt tests/input-tests.rs

e4c50b5

feat: HTTP header size limitation

7c76b05

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 theheader is over size.Closestiny-http#56The read and parse of `ClientConnection` uses stringent `AsciiStr`.No need for `String` or `&str` during reading.

feat: use 414 URI Too Long for request line

61bd5b9

Return HTTP status "414 URI Too Long" if request line is over2048 bytes.

Copy link

Kijewski commentedDec 27, 2022

IMHO 8192 bytes in total is much too small. If the browser is configured to send the full URL in theReferer 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)

Copy link

Member

bradfier commentedDec 28, 2022

I have to agree with@Kijewski here, I've seen no end of questionably designed applications that includekilobytes 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() onServerConfig. I appreciate this makes the change much bigger.

Copy link

Author

kolbma commentedFeb 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 withthousands 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.