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

configure header size limit for warp #5441

Closed

Conversation

tirumaraiselvan
Copy link
Contributor

@tirumaraiselvan tirumaraiselvan commented Jul 22, 2020

Description

Warp's default total header limit size for HTTP 1.x is 50KB which turns out to be little low (reported in Intercom). See: https://hackage.haskell.org/package/warp-3.3.13/docs/src/Network.Wai.Handler.Warp.Settings.html#defaultSettings

This increases the header limit size to 100KB and makes it configurable.

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

  • Server

Steps to test and verify

curl -v -H @temp.log localhost:8080/v1/version

where temp.log is a header file greater than 50KB. In prior versions, this will throw a bad request.

@tirumaraiselvan tirumaraiselvan requested a review from a team as a code owner July 22, 2020 08:10
@0x777
Copy link
Member

0x777 commented Jul 22, 2020

@tirumaraiselvan I don't think we should change the default. Make it customisable?

@tirumaraiselvan tirumaraiselvan force-pushed the increase-header-limit branch 2 times, most recently from f8e88f4 to 070f223 Compare July 22, 2020 10:24
@tirumaraiselvan
Copy link
Contributor Author

@0x777 I have made it configurable now.

@tirumaraiselvan tirumaraiselvan changed the title increase header size limit for warp configure header size limit for warp Jul 22, 2020
@tirumaraiselvan
Copy link
Contributor Author

Currently, waiting on some triage in warp bug: yesodweb/wai#807

@Vlix
Copy link

Vlix commented Jan 23, 2021

I've set up a PR for the yesodweb/wai#807 issue.

@Vlix
Copy link

Vlix commented Feb 4, 2021

yesodweb/wai#807 has been fixed with yesodweb/wai#838

The calculation of header length was wrong in such a way that it could trigger at different sizes (it might let 30kb pass, or it might throw an exception) depending on how large the header chunks are and how many of them there are.
It should now be fixed and actually accept headers with the size up to the maximum setting (default 50kb)

@manuFL
Copy link

manuFL commented Feb 15, 2022

Really need this one... any ETA for the merge?

@Vlix
Copy link

Vlix commented Feb 15, 2022

@tirumaraiselvan The do not merge label can be removed, I think.

@tirumaraiselvan
Copy link
Contributor Author

Hey folks, apologies for the tardiness on this. We are going to get this in soon.

@pranshi06
Copy link
Contributor

Closing this PR because the issue has been addressed in 3124c93.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s/do-not-merge Do not merge this pull request to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants