fix: fail on truncated bodies instead of NUL-padding them - #3195
Conversation
ByteUtil.readByteArray() returned its pre-allocated buffer when the stream ended before the requested number of bytes had been read, so the zero padding was indistinguishable from real content. On the Content-Length path (Body.readLocal) this produced no error at all: the body reported isRead() == true with its content padded to the announced length, and the truncated message was forwarded. The chunked reader handed the padded chunk to the observers and to the consumer, then failed one chunk later with the misleading "Empty chunk-size field"; ChunkedBody.writeStreamed() had already written it to the peer. Frame.read() padded HTTP/2 frame payloads the same way, although Frame.readByte() already threw EOFException for the frame header. readByteArray() now throws EOFException on a short read. As it extends IOException, every caller's existing handling turns it into a ReadingBodyException without a signature change. Reading until the end of the stream (length < 0) is unaffected. Note this changes a path that previously appeared to work: a truncated Content-Length body is now a hard body-read failure rather than silently padded. Closes #3193 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthrough
ChangesBody read integrity
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change rejects truncated HTTP and HTTP/2 content instead of forwarding zero-padded data, with existing error handling preserved. No actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation All changes are directly related to issue
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ByteUtil.readByteArray() returned its pre-allocated buffer when the stream ended before the requested number of bytes had been read, so the zero padding was indistinguishable from real content.
On the Content-Length path (Body.readLocal) this produced no error at all: the body reported isRead() == true with its content padded to the announced length, and the truncated message was forwarded. The chunked reader handed the padded chunk to the observers and to the consumer, then failed one chunk later with the misleading "Empty chunk-size field"; ChunkedBody.writeStreamed() had already written it to the peer. Frame.read() padded HTTP/2 frame payloads the same way, although Frame.readByte() already threw EOFException for the frame header.
readByteArray() now throws EOFException on a short read. As it extends IOException, every caller's existing handling turns it into a ReadingBodyException without a signature change. Reading until the end of the stream (length < 0) is unaffected.
Note this changes a path that previously appeared to work: a truncated Content-Length body is now a hard body-read failure rather than silently padded.
Closes #3193
Summary by CodeRabbit