Merge pull request servo#73 from servo/overflow

Clamp to MAX / INFINITY rather than panic on overflow in number parsing

Author: SimonSapin

src/color.rs

@@ -308,18 +308,18 @@ fn parse_color_function(name: &str, arguments: &mut Parser) -> Result<Color, ()>
         // Either integers or percentages, but all the same type.
         match try!(arguments.next()) {
             Token::Number(ref v) if v.int_value.is_some() => {
-                red = (v.value / 255.) as f32;
+                red = v.value / 255.;
                 try!(arguments.expect_comma());
                 green = try!(arguments.expect_integer()) as f32 / 255.;
                 try!(arguments.expect_comma());
                 blue = try!(arguments.expect_integer()) as f32 / 255.;
             }
             Token::Percentage(ref v) => {
-                red = v.unit_value as f32;
+                red = v.unit_value;
                 try!(arguments.expect_comma());
-                green = try!(arguments.expect_percentage()) as f32;
+                green = try!(arguments.expect_percentage());
                 try!(arguments.expect_comma());
-                blue = try!(arguments.expect_percentage()) as f32;
+                blue = try!(arguments.expect_percentage());
             }
             _ => return Err(())
         };
@@ -332,7 +332,7 @@ fn parse_color_function(name: &str, arguments: &mut Parser) -> Result<Color, ()>
         let lightness = (try!(arguments.expect_percentage())).max(0.).min(1.);
 
         // http://www.w3.org/TR/css3-color/#hsl-color
-        fn hue_to_rgb(m1: f64, m2: f64, mut h: f64) -> f64 {
+        fn hue_to_rgb(m1: f32, m2: f32, mut h: f32) -> f32 {
             if h < 0. { h += 1. }
             if h > 1. { h -= 1. }
 
@@ -344,14 +344,14 @@ fn parse_color_function(name: &str, arguments: &mut Parser) -> Result<Color, ()>
         let m2 = if lightness <= 0.5 { lightness * (saturation + 1.) }
                  else { lightness + saturation - lightness * saturation };
         let m1 = lightness * 2. - m2;
-        red = hue_to_rgb(m1, m2, hue + 1. / 3.) as f32;
-        green = hue_to_rgb(m1, m2, hue) as f32;
-        blue = hue_to_rgb(m1, m2, hue - 1. / 3.) as f32;
+        red = hue_to_rgb(m1, m2, hue + 1. / 3.);
+        green = hue_to_rgb(m1, m2, hue);
+        blue = hue_to_rgb(m1, m2, hue - 1. / 3.);
     }
 
     let alpha = if has_alpha {
         try!(arguments.expect_comma());
-        (try!(arguments.expect_number())).max(0.).min(1.) as f32
+        (try!(arguments.expect_number())).max(0.).min(1.)
     } else {
         1.
     };

src/nth.rs

@@ -57,7 +57,7 @@ fn parse_b(input: &mut Parser, a: i32) -> Result<(i32, i32), ()> {
     match input.next() {
         Ok(Token::Delim('+')) => parse_signless_b(input, a, 1),
         Ok(Token::Delim('-')) => parse_signless_b(input, a, -1),
-        Ok(Token::Number(ref value)) if value.signed => {
+        Ok(Token::Number(ref value)) if value.has_sign => {
             Ok((a, try!(value.int_value.ok_or(())) as i32))
         }
         _ => {
@@ -69,7 +69,7 @@ fn parse_b(input: &mut Parser, a: i32) -> Result<(i32, i32), ()> {
 
 fn parse_signless_b(input: &mut Parser, a: i32, b_sign: i32) -> Result<(i32, i32), ()> {
     match try!(input.next()) {
-        Token::Number(ref value) if !value.signed => {
+        Token::Number(ref value) if !value.has_sign => {
             Ok((a, b_sign * (try!(value.int_value.ok_or(())) as i32)))
         }
         _ => Err(())

src/parser.rs

@@ -512,7 +512,7 @@ impl<'i, 't> Parser<'i, 't> {
 
     /// Parse a <number-token> and return the integer value.
     #[inline]
-    pub fn expect_number(&mut self) -> Result<f64, ()> {
+    pub fn expect_number(&mut self) -> Result<f32, ()> {
         match try!(self.next()) {
             Token::Number(NumericValue { value, .. }) => Ok(value),
             _ => Err(())
@@ -521,7 +521,7 @@ impl<'i, 't> Parser<'i, 't> {
 
     /// Parse a <number-token> that does not have a fractional part, and return the integer value.
     #[inline]
-    pub fn expect_integer(&mut self) -> Result<i64, ()> {
+    pub fn expect_integer(&mut self) -> Result<i32, ()> {
         match try!(self.next()) {
             Token::Number(NumericValue { int_value, .. }) => int_value.ok_or(()),
             _ => Err(())
@@ -531,7 +531,7 @@ impl<'i, 't> Parser<'i, 't> {
     /// Parse a <percentage-token> and return the value.
     /// `0%` and `100%` map to `0.0` and `1.0` (not `100.0`), respectively.
     #[inline]
-    pub fn expect_percentage(&mut self) -> Result<f64, ()> {
+    pub fn expect_percentage(&mut self) -> Result<f32, ()> {
         match try!(self.next()) {
             Token::Percentage(PercentageValue { unit_value, .. }) => Ok(unit_value),
             _ => Err(())

src/serializer.rs

@@ -48,7 +48,7 @@ pub trait ToCss {
 fn write_numeric<W>(value: NumericValue, dest: &mut W) -> text_writer::Result
 where W: TextWriter {
     // `value.value >= 0` is true for negative 0.
-    if value.signed && value.value.is_sign_positive() {
+    if value.has_sign && value.value.is_sign_positive() {
         try!(dest.write_str("+"));
     }
 
@@ -93,11 +93,11 @@ impl<'a> ToCss for Token<'a> {
             Token::Delim(value) => try!(dest.write_char(value)),
 
             Token::Number(value) => try!(write_numeric(value, dest)),
-            Token::Percentage(PercentageValue { unit_value, int_value, signed }) => {
+            Token::Percentage(PercentageValue { unit_value, int_value, has_sign }) => {
                 let value = NumericValue {
                     value: unit_value * 100.,
                     int_value: int_value,
-                    signed: signed,
+                    has_sign: has_sign,
                 };
                 try!(write_numeric(value, dest));
                 try!(dest.write_char('%'));

src/tests.rs

@@ -397,6 +397,62 @@ fn line_numbers() {
     assert_eq!(input.next_including_whitespace(), Err(()));
 }
 
+#[test]
+fn overflow() {
+    use std::iter::repeat;
+    use std::f32;
+
+    let css = r"
+         2147483646
+         2147483647
+         2147483648
+         10000000000000
+         1000000000000000000000000000000000000000
+         1{309 zeros}
+
+         -2147483647
+         -2147483648
+         -2147483649
+         -10000000000000
+         -1000000000000000000000000000000000000000
+         -1{309 zeros}
+
+         3.30282347e+38
+         3.40282347e+38
+         3.402824e+38
+
+         -3.30282347e+38
+         -3.40282347e+38
+         -3.402824e+38
+
+    ".replace("{309 zeros}", &repeat('0').take(309).collect::<String>());
+    let mut input = Parser::new(&css);
+
+    assert_eq!(input.expect_integer(), Ok(2147483646));
+    assert_eq!(input.expect_integer(), Ok(2147483647));
+    assert_eq!(input.expect_integer(), Ok(2147483647)); // Clamp on overflow
+    assert_eq!(input.expect_integer(), Ok(2147483647));
+    assert_eq!(input.expect_integer(), Ok(2147483647));
+    assert_eq!(input.expect_integer(), Ok(2147483647));
+
+    assert_eq!(input.expect_integer(), Ok(-2147483647));
+    assert_eq!(input.expect_integer(), Ok(-2147483648));
+    assert_eq!(input.expect_integer(), Ok(-2147483648)); // Clamp on overflow
+    assert_eq!(input.expect_integer(), Ok(-2147483648));
+    assert_eq!(input.expect_integer(), Ok(-2147483648));
+    assert_eq!(input.expect_integer(), Ok(-2147483648));
+
+    assert_eq!(input.expect_number(), Ok(3.30282347e+38));
+    assert_eq!(input.expect_number(), Ok(f32::MAX));
+    assert_eq!(input.expect_number(), Ok(f32::INFINITY));
+    assert!(f32::MAX != f32::INFINITY);
+
+    assert_eq!(input.expect_number(), Ok(-3.30282347e+38));
+    assert_eq!(input.expect_number(), Ok(f32::MIN));
+    assert_eq!(input.expect_number(), Ok(f32::NEG_INFINITY));
+    assert!(f32::MIN != f32::NEG_INFINITY);
+}
+
 #[test]
 fn line_delimited() {
     let mut input = Parser::new(" { foo ; bar } baz;,");
@@ -533,11 +589,11 @@ fn one_component_value_to_json(token: Token, input: &mut Parser) -> Json {
         Token::Delim(value) => value.to_string().to_json(),
 
         Token::Number(value) => Json::Array(vec!["number".to_json()] + &*numeric(value)),
-        Token::Percentage(PercentageValue { unit_value, int_value, signed }) => Json::Array(
+        Token::Percentage(PercentageValue { unit_value, int_value, has_sign }) => Json::Array(
             vec!["percentage".to_json()] + &*numeric(NumericValue {
                 value: unit_value * 100.,
                 int_value: int_value,
-                signed: signed,
+                has_sign: has_sign,
             })),
         Token::Dimension(value, unit) => Json::Array(
             vec!["dimension".to_json()] + &*numeric(value) + &[unit.to_json()][..]),

src/tokenizer.rs

@@ -10,6 +10,7 @@ use std::char;
 use std::ascii::AsciiExt;
 use std::borrow::{Cow, ToOwned};
 use std::borrow::Cow::{Owned, Borrowed};
+use std::i32;
 
 use self::Token::*;
 
@@ -177,29 +178,29 @@ impl<'a> Token<'a> {
 #[derive(PartialEq, Debug, Copy, Clone)]
 pub struct NumericValue {
     /// The value as a float
-    pub value: f64,
+    pub value: f32,
 
     /// If the origin source did not include a fractional part, the value as an integer.
-    pub int_value: Option<i64>,
+    pub int_value: Option<i32>,
 
     /// Whether the number had a `+` or `-` sign.
     ///
     /// This is used is some cases like the <An+B> micro syntax. (See the `parse_nth` function.)
-    pub signed: bool,
+    pub has_sign: bool,
 }
 
 
 /// The numeric value of `Percentage` tokens.
 #[derive(PartialEq, Debug, Copy, Clone)]
 pub struct PercentageValue {
     /// The value as a float, divided by 100 so that the nominal range is 0.0 to 1.0.
-    pub unit_value: f64,
+    pub unit_value: f32,
 
     /// If the origin source did not include a fractional part, the value as an integer. It is **not** divided by 100.
-    pub int_value: Option<i64>,
+    pub int_value: Option<i32>,
 
     /// Whether the number had a `+` or `-` sign.
-    pub signed: bool,
+    pub has_sign: bool,
 }
 
 
@@ -653,32 +654,51 @@ fn consume_name<'a>(tokenizer: &mut Tokenizer<'a>) -> Cow<'a, str> {
 }
 
 
-fn consume_digits(tokenizer: &mut Tokenizer) {
-    while !tokenizer.is_eof() {
-        match tokenizer.next_char() {
-            '0'...'9' => tokenizer.advance(1),
-            _ => break
-        }
-    }
-}
-
-
 fn consume_numeric<'a>(tokenizer: &mut Tokenizer<'a>) -> Token<'a> {
     // Parse [+-]?\d*(\.\d+)?([eE][+-]?\d+)?
     // But this is always called so that there is at least one digit in \d*(\.\d+)?
-    let start_pos = tokenizer.position();
-    let mut is_integer = true;
-    let signed = matches!(tokenizer.next_char(), '-' | '+');
-    if signed {
+
+    // Do all the math in f64 so that large numbers overflow to +/-inf
+    // and i32::{MIN, MAX} are within range.
+
+    let (has_sign, sign) = match tokenizer.next_char() {
+        '-' => (true, -1.),
+        '+' => (true, 1.),
+        _ => (false, 1.),
+    };
+    if has_sign {
         tokenizer.advance(1);
     }
-    consume_digits(tokenizer);
+
+    let mut integral_part: f64 = 0.;
+    while let Some(digit) = tokenizer.next_char().to_digit(10) {
+        integral_part = integral_part * 10. + digit as f64;
+        tokenizer.advance(1);
+        if tokenizer.is_eof() {
+            break
+        }
+    }
+
+    let mut is_integer = true;
+
+    let mut fractional_part: f64 = 0.;
     if tokenizer.has_at_least(1) && tokenizer.next_char() == '.'
             && matches!(tokenizer.char_at(1), '0'...'9') {
         is_integer = false;
-        tokenizer.advance(2);  // '.' and first digit
-        consume_digits(tokenizer);
+        tokenizer.advance(1);  // Consume '.'
+        let mut divisor = 10.;
+        while let Some(digit) = tokenizer.next_char().to_digit(10) {
+            fractional_part += digit as f64 / divisor;
+            divisor *= 10.;
+            tokenizer.advance(1);
+            if tokenizer.is_eof() {
+                break
+            }
+        }
     }
+
+    let mut value = sign * (integral_part + fractional_part);
+
     if (
         tokenizer.has_at_least(1)
         && matches!(tokenizer.next_char(), 'e' | 'E')
@@ -690,37 +710,56 @@ fn consume_numeric<'a>(tokenizer: &mut Tokenizer<'a>) -> Token<'a> {
         && matches!(tokenizer.char_at(2), '0'...'9')
     ) {
         is_integer = false;
-        tokenizer.advance(2);  // 'e' or 'E', and sign or first digit
-        consume_digits(tokenizer);
-    }
-    let (value, int_value) = {
-        let mut repr = tokenizer.slice_from(start_pos);
-        // Remove any + sign as int::parse() does not parse them.
-        if repr.starts_with("+") {
-            repr = &repr[1..]
+        tokenizer.advance(1);
+        let (has_sign, sign) = match tokenizer.next_char() {
+            '-' => (true, -1.),
+            '+' => (true, 1.),
+            _ => (false, 1.),
+        };
+        if has_sign {
+            tokenizer.advance(1);
+        }
+        let mut exponent: f64 = 0.;
+        while let Some(digit) = tokenizer.next_char().to_digit(10) {
+            exponent = exponent * 10. + digit as f64;
+            tokenizer.advance(1);
+            if tokenizer.is_eof() {
+                break
+            }
         }
-        // TODO: handle overflow
-        (repr.parse::<f64>().unwrap(), if is_integer {
-            Some(repr.parse::<i64>().unwrap())
+        value *= f64::powf(10., sign * exponent);
+    }
+
+    let int_value = if is_integer {
+        Some(if value >= i32::MAX as f64 {
+            i32::MAX
+        } else if value <= i32::MIN as f64 {
+            i32::MIN
         } else {
-            None
+            value as i32
         })
+    } else {
+        None
     };
+
     if !tokenizer.is_eof() && tokenizer.next_char() == '%' {
         tokenizer.advance(1);
         return Percentage(PercentageValue {
-            unit_value: value / 100.,
+            unit_value: value as f32 / 100.,
             int_value: int_value,
-            signed: signed,
+            has_sign: has_sign,
         })
     }
     let value = NumericValue {
-        value: value,
+        value: value as f32,
         int_value: int_value,
-        signed: signed,
+        has_sign: has_sign,
     };
-    if is_ident_start(tokenizer) { Dimension(value, consume_name(tokenizer)) }
-    else { Number(value) }
+    if is_ident_start(tokenizer) {
+        Dimension(value, consume_name(tokenizer))
+    } else {
+        Number(value)
+    }
 }