fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(h2): honour maxResponseSize - #5676

Merged
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size
Aug 19, 2026
Merged

fix(h2): honour maxResponseSize#5676
metcoder95 merged 2 commits into
nodejs:mainfrom
ondraulehla:fix/h2-max-response-size

Conversation

@ondraulehla

Copy link
Copy Markdown
Contributor

This relates to...

Nothing filed. I noticed it while reading the two client implementations side by side after #5638.

Rationale

maxResponseSize is documented on the HTTP/2 client's own page, docs/docs/api/H2CClient.md:

maxResponseSize {number} The maximum allowed response body size in bytes. Use -1 to disable. Default: -1.

It is declared in types/h2c-client.d.ts and validated in lib/dispatcher/client.js (an H2CClient with maxResponseSize: -5 throws InvalidArgumentError), then stored into client[kMaxResponseSize]. lib/dispatcher/client-h2.js never reads that symbol, so the value is validated and then ignored. onData() forwards every chunk to request.onResponseData(chunk) with no byte accounting.

Server sends 4 MiB, client asks for maxResponseSize: 1024:

HTTP/1.1 Clienth2c H2CClient
beforeUND_ERR_RES_EXCEEDED_MAX_SIZEresolves with 4194304 bytes
afterUND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

Same split for a streaming consumer: over h1 the body errors mid-stream, over h2 it just completes.

The enforcement path itself isn't a regression, h2 has never honoured the option: git log -S kMaxResponseSize -- lib/dispatcher/client-h2.js is empty, and 7.29.0 ignores it too once you pass allowH2: true.

What did change is what an ordinary caller gets. Against a TLS origin offering h2, with h2 mentioned nowhere in the code:

constagent=newAgent({maxResponseSize: 1024})awaitrequest(url,{dispatcher: agent})
7.29.08.10.0
plain new Agent({ maxResponseSize })UND_ERR_RES_EXCEEDED_MAX_SIZEresolves, 4194304 bytes
connect: { allowH2: false }UND_ERR_RES_EXCEEDED_MAX_SIZEUND_ERR_RES_EXCEEDED_MAX_SIZE

That's #4828 making allowH2 default to true. Opting out of h2 brings the limit back, which is the clearest sign of where it goes missing.

Changes

onData() counts bytes against maxResponseSize, mirroring onBody() in client-h1.js. The counter and the limit live on the pre-shaped per-request state object, so the hidden class is unchanged.

One deliberate difference from h1, and I'd like a second opinion on it. client-h1.js calls util.destroy(socket, ...) because HTTP/1.1 can't abandon one response without losing framing. Over h2 that would take out every sibling stream on the session, so this calls the existing per-request state.abort() instead, which resets only the offending stream. I verified the session survives: after an oversized stream is reset, later requests on the same client still succeed and the server reports one connection.

Tests

Two cases added to test/max-response-size.js, which was http/1.1 only. Both fail before the change (the oversized response resolves instead of throwing) and pass after:

  • an h2c response over the limit rejects with ResponseExceededMaxSizeError
  • the session stays usable, an oversized stream errors, a following request on the same client succeeds, and the server sees one connection

test/max-response-size.js: 6 pass. test/http2*.js test/h2c-client.js: 105 pass, 0 fail. The default (-1) path is unchanged, a 4 MiB body is still delivered in full with no limit set and with maxResponseSize: -1.

Signed-off-by: Ondřej Úlehla <106835858+ondraulehla@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (8f9cc1d) to head (2392304).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5676 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01% 
==========================================
Files 110 110 Lines 38777 38791 +14 ==========================================
+ Hits 36241 36258 +17 + Misses 2536 2533 -3 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@metcoder95
metcoder95 merged commit de1223f into nodejs:mainAug 19, 2026
36 of 38 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 31, 2026
Sign up for freeto 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.

4 participants

@ondraulehla@codecov-commenter@mcollina@metcoder95