From 85492842ac6d5ada4dd065bf5e206c2cb25275e7 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Mon, 14 Oct 2019 22:31:22 -0700 Subject: [PATCH 1/2] Fix off-by-one error in FromPyObject for BigInt Fixes #629 --- src/types/num.rs | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/types/num.rs b/src/types/num.rs index 4ec02cd4775..91182f58479 100644 --- a/src/types/num.rs +++ b/src/types/num.rs @@ -308,7 +308,7 @@ mod bigint_conversion { } else if n_bits == 0 { 0 } else { - (n_bits as usize - 1) / 8 + 1 + (n_bits as usize - 1 + $is_signed) / 8 + 1 }; if n_bytes <= 128 { extract_small(ob, n_bytes, $is_signed) @@ -335,6 +335,8 @@ mod bigint_conversion { use crate::types::{PyDict, PyModule}; use indoc::indoc; use num_traits::{One, Zero}; + use std::any::TypeId; + use std::fmt; fn python_fib(py: Python) -> &PyModule { let fib_code = indoc!( @@ -420,6 +422,49 @@ mod bigint_conversion { let zero: BigInt = FromPyObject::extract(fib.call1("fib", (0,)).unwrap()).unwrap(); assert_eq!(zero, BigInt::from(0)); } + + /// `OverflowError` on converting python int to BigInt + #[test] + fn issue_629() { + let gil = Python::acquire_gil(); + let py = gil.python(); + fn test(value: T, py: Python) + where + T: 'static + + Clone + + ToPyObject + + for<'a> FromPyObject<'a> + + Eq + + fmt::Display + + fmt::Debug, + { + let type_id = TypeId::of::(); + let type_name = if type_id == TypeId::of::() { + "BigInt" + } else { + assert_eq!(type_id, TypeId::of::()); + "BigUint" + }; + println!("{}: {}", type_name, value); + let python_value = value.clone().to_object(py); + let roundtrip_value = python_value.extract::(py).unwrap(); + assert_eq!(value, roundtrip_value); + } + for i in 0..=256usize { + test(BigInt::from(i), py); + test(BigUint::from(i), py); + test(-BigInt::from(i), py); + test(BigInt::one() << i, py); + test(BigUint::one() << i, py); + test(-BigInt::one() << i, py); + test((BigInt::one() << i) + 1u32, py); + test((BigUint::one() << i) + 1u32, py); + test((-BigInt::one() << i) + 1u32, py); + test((BigInt::one() << i) - 1u32, py); + test((BigUint::one() << i) - 1u32, py); + test((-BigInt::one() << i) - 1u32, py); + } + } } } From f55037c3779521520971cd528b05680b283e440c Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 18 Oct 2019 16:20:16 -0700 Subject: [PATCH 2/2] switch to using macro for test & change test name --- src/types/num.rs | 58 ++++++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/src/types/num.rs b/src/types/num.rs index 91182f58479..e65820c8c06 100644 --- a/src/types/num.rs +++ b/src/types/num.rs @@ -335,8 +335,6 @@ mod bigint_conversion { use crate::types::{PyDict, PyModule}; use indoc::indoc; use num_traits::{One, Zero}; - use std::any::TypeId; - use std::fmt; fn python_fib(py: Python) -> &PyModule { let fib_code = indoc!( @@ -423,46 +421,34 @@ mod bigint_conversion { assert_eq!(zero, BigInt::from(0)); } - /// `OverflowError` on converting python int to BigInt + /// `OverflowError` on converting python int to BigInt, see issue #629 #[test] - fn issue_629() { + fn check_overflow() { let gil = Python::acquire_gil(); let py = gil.python(); - fn test(value: T, py: Python) - where - T: 'static - + Clone - + ToPyObject - + for<'a> FromPyObject<'a> - + Eq - + fmt::Display - + fmt::Debug, - { - let type_id = TypeId::of::(); - let type_name = if type_id == TypeId::of::() { - "BigInt" - } else { - assert_eq!(type_id, TypeId::of::()); - "BigUint" + macro_rules! test { + ($T:ty, $value:expr, $py:expr) => { + let value = $value; + println!("{}: {}", stringify!($T), value); + let python_value = value.clone().to_object(py); + let roundtrip_value = python_value.extract::<$T>(py).unwrap(); + assert_eq!(value, roundtrip_value); }; - println!("{}: {}", type_name, value); - let python_value = value.clone().to_object(py); - let roundtrip_value = python_value.extract::(py).unwrap(); - assert_eq!(value, roundtrip_value); } for i in 0..=256usize { - test(BigInt::from(i), py); - test(BigUint::from(i), py); - test(-BigInt::from(i), py); - test(BigInt::one() << i, py); - test(BigUint::one() << i, py); - test(-BigInt::one() << i, py); - test((BigInt::one() << i) + 1u32, py); - test((BigUint::one() << i) + 1u32, py); - test((-BigInt::one() << i) + 1u32, py); - test((BigInt::one() << i) - 1u32, py); - test((BigUint::one() << i) - 1u32, py); - test((-BigInt::one() << i) - 1u32, py); + // test a lot of values to help catch other bugs too + test!(BigInt, BigInt::from(i), py); + test!(BigUint, BigUint::from(i), py); + test!(BigInt, -BigInt::from(i), py); + test!(BigInt, BigInt::one() << i, py); + test!(BigUint, BigUint::one() << i, py); + test!(BigInt, -BigInt::one() << i, py); + test!(BigInt, (BigInt::one() << i) + 1u32, py); + test!(BigUint, (BigUint::one() << i) + 1u32, py); + test!(BigInt, (-BigInt::one() << i) + 1u32, py); + test!(BigInt, (BigInt::one() << i) - 1u32, py); + test!(BigUint, (BigUint::one() << i) - 1u32, py); + test!(BigInt, (-BigInt::one() << i) - 1u32, py); } } }