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

http: Restrict maximum size of http + headers #6859

Merged
merged 1 commit into from Oct 21, 2015

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Oct 20, 2015

Prevent memory exhaustion by sending lots of data.
Also add a test to httpbasics.py.

Closes #6425

@dcousens
Copy link
Contributor

dcousens commented Oct 20, 2015

Will this restrict #6844 such that transactions greater than 8kB will be restricted?

@@ -414,6 +417,7 @@ bool InitHTTPServer()
}

evhttp_set_timeout(http, GetArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT));
evhttp_set_max_headers_size(http, MAX_HEADERS_SIZE);
Copy link
Contributor

@dcousens dcousens Oct 20, 2015

Choose a reason for hiding this comment

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

Wait, this PR is only further restricting the headers.
Shouldn't the title be then: "http: Restrict maximum size of headers"

Copy link
Contributor

@pstratem pstratem Oct 21, 2015

Choose a reason for hiding this comment

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

This should also be restricting the body size, but to something much larger.

Edit: derp it is below!

@laanwj
Copy link
Member Author

laanwj commented Oct 20, 2015

To be precise, it restricts the size of request line + headers, the first part of the request. The body is limited separately (to a much larger size).

#6844 should use POST data to submit the transaction, which is not affected by this.

@jgarzik
Copy link
Contributor

jgarzik commented Oct 20, 2015

ut ACK - agree "http + headers" in commit msg seemed to imply http body

Prevent memory exhaustion by sending lots of data.
Also add a test to `httpbasics.py`.

Closes bitcoin#6425
@laanwj laanwj force-pushed the 2015_10_max_http_headers branch from 3ae69de to 41db8c4 Compare Oct 20, 2015
@laanwj
Copy link
Member Author

laanwj commented Oct 20, 2015

Updated the ocmmit message

@sipa
Copy link
Member

sipa commented Oct 20, 2015

@dcousens
Copy link
Contributor

dcousens commented Oct 21, 2015

ACK

@pstratem
Copy link
Contributor

pstratem commented Oct 21, 2015

utACK

@laanwj laanwj merged commit 41db8c4 into bitcoin:master Oct 21, 2015
laanwj added a commit that referenced this pull request Oct 21, 2015
41db8c4 http: Restrict maximum size of request line + headers (Wladimir J. van der Laan)
sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request Mar 12, 2018
HTTP Server cherries from Core:

bitcoin/bitcoin#6719 - Make HTTP server shutdown more graceful
bitcoin/bitcoin#6859 - http: Restrict maximum size of http + headers
bitcoin/bitcoin#6990 - http: speed up shutdown
bitcoin/bitcoin#7966 - http: Do a pending c++11 simplification handling work items
bitcoin/bitcoin#8421 - httpserver: drop boost (#8023 dependency)
bitcoin/bitcoin#11006 - Improve shutdown process
marlengit pushed a commit to marlengit/BitcoinUnlimited that referenced this pull request Sep 25, 2018
HTTP Server cherries from Core:

bitcoin/bitcoin#6719 - Make HTTP server shutdown more graceful
bitcoin/bitcoin#6859 - http: Restrict maximum size of http + headers
bitcoin/bitcoin#6990 - http: speed up shutdown
bitcoin/bitcoin#7966 - http: Do a pending c++11 simplification handling work items
bitcoin/bitcoin#8421 - httpserver: drop boost (#8023 dependency)
bitcoin/bitcoin#11006 - Improve shutdown process
zkbot added a commit to zcash/zcash that referenced this pull request Sep 30, 2020
Small httpserver.cpp backports

Also includes a change to the `uiInterface.NotifyBlockTip` signal API.
These remove merge conflicts from subsequent backports for `sync.h`.

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6859
- bitcoin/bitcoin#7112
  - Only the non-QT changes.
- bitcoin/bitcoin#7966
- bitcoin/bitcoin#8421
  - We already backported the second commit in #2555
zkbot added a commit to zcash/zcash that referenced this pull request Oct 1, 2020
Small httpserver.cpp backports

Also includes a change to the `uiInterface.NotifyBlockTip` signal API.
These remove merge conflicts from subsequent backports for `sync.h`.

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6859
- bitcoin/bitcoin#7112
  - Only the non-QT changes.
- bitcoin/bitcoin#7966
- bitcoin/bitcoin#8421
  - We already backported the second commit in #2555
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std::getline() in rpcprotocol.cpp should be replaced by an alternative with a length limit
5 participants