Skip to content

Double the hardcoded max http header count - #26130

Merged
Jarred-Sumner merged 2 commits into
mainfrom
ali/double-max-http-header-count-httpparserh
Jan 15, 2026
Merged

Jarred-Sumner merged 2 commits into
mainfrom
ali/double-max-http-header-count-httpparserh

Conversation

@alii

@alii alii commented Jan 15, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Doubles the hardcoded max http header count

How did you verify your code works?

ci (?)

@robobun

robobun commented Jan 15, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 10:55 PM PT - Jan 14th, 2026

❌ @autofix-ci[bot], your commit 67e03ff has 3 failures in Build #34928 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 26130

That installs a local version of the PR into your bun-26130 executable, so you can run:

bun-26130 --bun

@coderabbitai

coderabbitai Bot commented Jan 15, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request increases the maximum HTTP header count in UWS from 100 to 200 and reorganizes imports in a test file by moving the HttpsProxyAgent import earlier without changing functionality.

Changes

Cohort / File(s) Summary
HTTP Header Limit Configuration
packages/bun-uws/src/HttpParser.h
Increased UWS_HTTP_MAX_HEADERS_COUNT macro from 100 to 200
Test Import Organization
test/js/node/http/node-http-agent-tls-options.test.mts
Relocated HttpsProxyAgent import to earlier position in file for improved organization
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Double the hardcoded max http header count' accurately and specifically describes the main change: increasing UWS_HTTP_MAX_HEADERS_COUNT from 100 to 200.
Description check ✅ Passed The PR description follows the required template structure with both 'What does this PR do?' and 'How did you verify your code works?' sections, though verification details are vague.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ed75a0e and 67e03ff.

📒 Files selected for processing (2)
  • packages/bun-uws/src/HttpParser.h
  • test/js/node/http/node-http-agent-tls-options.test.mts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

  • test/js/node/http/node-http-agent-tls-options.test.mts
📚 Learning: 2026-01-10T00:28:26.694Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 25938
File: packages/bun-uws/src/HttpParser.h:0-0
Timestamp: 2026-01-10T00:28:26.694Z
Learning: In packages/bun-uws/src/HttpParser.h, the headData and headLength fields on HttpRequest are intentionally left populated for all request types (not just CONNECT/upgrade), as they will be used in the future for parse errors and other Node.js compatibility features. They should not be cleared after the requestHandler call.

Applied to files:

  • packages/bun-uws/src/HttpParser.h
🔇 Additional comments (2)
packages/bun-uws/src/HttpParser.h (1)

20-22: LGTM! Consider the memory footprint increase.

Doubling the header limit from 100 to 200 is reasonable for supporting HTTP requests with many headers. Note that this increases the HttpRequest struct size by approximately 3.2KB (100 additional Header structs × ~32 bytes each) per request object, which should be acceptable for most use cases.

The #ifndef guard allows users to override this default if needed.

test/js/node/http/node-http-agent-tls-options.test.mts (1)

11-11: LGTM!

Import reorganization moves HttpsProxyAgent to the top of the import section, grouping it with other imports. No behavioral change.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@Jarred-Sumner
Jarred-Sumner merged commit 97feb66 into main Jan 15, 2026
54 of 56 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the ali/double-max-http-header-count-httpparserh branch January 15, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants