Skip to content

Commit 8c663c8

Browse files
Copilotdannywillems
andcommitted
Fix TryFromIntError when reading Number<u32> and Number<u64> from binprot
The issue occurred when binprot streams contained i64 values outside the i32 range. The binprot library reads all integers as i64 internally and uses try_from to convert to target types, which fails with TryFromIntError for out-of-range values. The fix changes Number<u32> and Number<u64> to read i64 directly and cast to the target type, avoiding the intermediate conversion that could fail. Writing still uses the original format to maintain wire compatibility. Added regression tests to verify the fix handles: - i64 values larger than i32::MAX - Negative i64 values - All existing test cases continue to pass Co-authored-by: dannywillems <6018454+dannywillems@users.noreply.github.com>
1 parent 2731abd commit 8c663c8

File tree

1 file changed

+97
-2
lines changed

1 file changed

+97
-2
lines changed

mina-p2p-messages/src/number.rs

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,52 @@ macro_rules! binprot_number {
197197

198198
binprot_number!(i32, i32);
199199
binprot_number!(i64, i64);
200-
binprot_number!(u32, i32);
201-
binprot_number!(u64, i64);
202200
binprot_number!(f64, f64);
203201

202+
// Custom implementations for u32 and u64 to handle the full i64 range
203+
// without TryFromIntError. The binprot format stores all integers as i64
204+
// internally, and conversion can fail if the value is outside the target
205+
// type's range. We read as i64 first, then cast to handle out-of-range values
206+
// gracefully. Writing still uses the smaller type to maintain wire format
207+
// compatibility.
208+
impl binprot::BinProtRead for Number<u32> {
209+
fn binprot_read<R: std::io::Read + ?Sized>(r: &mut R) -> Result<Self, binprot::Error>
210+
where
211+
Self: Sized,
212+
{
213+
// Read the value as i64 first to avoid TryFromIntError when the value
214+
// is outside i32 range, then cast to u32 using wrapping semantics.
215+
let value = i64::binprot_read(r)?;
216+
Ok(Self(value as u32))
217+
}
218+
}
219+
220+
impl binprot::BinProtWrite for Number<u32> {
221+
fn binprot_write<W: std::io::Write>(&self, w: &mut W) -> std::io::Result<()> {
222+
// Write as i32 to maintain wire format compatibility
223+
(self.0 as i32).binprot_write(w)
224+
}
225+
}
226+
227+
impl binprot::BinProtRead for Number<u64> {
228+
fn binprot_read<R: std::io::Read + ?Sized>(r: &mut R) -> Result<Self, binprot::Error>
229+
where
230+
Self: Sized,
231+
{
232+
// Read the value as i64 first to handle the full range without errors,
233+
// then cast to u64 using wrapping semantics.
234+
let value = i64::binprot_read(r)?;
235+
Ok(Self(value as u64))
236+
}
237+
}
238+
239+
impl binprot::BinProtWrite for Number<u64> {
240+
fn binprot_write<W: std::io::Write>(&self, w: &mut W) -> std::io::Result<()> {
241+
// Write as i64 to maintain wire format compatibility
242+
(self.0 as i64).binprot_write(w)
243+
}
244+
}
245+
204246
#[cfg(test)]
205247
mod tests {
206248
use binprot::{BinProtRead, BinProtWrite};
@@ -257,4 +299,57 @@ mod tests {
257299

258300
max_number_test!(u32_max, u32);
259301
max_number_test!(u64_max, u64);
302+
303+
/// Test that Number<u32> can read i64 values outside i32 range without error.
304+
/// This is a regression test for the TryFromIntError issue.
305+
#[test]
306+
fn u32_read_large_i64() {
307+
// Create a binprot-encoded i64 value larger than i32::MAX
308+
let large_i64 = (i32::MAX as i64) + 1000;
309+
let mut buf = Vec::new();
310+
large_i64.binprot_write(&mut buf).unwrap();
311+
312+
// This should not fail with TryFromIntError
313+
let mut r = buf.as_slice();
314+
let result = super::Number::<u32>::binprot_read(&mut r);
315+
assert!(result.is_ok(), "Should handle i64 values outside i32 range");
316+
317+
// The value should wrap as expected when cast from i64 to u32
318+
let n = result.unwrap();
319+
assert_eq!(n.0, large_i64 as u32);
320+
}
321+
322+
/// Test that Number<u32> can read negative i64 values without error.
323+
#[test]
324+
fn u32_read_negative_i64() {
325+
// Create a binprot-encoded negative i64 value
326+
let negative_i64 = -1000i64;
327+
let mut buf = Vec::new();
328+
negative_i64.binprot_write(&mut buf).unwrap();
329+
330+
// This should not fail with TryFromIntError
331+
let mut r = buf.as_slice();
332+
let result = super::Number::<u32>::binprot_read(&mut r);
333+
assert!(result.is_ok(), "Should handle negative i64 values");
334+
335+
// The value should wrap as expected when cast from i64 to u32
336+
let n = result.unwrap();
337+
assert_eq!(n.0, negative_i64 as u32);
338+
}
339+
340+
/// Test that Number<u64> can read i64 values without error.
341+
#[test]
342+
fn u64_read_i64() {
343+
// Test with a negative value (which would fail with try_from)
344+
let negative_i64 = -5000i64;
345+
let mut buf = Vec::new();
346+
negative_i64.binprot_write(&mut buf).unwrap();
347+
348+
let mut r = buf.as_slice();
349+
let result = super::Number::<u64>::binprot_read(&mut r);
350+
assert!(result.is_ok(), "Should handle negative i64 values");
351+
352+
let n = result.unwrap();
353+
assert_eq!(n.0, negative_i64 as u64);
354+
}
260355
}

0 commit comments

Comments
 (0)