From 9b29ac98a59c5b8bc82a4a15177ad75053fbb067 Mon Sep 17 00:00:00 2001
From: Michael Debertol <michael.debertol@gmail.com>
Date: Tue, 1 Jun 2021 18:12:37 +0200
Subject: [PATCH] seq: reject NaN arguments

Move the validation logic to an argument validator.
---
 src/uu/seq/src/seq.rs     | 68 ++++++++++++++-------------------------
 tests/by-util/test_seq.rs | 16 +++++++++
 2 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs
index bdab044c5..60aa7e654 100644
--- a/src/uu/seq/src/seq.rs
+++ b/src/uu/seq/src/seq.rs
@@ -13,7 +13,6 @@ use num_traits::Zero;
 use num_traits::{Num, ToPrimitive};
 use std::cmp;
 use std::io::{stdout, Write};
-use std::str::FromStr;
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static ABOUT: &str = "Display numbers from FIRST to LAST, in steps of INCREMENT.";
@@ -44,6 +43,18 @@ enum Number {
 }
 
 impl Number {
+    fn new(mut s: &str) -> Self {
+        if s.starts_with('+') {
+            s = &s[1..];
+        }
+
+        match s.parse::<BigInt>() {
+            Ok(n) => Number::BigInt(n),
+            // The argument validator made sure this is a valid float.
+            Err(_) => Number::F64(s.parse::<f64>().unwrap()),
+        }
+    }
+
     fn is_zero(&self) -> bool {
         match self {
             Number::BigInt(n) => n.is_zero(),
@@ -60,27 +71,6 @@ impl Number {
     }
 }
 
-impl FromStr for Number {
-    type Err = String;
-    /// Tries to parse this string as a BigInt, or if that fails as an f64.
-    fn from_str(mut s: &str) -> Result<Self, Self::Err> {
-        if s.starts_with('+') {
-            s = &s[1..];
-        }
-
-        match s.parse::<BigInt>() {
-            Ok(n) => Ok(Number::BigInt(n)),
-            Err(_) => match s.parse::<f64>() {
-                Ok(n) => Ok(Number::F64(n)),
-                Err(e) => Err(format!(
-                    "seq: invalid floating point argument `{}`: {}",
-                    s, e
-                )),
-            },
-        }
-    }
-}
-
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let usage = get_usage();
     let matches = App::new(executable!())
@@ -116,7 +106,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .takes_value(true)
                 .allow_hyphen_values(true)
                 .max_values(3)
-                .required(true),
+                .required(true)
+                .validator(|value| {
+                    match value.parse::<f64>() {
+                        Ok(value) if value.is_nan() => Err("can not be NaN".to_string()),
+                        Ok(_) => Ok(()),
+                        Err(e) => Err(e.to_string()),
+                    }
+                    .map_err(|e| format!("invalid floating point argument `{}`: {}", value, e))
+                }),
         )
         .get_matches_from(args);
 
@@ -136,13 +134,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = len - dec;
         padding = dec;
-        match slice.parse() {
-            Ok(n) => n,
-            Err(s) => {
-                show_error!("{}", s);
-                return 1;
-            }
-        }
+        Number::new(slice)
     } else {
         Number::BigInt(BigInt::one())
     };
@@ -152,13 +144,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = cmp::max(largest_dec, len - dec);
         padding = cmp::max(padding, dec);
-        match slice.parse() {
-            Ok(n) => n,
-            Err(s) => {
-                show_error!("{}", s);
-                return 1;
-            }
-        }
+        Number::new(slice)
     } else {
         Number::BigInt(BigInt::one())
     };
@@ -169,13 +155,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let last = {
         let slice = numbers[numbers.len() - 1];
         padding = cmp::max(padding, slice.find('.').unwrap_or_else(|| slice.len()));
-        match slice.parse::<Number>() {
-            Ok(n) => n,
-            Err(s) => {
-                show_error!("{}", s);
-                return 1;
-            }
-        }
+        Number::new(slice)
     };
     if largest_dec > 0 {
         largest_dec -= 1;
diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs
index 98eb23598..7545ebde7 100644
--- a/tests/by-util/test_seq.rs
+++ b/tests/by-util/test_seq.rs
@@ -1,5 +1,21 @@
 use crate::common::util::*;
 
+#[test]
+fn test_rejects_nan() {
+    new_ucmd!()
+        .args(&["NaN"])
+        .fails()
+        .stderr_only("error: Invalid value for '<numbers>...': invalid floating point argument `NaN`: can not be NaN");
+}
+
+#[test]
+fn test_rejects_non_floats() {
+    new_ucmd!()
+        .args(&["foo"])
+        .fails()
+        .stderr_only("error: Invalid value for '<numbers>...': invalid floating point argument `foo`: invalid float literal");
+}
+
 // ---- Tests for the big integer based path ----
 
 #[test]