Skip to content

fix: add MaxHeaderSize guard to HeaderScanner - #1533

Open
lxcxjxhx wants to merge 1 commit into
cloudwego:mainfrom
lxcxjxhx:fix/header-size-limit
Open

lxcxjxhx wants to merge 1 commit into
cloudwego:mainfrom
lxcxjxhx:fix/header-size-limit

Conversation

@lxcxjxhx

Copy link
Copy Markdown

What

Add a MaxHeaderSize field to HeaderScanner. When set to a positive value, the scanner stops parsing and returns errHeaderTooLong once the cumulative header bytes exceed the limit. The default is 0 (no limit), so existing callers are unaffected.

Why

Malicious or buggy clients can send an arbitrarily large header block and cause the parser to loop until OOM. Having a built-in size guard lets callers defend against this without wrapping the scanner externally.

Changes

  • headerscanner.go: add MaxHeaderSize int field and a size check at the top of Next()
  • headerscanner_test.go: add TestHeaderScannerMaxHeaderSize and TestHeaderScannerMaxHeaderSizeDisabled

Testing

go test ./pkg/protocol/http1/ext/...

All existing tests pass. New tests cover:

  • Headers within limit → parsed normally
  • Headers exceeding limit → errHeaderTooLong
  • MaxHeaderSize=0 → no limit (backward compatible)

Closes #1497

@CLAassistant

CLAassistant commented Aug 23, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@lxcxjxhx
lxcxjxhx force-pushed the fix/header-size-limit branch from ddab209 to 1db5931 Compare November 7, 2026 13:36
@lxcxjxhx

lxcxjxhx commented Nov 7, 2026

Copy link
Copy Markdown
Author

Rebased onto the current main so CI runs on today's toolchain.

The red checks on the previous head were both pkg/common/compress assertions, not this change:

--- FAIL: TestCompressAppendGzipBytesLevel
--- FAIL: TestCompressWriteGzipLevel
FAIL	github.com/cloudwego/hertz/pkg/common/compress

This PR touches pkg/protocol/http1/ext/headerscanner.go and its test; the compress package does not import it, and the failures date from the 2026-08-24 run while main's own Unit Tests have been green since.

Not run locally: this host has no Go toolchain, so the evidence is the rebased diff plus the added headerscanner_test.go cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

improvement(http1): robustness of request header parsing in req/header.go

2 participants