-
Notifications
You must be signed in to change notification settings - Fork 43
Fix capsule protocol handling #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Changes: - Remove loop in Capsule::decode() - now decodes single capsule instead of skipping GREASE - Add explicit Grease variant to Capsule enum - Fix GREASE detection per RFC 9297 Section 5.4 (0x29 * N + 0x17) - Change Capsule::read() to return Option<Self> (returns None on clean EOF) - Update web-transport-quinn to handle new API This is foundational for properly handling HTTP/3 frames that may contain multiple capsules or GREASE values. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce support for GREASE capsules and improve capsule parsing robustness across two files. In 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@web-transport-proto/src/capsule.rs`:
- Around line 27-34: The size check currently uses payload.remaining() which
only reflects buffered bytes and delays rejecting declared-too-large capsules;
change the logic in the block around buf.take(...) to first compare
payload.limit() (the declared length) against MAX_MESSAGE_SIZE and return
CapsuleError::MessageTooLong immediately when limit() > MAX_MESSAGE_SIZE, then
keep the existing check that if payload.remaining() < payload.limit() return
CapsuleError::UnexpectedEnd; update references to payload.limit(),
payload.remaining(), MAX_MESSAGE_SIZE, CapsuleError::MessageTooLong,
CapsuleError::UnexpectedEnd and ensure behavior in the calling read() remains
unchanged.
- Line 115: Capsule::Grease currently serializes to zero bytes which produces an
invalid wire capsule; update the encode/write path for the Grease variant (e.g.,
in Capsule::encode and any write methods that match on Self::Grease) to emit a
valid GREASE capsule header with a GREASE type byte computed as 0x29 * n + 0x17
(choose a random n per encode) followed by a zero-length payload (i.e., write
the type and a zero length prefix), or alternatively return an Err/panic from
encode/write for the Grease variant if you intend it to be decode-only; make
sure to reference and change the match arm handling Self::Grease so callers
receive either a proper GREASE capsule or a clear error.
🧹 Nitpick comments (2)
web-transport-proto/src/capsule.rs (2)
44-47: RedundantMAX_MESSAGE_SIZEcheck — already guaranteed by the outer guard.After the check on line 28 ensures the full payload is ≤
MAX_MESSAGE_SIZE, the remaining bytes after consuming the 4-byte error code will always be less thanMAX_MESSAGE_SIZE. This inner check is dead code.
159-160:CapsuleError::UnknownTypeis now dead code.Since
decodeno longer returns this variant (unknown types produceCapsule::Unknown), this error variant is unreachable. Consider removing it to avoid confusion.
Summary
Capsule::decode()- now decodes single capsule instead of skipping GREASEGreasevariant toCapsuleenumCapsule::read()to returnOption<Self>(returnsNoneon clean EOF)Context
This changes the public API of the protocol layer to properly handle GREASE capsules and support decoding multiple capsules from a single buffer. Previously,
decode()would loop internally and skip GREASE capsules, which made it impossible to decode multiple capsules from HTTP/3 DATA frames.Breaking Changes
Capsule::read()now returnsOption<Capsule>instead ofCapsuleCapsuleenum has newGreasevariant🤖 Generated with Claude Code