src: fix crash when writing odd-length hex string via Writev#63658
Open
RajeshKumar11 wants to merge 1 commit into
Open
src: fix crash when writing odd-length hex string via Writev#63658RajeshKumar11 wants to merge 1 commit into
RajeshKumar11 wants to merge 1 commit into
Conversation
fd3d0ec to
6a0c023
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63658 +/- ##
==========================================
- Coverage 91.95% 90.34% -1.61%
==========================================
Files 379 732 +353
Lines 166454 236434 +69980
Branches 25427 44532 +19105
==========================================
+ Hits 153058 213614 +60556
- Misses 13104 14522 +1418
- Partials 292 8298 +8006
🚀 New features to boost your workflow:
|
6a0c023 to
84ef8a0
Compare
StringBytes::StorageSize had a CHECK that fatal-asserted when a hex-encoded string with an odd number of characters was written through Writev (e.g. via HTTP requests which are automatically corked). Writing the same string via a single Write did not crash because StringBytes::Write delegates to HexDecode, which silently drops the trailing incomplete nibble. Remove the CHECK and let integer division handle odd lengths, which is consistent with StringBytes::Size and HexDecode. Fixes: nodejs#45150 Signed-off-by: RajeshKumar11 <kakumanurajeshkumar@gmail.com>
84ef8a0 to
0e3192c
Compare
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.
Summary
Writing an odd-length hex string to a stream that batches writes via
Writev (e.g.
http.requestwhich auto-corks its socket) wouldfatal-assert:
Reproduction:
Root cause
StringBytes::StorageSizehad aCHECKthat asserted the hex stringlength was even. When writes are batched through Writev, this check
runs before any data is written. A single Write (non-corked path)
did not crash because
StringBytes::WritecallsHexDecode, whichsilently drops the trailing incomplete nibble.
Fix
Remove the CHECK and use integer division for the HEX case, which
naturally rounds down for odd lengths. This is already the behaviour
of
StringBytes::SizeandHexDecode.Test
Added
test/parallel/test-http-odd-hex-write.jswith three cases:Fixes: #45150