From fd84aec4853428000530bafbeef49442a8468582 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Tue, 4 Aug 2015 18:00:24 -0700 Subject: [PATCH] Fix parsing min, max, and epsilon f64 values --- serde_json/src/de.rs | 155 ++++++++++++++++++++++++--------- serde_json/src/ser.rs | 14 +-- serde_tests/tests/test_json.rs | 13 +-- 3 files changed, 122 insertions(+), 60 deletions(-) diff --git a/serde_json/src/de.rs b/serde_json/src/de.rs index 26ab0703..4a6e139e 100644 --- a/serde_json/src/de.rs +++ b/serde_json/src/de.rs @@ -110,10 +110,10 @@ impl Deserializer } b'-' => { try!(self.bump()); - self.parse_number(false, visitor) + self.parse_integer(false, visitor) } b'0' ... b'9' => { - self.parse_number(true, visitor) + self.parse_integer(true, visitor) } b'"' => { try!(self.parse_string()); @@ -151,11 +151,114 @@ impl Deserializer Ok(()) } - fn parse_number(&mut self, pos: bool, mut visitor: V) -> Result + fn parse_integer(&mut self, pos: bool, visitor: V) -> Result where V: de::Visitor, { - let res = try!(self.parse_integer()); + match self.ch_or_null() { + b'0' => { + try!(self.bump()); + // There can be only one leading '0'. + match self.ch_or_null() { + b'0' ... b'9' => { + Err(self.error(ErrorCode::InvalidNumber)) + } + _ => { + self.parse_number(pos, 0, visitor) + } + } + }, + c @ b'1' ... b'9' => { + try!(self.bump()); + + let mut res: u64 = (c as u64) - ('0' as u64); + + loop { + match self.ch_or_null() { + c @ b'0' ... b'9' => { + try!(self.bump()); + + let digit = (c as u64) - ('0' as u64); + + // We need to be careful with overflow. If we can, try to keep the + // number as a `u64` until we grow too large. At that point, switch to + // parsing the value as a `f64`. + match res.checked_mul(10) { + Some(res_) => { + res = res_; + match res.checked_add(digit) { + Some(res_) => { res = res_; } + None => { + return self.parse_float( + pos, + (res as f64) + (digit as f64), + visitor); + } + } + } + None => { + return self.parse_float( + pos, + (res as f64) * 10.0 + (digit as f64), + visitor); + } + } + } + _ => { + return self.parse_number(pos, res, visitor); + } + } + } + } + _ => { + Err(self.error(ErrorCode::InvalidNumber)) + } + } + } + + fn parse_float(&mut self, + pos: bool, + mut res: f64, + mut visitor: V) -> Result + where V: de::Visitor, + { + loop { + match self.ch_or_null() { + c @ b'0' ... b'9' => { + try!(self.bump()); + + let digit = (c as u64) - ('0' as u64); + + res *= 10.0; + res += digit as f64; + } + _ => { + match self.ch_or_null() { + b'.' => { + return self.parse_decimal(pos, res, visitor); + } + b'e' | b'E' => { + return self.parse_exponent(pos, res, visitor); + } + _ => { + if !pos { + res = -res; + } + + return visitor.visit_f64(res); + } + } + } + } + } + } + + fn parse_number(&mut self, + pos: bool, + res: u64, + mut visitor: V) -> Result + where V: de::Visitor, + { match self.ch_or_null() { b'.' => { self.parse_decimal(pos, res as f64, visitor) @@ -167,53 +270,19 @@ impl Deserializer if pos { visitor.visit_u64(res) } else { - let res = (res as i64).wrapping_neg(); + let res_i64 = (res as i64).wrapping_neg(); - // Make sure we didn't underflow. - if res > 0 { - Err(self.error(ErrorCode::InvalidNumber)) + // Convert into a float if we underflow. + if res_i64 > 0 { + visitor.visit_f64(-(res as f64)) } else { - visitor.visit_i64(res) + visitor.visit_i64(res_i64) } } } } } - fn parse_integer(&mut self) -> Result { - let mut accum: u64 = 0; - - match self.ch_or_null() { - b'0' => { - try!(self.bump()); - - // There can be only one leading '0'. - match self.ch_or_null() { - b'0' ... b'9' => { - return Err(self.error(ErrorCode::InvalidNumber)); - } - _ => () - } - }, - b'1' ... b'9' => { - while !self.eof() { - match self.ch_or_null() { - c @ b'0' ... b'9' => { - accum = try_or_invalid!(self, accum.checked_mul(10)); - accum = try_or_invalid!(self, accum.checked_add((c as u64) - ('0' as u64))); - - try!(self.bump()); - } - _ => break, - } - } - } - _ => { return Err(self.error(ErrorCode::InvalidNumber)); } - } - - Ok(accum) - } - fn parse_decimal(&mut self, pos: bool, mut res: f64, diff --git a/serde_json/src/ser.rs b/serde_json/src/ser.rs index 67674aa5..b03ce38b 100644 --- a/serde_json/src/ser.rs +++ b/serde_json/src/ser.rs @@ -465,12 +465,7 @@ fn fmt_f32_or_null(wr: &mut W, value: f32) -> io::Result<()> match value.classify() { FpCategory::Nan | FpCategory::Infinite => wr.write_all(b"null"), _ => { - let s = format!("{:?}", value); - try!(wr.write_all(s.as_bytes())); - if !s.contains('.') { - try!(wr.write_all(b".0")) - } - Ok(()) + write!(wr, "{:?}", value) } } } @@ -481,12 +476,7 @@ fn fmt_f64_or_null(wr: &mut W, value: f64) -> io::Result<()> match value.classify() { FpCategory::Nan | FpCategory::Infinite => wr.write_all(b"null"), _ => { - let s = format!("{:?}", value); - try!(wr.write_all(s.as_bytes())); - if !s.contains('.') { - try!(wr.write_all(b".0")) - } - Ok(()) + write!(wr, "{:?}", value) } } } diff --git a/serde_tests/tests/test_json.rs b/serde_tests/tests/test_json.rs index 69b1b7f8..d8e177b7 100644 --- a/serde_tests/tests/test_json.rs +++ b/serde_tests/tests/test_json.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::f64; use std::fmt::Debug; use std::i64; use std::marker::PhantomData; @@ -108,12 +109,12 @@ fn test_write_i64() { #[test] fn test_write_f64() { - let min_string = f64::MIN.to_string(); - let max_string = f64::MAX.to_string(); - let epsilon_string = f64::EPSILON.to_string(); + let min_string = format!("{:?}", f64::MIN); + let max_string = format!("{:?}", f64::MAX); + let epsilon_string = format!("{:?}", f64::EPSILON); let tests = &[ - (3.0, "3.0"), + (3.0, "3"), (3.1, "3.1"), (-1.5, "-1.5"), (0.5, "0.5"), @@ -727,7 +728,6 @@ fn test_parse_number_errors() { ("1e", Error::SyntaxError(ErrorCode::InvalidNumber, 1, 2)), ("1e+", Error::SyntaxError(ErrorCode::InvalidNumber, 1, 3)), ("1a", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 2)), - ("777777777777777777777777777", Error::SyntaxError(ErrorCode::InvalidNumber, 1, 20)), ("1e777777777777777777777777777", Error::SyntaxError(ErrorCode::InvalidNumber, 1, 23)), ]); } @@ -773,6 +773,9 @@ fn test_parse_f64() { ("0.00e00", 0.0), ("0.00e+00", 0.0), ("0.00e-00", 0.0), + (&format!("{:?}", (i64::MIN as f64) - 1.0), (i64::MIN as f64) - 1.0), + (&format!("{:?}", (u64::MAX as f64) + 1.0), (u64::MAX as f64) + 1.0), + (&format!("{:?}", f64::EPSILON), f64::EPSILON), ]); }