Refuse to build feature objects larger than u16::MAX bytes#4705
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
|
No issues found. The PR diff now includes a second change beyond my prior review (the
Cross-cutting: still no unit test exercising the new boundary (65534/65535-byte features round-tripping through serialize/deserialize), though the author noted the change is trivial. |
| if byte_offset >= u16::MAX as usize { | ||
| return Err(()); | ||
| } |
There was a problem hiding this comment.
Shouldn't we also change the de-serialization code?
There was a problem hiding this comment.
Didn't really seem worth it tbh but certainly could.
There was a problem hiding this comment.
I think it would be worth it to keep the ser/deser code aligned. Should we also check when parsing with FeatureFlags::from?
There was a problem hiding this comment.
I'm not sure its worth making all the conversion logic fallible for this. Technically maybe we should, but given we won't deserialize them I'm not sure where they'd come from?
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
These aren't serialize-able and clearly bogus. Reported by Project Loupe
This shouldn't really matter in practice, but it aligns the serialization and deserialization logic. Reported by Project Loupe.
TheBlueMatt
force-pushed
the
2026-05-no-huge-features
branch
from
737d519 to
87c8c32
Compare
4 participants
These aren't serialize-able and clearly bogus.
Reported by Project Loupe
Not gonna bother with backports here this is trivial