Added extra header validation for total size of headers#219
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds safeguards in net/http to prevent excessively large HTTP headers by enforcing both per-field limits (key/value) and a new maximum total size for response headers during parsing.
Changes:
- Add a
Net::HTTPResponse::MAX_RESPONSE_HEADER_LENGTHlimit and enforce it while reading response headers. - Enforce
MAX_KEY_LENGTHvalidation consistently viavalidate_field_name, including for#[]=,#set_field, and#add_fieldpaths. - Add tests covering oversized response headers and overlong header keys/values.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/net/http/test_httpresponse.rb | Adds a regression test ensuring oversized response headers raise Net::HTTPBadResponse. |
| test/net/http/test_httpheader.rb | Adds tests for rejecting overlong header keys and values. |
| lib/net/http/response.rb | Introduces and enforces a total response header byte-size limit during header parsing. |
| lib/net/http/header.rb | Centralizes key-length validation and adds value-length validation for additional setter paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
304
to
+307
| val = val.to_s | ||
| if val.bytesize > MAX_FIELD_LENGTH | ||
| raise ArgumentError, "header has too long field value: #{val.bytesize}" | ||
| end |
The length limits only ran in initialize_http_header, which responses bypass: each_response_header builds the response through add_field and set_field, so an oversized response header field was never bounded. Check the field value length in set_field and append_field_value, and fold the key length check into validate_field_name so set_field and initialize_http_header share one place for validating field names. Co-authored-by: Yusuke Endoh <mame@ruby-lang.org> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
each_response_header read header lines until the blank separator with no bound on their total size, so a server could exhaust client memory by sending a large header block. Cap the cumulative size at 1 MiB and raise Net::HTTPBadResponse once it is exceeded. Co-authored-by: Yusuke Endoh <mame@ruby-lang.org> Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a safeguard to limit the total size of HTTP headers.