Merge pull request #174 from maciejhirsz/fixes

Fixes and formatting

Author: maciejhirsz

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "json"
-version = "0.12.1"
+version = "0.12.2"
 authors = ["Maciej Hirsz <hello@maciej.codes>"]
 description = "JSON implementation in Rust"
 repository = "https://github.com/maciejhirsz/json-rust"

src/codegen.rs

@@ -390,32 +390,27 @@ fn extend_from_slice(dst: &mut Vec<u8>, src: &[u8]) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::borrow::Borrow;
-    use crate::JsonValue;
-    use crate::parse;
-
-  // found while fuzzing the DumpGenerator
-
-  #[test]
-  fn should_not_panic_on_bad_bytes() {
-    let data = [0, 12, 128, 88, 64, 99].to_vec();
-    let s = unsafe {
-        String::from_utf8_unchecked(data)
-    };
-
-    let mut generator = DumpGenerator::new();
-    generator.write_string(&s);
-  }
-
-  #[test]
-  fn should_not_panic_on_bad_bytes_2() {
-    let data = b"\x48\x48\x48\x57\x03\xE8\x48\x48\xE8\x03\x8F\x48\x29\x48\x48";
-    let s = unsafe {
-        String::from_utf8_unchecked(data.to_vec())
-    };
-
-    let mut generator = DumpGenerator::new();
-    generator.write_string(&s);
-  }
 
+    // found while fuzzing the DumpGenerator
+    #[test]
+    fn should_not_panic_on_bad_bytes() {
+        let data = [0, 12, 128, 88, 64, 99].to_vec();
+        let s = unsafe {
+            String::from_utf8_unchecked(data)
+        };
+
+        let mut generator = DumpGenerator::new();
+        generator.write_string(&s).unwrap();
+    }
+
+    #[test]
+    fn should_not_panic_on_bad_bytes_2() {
+        let data = b"\x48\x48\x48\x57\x03\xE8\x48\x48\xE8\x03\x8F\x48\x29\x48\x48";
+        let s = unsafe {
+            String::from_utf8_unchecked(data.to_vec())
+        };
+
+        let mut generator = DumpGenerator::new();
+        generator.write_string(&s).unwrap();
+    }
 }

src/number.rs

@@ -1,5 +1,6 @@
 use std::{ ops, fmt, f32, f64 };
-use std::num::FpCategory;
+use std::convert::{TryFrom, Infallible};
+use std::num::{FpCategory, TryFromIntError};
 use crate::util::grisu2;
 use crate::util::print_dec;
 
@@ -383,6 +384,23 @@ impl PartialEq<Number> for f32 {
     }
 }
 
+/// Error type generated when trying to convert a `Number` into an
+/// integer of inadequate size.
+#[derive(Clone, Copy)]
+pub struct NumberOutOfScope;
+
+impl From<Infallible> for NumberOutOfScope {
+    fn from(_: Infallible) -> NumberOutOfScope {
+        NumberOutOfScope
+    }
+}
+
+impl From<TryFromIntError> for NumberOutOfScope {
+    fn from(_: TryFromIntError) -> NumberOutOfScope {
+        NumberOutOfScope
+    }
+}
+
 macro_rules! impl_unsigned {
     ($( $t:ty ),*) => ($(
         impl From<$t> for Number {
@@ -396,11 +414,24 @@ macro_rules! impl_unsigned {
             }
         }
 
+        impl TryFrom<Number> for $t {
+            type Error = NumberOutOfScope;
+
+            fn try_from(num: Number) -> Result<Self, Self::Error> {
+                let (positive, mantissa, exponent) = num.as_parts();
+
+                if !positive || exponent != 0 {
+                    return Err(NumberOutOfScope);
+                }
+
+                TryFrom::try_from(mantissa).map_err(Into::into)
+            }
+        }
+
         impl_integer!($t);
     )*)
 }
 
-
 macro_rules! impl_signed {
     ($( $t:ty ),*) => ($(
         impl From<$t> for Number {
@@ -421,34 +452,32 @@ macro_rules! impl_signed {
             }
         }
 
-        impl_integer!($t);
-    )*)
-}
-
+        impl TryFrom<Number> for $t {
+            type Error = NumberOutOfScope;
 
-macro_rules! impl_integer {
-    ($t:ty) => {
-        impl From<Number> for $t {
-            fn from(num: Number) -> $t {
+            fn try_from(num: Number) -> Result<Self, Self::Error> {
                 let (positive, mantissa, exponent) = num.as_parts();
 
-                if exponent <= 0 {
-                    if positive {
-                        mantissa as $t
-                    } else {
-                        -(mantissa as i64) as $t
-                    }
-                } else {
-                    // This may overflow, which is fine
-                    if positive {
-                        (mantissa * 10u64.pow(exponent as u32)) as $t
-                    } else {
-                        (-(mantissa as i64) * 10i64.pow(exponent as u32)) as $t
-                    }
+                if exponent != 0 {
+                    return Err(NumberOutOfScope);
                 }
+
+                let mantissa = if positive {
+                    mantissa as i64
+                } else {
+                    -(mantissa as i64)
+                };
+
+                TryFrom::try_from(mantissa).map_err(Into::into)
             }
         }
 
+        impl_integer!($t);
+    )*)
+}
+
+macro_rules! impl_integer {
+    ($t:ty) => {
         impl PartialEq<$t> for Number {
             fn eq(&self, other: &$t) -> bool {
                 *self == Number::from(*other)

src/parser.rs

@@ -769,7 +769,6 @@ mod tests {
     use crate::stringify;
     use crate::JsonValue;
 
-    #[macro_use]
     use crate::object;
     use crate::array;
 
@@ -840,7 +839,7 @@ mod tests {
     fn it_should_parse_json_nested_object() {
         let s = "{\"a\":1,\"b\":{\"c\":2,\"d\":{\"e\":{\"f\":{\"g\":3,\"h\":[]}}},\"i\":4,\"j\":[],\"k\":{\"l\":5,\"m\":{}}}}";
         let actual = parse(s).unwrap();
-        let mut expected = object! {
+        let expected = object! {
             "a" => 1,
             "b" => object!{
                 "c" => 2,
@@ -868,7 +867,7 @@ mod tests {
     fn it_should_parse_json_complex_object() {
         let s = "{\"a\":1,\"b\":{\"c\":2,\"d\":{\"e\":{\"f\":{\"g\":3,\"h\":[{\"z\":1},{\"y\":2,\"x\":[{},{}]}]}}},\"i\":4,\"j\":[],\"k\":{\"l\":5,\"m\":{}}}}";
         let actual = parse(s).unwrap();
-        let mut expected = object! {
+        let expected = object! {
             "a" => 1,
             "b" => object!{
                 "c" => 2,

src/util/print_dec.rs

@@ -59,9 +59,10 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
         return wr.write_all(b"0");
     }
 
-    let mut buf: [u8; 30] = mem::uninitialized();
-    let mut curr = buf.len() as isize;
-    let buf_ptr = buf.as_mut_ptr();
+    const BUF_LEN: usize = 30;
+    let mut buf = mem::MaybeUninit::<[u8; BUF_LEN]>::uninit();
+    let mut curr = BUF_LEN as isize;
+    let buf_ptr = buf.as_mut_ptr() as *mut u8;
     let lut_ptr = DEC_DIGITS_LUT.as_ptr();
 
     if exponent == 0 {
@@ -70,7 +71,7 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
         return wr.write_all(
             slice::from_raw_parts(
                 buf_ptr.offset(curr),
-                buf.len() - curr as usize
+                BUF_LEN - curr as usize
             )
         );
     } else if exponent < 0 {
@@ -112,7 +113,7 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
             write_num(&mut n, &mut curr, buf_ptr, lut_ptr);
 
             return wr.write_all(
-                slice::from_raw_parts(buf_ptr.offset(curr), buf.len() - curr as usize)
+                slice::from_raw_parts(buf_ptr.offset(curr), BUF_LEN - curr as usize)
             );
         }
 
@@ -159,7 +160,7 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
                 ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2);
             }
 
-            let printed_so_far = buf.len() as u16 - curr as u16;
+            let printed_so_far = BUF_LEN as u16 - curr as u16;
 
 
             if printed_so_far <= e {
@@ -183,7 +184,7 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
         wr.write_all(
             slice::from_raw_parts(
                 buf_ptr.offset(curr),
-                buf.len() - curr as usize
+                BUF_LEN - curr as usize
             )
         )?;
 
@@ -203,14 +204,14 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
 
     // Exponent greater than 0
     write_num(&mut n, &mut curr, buf_ptr, lut_ptr);
-    let printed = buf.len() - curr as usize;
+    let printed = BUF_LEN - curr as usize;
 
     // No need for `e` notation, just print out zeroes
     if (printed + exponent as usize) <= 20 {
         wr.write_all(
             slice::from_raw_parts(
                 buf_ptr.offset(curr),
-                buf.len() - curr as usize
+                BUF_LEN - curr as usize
             )
         )?;
 
@@ -230,7 +231,7 @@ pub unsafe fn write<W: io::Write>(wr: &mut W, positive: bool, mut n: u64, expone
     wr.write_all(
         slice::from_raw_parts(
             buf_ptr.offset(curr),
-            buf.len() - curr as usize
+            BUF_LEN - curr as usize
         )
     )?;
     wr.write_all(b"e")?;

src/value/mod.rs

@@ -1,4 +1,5 @@
 use std::ops::{Index, IndexMut, Deref};
+use std::convert::TryInto;
 use std::{fmt, mem, usize, u8, u16, u32, u64, isize, i8, i16, i32, i64, f32};
 use std::io::{self, Write};
 
@@ -229,11 +230,7 @@ impl JsonValue {
 
     pub fn as_u64(&self) -> Option<u64> {
         self.as_number().and_then(|value| {
-            if value.is_sign_positive() {
-                Some(value.into())
-            } else {
-                None
-            }
+            value.try_into().ok()
         })
     }
 
@@ -254,7 +251,7 @@ impl JsonValue {
     }
 
     pub fn as_i64(&self) -> Option<i64> {
-        self.as_number().map(|value| value.into())
+        self.as_number().and_then(|value| value.try_into().ok())
     }
 
     pub fn as_i32(&self) -> Option<i32> {

tests/customgen.rs

@@ -2,7 +2,7 @@
 extern crate json;
 
 use json::codegen::Generator;
-use json::object::{self, Object};
+use json::object::Object;
 use json::{JsonValue, Null};
 use std::io;