Skip to content

quic: truncate CONNECTION_CLOSE frame reason field - #66644

Merged
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
efekrskl:quic/truncate-close-reason
Oct 20, 2026
Merged

nodejs-github-bot merged 4 commits into
nodejs:mainfrom
efekrskl:quic/truncate-close-reason

Conversation

@efekrskl

Copy link
Copy Markdown
Member

Long reason strings inside a CONNECTION_CLOSE frame could exceed the space available and crash the process.

This PR limits the reason string to 256 UTF-8 bytes. Any string longer than the limit will get truncated. Also adds new tests and updates the documentation.

I think 256 bytes is a sensible default because the default maximum UDP datagram payload size is 1200 bytes. Before the handshake ngtcp2 can coalesce packets, encoding the same reason with 768 (3x256) bytes at most, this should leave enough headroom. This also matches Google QUICHE's limit

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 10, 2026
Signed-off-by: Efe Karasakal <hi@efe.dev>
Signed-off-by: Efe Karasakal <hi@efe.dev>
@efekrskl
efekrskl force-pushed the quic/truncate-close-reason branch from 8f2fdd1 to 63c637c Compare October 10, 2026 12:38
@codecov

codecov Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.49%. Comparing base (c0daf39) to head (f37498d).
⚠️ Report is 155 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66644      +/-   ##
==========================================
+ Coverage   90.43%   90.49%   +0.06%     
==========================================
  Files         791      797       +6     
  Lines      276593   277598    +1005     
  Branches    53112    53330     +218     
==========================================
+ Hits       250126   251204    +1078     
+ Misses      16867    16800      -67     
+ Partials     9600     9594       -6     

see 83 files with indirect coverage changes

🚀 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.

@inoway46

This comment has been minimized.

@efekrskl efekrskl added the quic Issues and PRs related to the QUIC transport implementation. label Oct 13, 2026
Comment thread test/parallel/test-quic-session-close-reason-truncation.mjs Outdated
@pimterry pimterry moved this to Ready for Review in QUIC Team Oct 15, 2026
Signed-off-by: Efe Karasakal <hi@efe.dev>
@pimterry pimterry added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 16, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 16, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@pimterry pimterry added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 19, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 19, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@pimterry pimterry added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Oct 20, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 1a54c4f into nodejs:main Oct 20, 2026
76 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 1a54c4f

@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in QUIC Team Oct 20, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants