-
Notifications
You must be signed in to change notification settings - Fork 252
Add support for Smithy bigInteger and bigDecimal types as string wrappers in aws-smithy-types, allowing users to parse with their preferred big number library. #4418
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
Changes from 1 commit
50a694f
20e566b
813537f
f966002
37cb0f6
3f5818c
b367781
df1dd7e
2465383
3a970ce
5c9b731
06332c8
3679a5c
96b2a07
f81e913
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -322,13 +322,8 @@ impl<'a> JsonTokenIterator<'a> { | |
| Ok(Token::ValueNumber { | ||
| offset, | ||
| value: if floating { | ||
| // For BigInteger/BigDecimal support: Use NaN for f64 validation when the number | ||
| // exceeds f64 range (e.g., 1.8e308 > f64::MAX), allowing tokenization to succeed | ||
| // while preserving the original string for arbitrary precision types. | ||
| Number::Float( | ||
| f64::from_str(number_str) | ||
| .map_err(|_| self.error_at(start, InvalidNumber)) | ||
| .map(|f| if f.is_finite() { f } else { f64::NAN })?, | ||
| f64::from_str(number_str).map_err(|_| self.error_at(start, InvalidNumber))?, | ||
| ) | ||
| } else if negative { | ||
| // If the negative value overflows, then stuff it into an f64 | ||
|
|
@@ -666,29 +661,38 @@ mod tests { | |
| } | ||
|
|
||
| #[test] | ||
| fn out_of_range_floats_use_nan() { | ||
| // Values exceeding f64::MAX should tokenize successfully with NaN | ||
| // to support BigInteger/BigDecimal arbitrary precision types | ||
| let expect_nan = |input| { | ||
| fn out_of_range_floats_produce_infinity() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did this change to infinity? I thought we chose nan?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, I see. the behavior of the parser is Infinity, that's fine.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is +/- infinity a valid value for Smithy |
||
| // Values exceeding f64::MAX should tokenize as infinity | ||
| // The consumer layer (token.rs) will convert to NaN for BigInteger/BigDecimal | ||
| let expect_infinity = |input, should_be_positive| { | ||
| let token = json_token_iter(input).next().unwrap().unwrap(); | ||
| if let Token::ValueNumber { | ||
| value: Number::Float(f), | ||
| .. | ||
| } = token | ||
| { | ||
| assert!(f.is_nan(), "Expected NaN for out-of-range value, got {}", f); | ||
| assert!( | ||
| f.is_infinite(), | ||
| "Expected infinity for out-of-range value, got {}", | ||
| f | ||
| ); | ||
| if should_be_positive { | ||
| assert!(f.is_sign_positive(), "Expected positive infinity"); | ||
| } else { | ||
| assert!(f.is_sign_negative(), "Expected negative infinity"); | ||
| } | ||
| } else { | ||
| panic!("Expected Float token, got {:?}", token); | ||
| } | ||
| }; | ||
|
|
||
| // Values > f64::MAX | ||
| expect_nan(b"1.8e308"); | ||
| expect_nan(b"9.9e999"); | ||
| expect_infinity(b"1.8e308", true); | ||
| expect_infinity(b"9.9e999", true); | ||
|
|
||
| // Negative values < -f64::MAX | ||
| expect_nan(b"-1.8e308"); | ||
| expect_nan(b"-9.9e999"); | ||
| expect_infinity(b"-1.8e308", false); | ||
| expect_infinity(b"-9.9e999", false); | ||
| } | ||
|
|
||
| // These cases actually shouldn't parse according to the spec, but it's easier | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,9 +26,28 @@ impl std::fmt::Display for BigNumberError { | |
|
|
||
| impl std::error::Error for BigNumberError {} | ||
|
|
||
| /// Validates that a string contains only valid JSON number characters. | ||
| /// Prevents JSON injection by rejecting strings with quotes, commas, braces, etc. | ||
| fn is_valid_number_string(s: &str) -> bool { | ||
| /// Validates that a string is a valid BigInteger format. | ||
| /// Only allows digits and an optional leading sign. | ||
| fn is_valid_big_integer(s: &str) -> bool { | ||
| if s.is_empty() { | ||
| return false; | ||
| } | ||
|
|
||
| let mut chars = s.chars(); | ||
|
|
||
| // Check first character (can be sign or digit) | ||
| match chars.next() { | ||
| Some('-') | Some('+') | Some('0'..='9') => {} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you really proceed a number with |
||
| _ => return false, | ||
| } | ||
|
|
||
| // Rest must be digits only | ||
| chars.all(|c| matches!(c, '0'..='9')) | ||
| } | ||
|
|
||
| /// Validates that a string is a valid BigDecimal format. | ||
| /// Allows digits, sign, decimal point, and scientific notation. | ||
| fn is_valid_big_decimal(s: &str) -> bool { | ||
| if s.is_empty() { | ||
| return false; | ||
| } | ||
|
|
@@ -54,7 +73,7 @@ impl std::str::FromStr for BigInteger { | |
| type Err = BigNumberError; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| if !is_valid_number_string(s) { | ||
| if !is_valid_big_integer(s) { | ||
| return Err(BigNumberError::InvalidFormat(s.to_string())); | ||
| } | ||
| Ok(Self(s.to_string())) | ||
|
|
@@ -84,7 +103,7 @@ impl std::str::FromStr for BigDecimal { | |
| type Err = BigNumberError; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| if !is_valid_number_string(s) { | ||
| if !is_valid_big_decimal(s) { | ||
| return Err(BigNumberError::InvalidFormat(s.to_string())); | ||
| } | ||
| Ok(Self(s.to_string())) | ||
|
|
@@ -166,6 +185,25 @@ mod tests { | |
| assert!(BigInteger::from_str("").is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn big_integer_rejects_decimal_and_scientific() { | ||
| // BigInteger should reject decimal points | ||
| assert!(BigInteger::from_str("123.45").is_err()); | ||
| assert!(BigInteger::from_str("123.0").is_err()); | ||
|
|
||
| // BigInteger should reject scientific notation | ||
| assert!(BigInteger::from_str("1e10").is_err()); | ||
| assert!(BigInteger::from_str("1E10").is_err()); | ||
| assert!(BigInteger::from_str("1.23e10").is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn big_integer_accepts_signs() { | ||
| assert!(BigInteger::from_str("+123").is_ok()); | ||
| assert!(BigInteger::from_str("-123").is_ok()); | ||
| assert_eq!(BigInteger::from_str("+123").unwrap().as_ref(), "+123"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn big_decimal_rejects_invalid_chars() { | ||
| assert!(BigDecimal::from_str("abc").is_err()); | ||
|
|
||
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.
Note to reviewers:
I have added serialization and parsing logic for the following protocols:
Let me know if there are any other protocols.