fix: close review issues #6-#13 - #14
Closed
enochgroot wants to merge 2 commits into
Closed
enochgroot wants to merge 2 commits into
enochgroot wants to merge 2 commits into
Conversation
Pin nan 2.28.0 and c8 12.0.0; CI uses npm ci. Raise engines.node to >=22 and drop Node 18/20 from CI. Pin GitHub Actions to full SHAs. Cap Stream receive before allocate and drop buf on close. Apply kMaxContainer to pack array/map walks. Cap CLI stdin concat at MAX_STDIN_BYTES. Document a risk-based bump window in SECURITY.md. Closes #6 #7 #8 #9 #10 #11 #12 #13
This was referenced Sep 21, 2026
Node treats any error listener as handling, so dropBuf on the socket swallowed ECONNRESET. Drop the buffer, then emit on Stream when !dead so rejectLimit plus destroy(err) does not double-fire.
Owner
Author
|
Addressed review OPEN Low (9): Stream now re-emits underlying socket errors after dropBuf, gated on |
Owner
Author
|
Superseded by a rebase onto live upstream 3.4.0 ( |
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.
Closes #6 #7 #8 #9 #10 #11 #12 #13
One PR for the eight OPEN review findings on the 2.0 modernization tree.
Changes
nanto exact2.28.0; lockfile matches; CI usesnpm ci.engines.nodeis>=22; CI matrix is Node 22/24 only (no 18/20).msgpack.Streamrejects a receive concat that would exceedMAX_STREAM_BYTESbefore allocate; dropsbufon close/end/error.uses:pinned to full SHAs (checkout@v4.2.2,setup-node@v4.4.0,setup-python@v5.6.0).pack()snapshotsLength()then applieskMaxContaineron array and map walks.json2msgpack/msgpack2jsonrefuse stdin larger thanMAX_STDIN_BYTESbefore concat.SECURITY.mddocuments a risk-based bump window.c8to exact12.0.0.Tests
npm test: 135 pass.JS coverage gate 95% (c8
--check-coverage).Native coverage: 96.0% lines, 99.5% branches, 100% functions (
gcovr --fail-under-line 95 --fail-under-branch 95).Sabotage: Stream cap tests fail without the allocate check; pack sparse-array test fails without
kMaxContainer; CLI tests fail without the stdin cap.Coverage gate in
package.json/COVERAGE.mdis unchanged.