Check for correctness of feature argument number when parsing feature arguments

src/cachedev.rs

@@ -97,13 +97,32 @@ impl FromStr for CacheTargetParams {
         let origin_dev = parse_device(vals[3], "origin sub-device for cache target")?;
 
         let block_size = Sectors(parse_value(vals[4], "data block size")?);
-        let num_feature_args: usize = parse_value(vals[5], "number of feature args")?;
+
+        let num_feature_args = if vals.len() == 5 {
+            0
+        } else {
+            parse_value::<usize>(vals[5], "number of feature args")?
+        };
 
         let end_feature_args_index = 6 + num_feature_args;
-        let feature_args: Vec<String> = vals[6..end_feature_args_index]
-            .iter()
-            .map(|x| (*x).to_string())
-            .collect();
+
+        if vals.len() < end_feature_args_index + 2 {
+            let err_msg = format!(
+                "Expected {} feature arguments but found {}",
+                vals[5],
+                vals.len() - 6
+            );
+            return Err(DmError::Dm(ErrorEnum::Invalid, err_msg));
+        }
+
+        let feature_args: Vec<String> = if num_feature_args == 0 {
+            vec![]
+        } else {
+            vals[6..end_feature_args_index]
+                .iter()
+                .map(|x| (*x).to_string())
+                .collect()
+        };
 
         let policy = vals[end_feature_args_index].to_owned();
 
@@ -1059,4 +1078,51 @@ mod tests {
     fn loop_test_suspend() {
         test_with_spec(2, test_suspend);
     }
+
+    #[test]
+    fn test_cache_target_params_zero() {
+        let result = "cache 42:42 42:43 42:44 16 0 default 0"
+            .parse::<CacheTargetParams>()
+            .unwrap();
+        assert_eq!(result.feature_args, HashSet::new());
+    }
+
+    #[test]
+    fn test_cache_target_params_correct_feature_args() {
+        let result = "cache 42:42 42:43 42:44 16 2 writethrough passthrough default 0"
+            .parse::<CacheTargetParams>()
+            .unwrap();
+        let expected = vec!["writethrough".to_owned(), "passthrough".to_owned()]
+            .iter()
+            .cloned()
+            .collect::<HashSet<_>>();
+        assert_eq!(result.feature_args, expected);
+    }
+
+    #[test]
+    fn test_cache_target_params_missing_1_feature_arg() {
+        let result = "cache 42:42 42:43 42:44 16 3 writethrough passthrough default 0"
+            .parse::<CacheTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
+
+    #[test]
+    fn test_cache_target_params_missing_2_feature_args() {
+        let result = "cache 42:42 42:43 42:44 16 4 writethrough passthrough default 0"
+            .parse::<CacheTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
+
+    #[test]
+    fn test_cache_target_params_missing_3_feature_args() {
+        let result =
+            "cache 42:42 42:43 42:44 16 4 writethrough default 0".parse::<CacheTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
+
+    #[test]
+    fn test_cache_target_params_less_than_8_values() {
+        let result = "cache 42:42 42:43 42:44 16 1 writethrough".parse::<CacheTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
 }

src/lineardev.rs

@@ -296,12 +296,23 @@ impl FromStr for FlakeyTargetParams {
         let up_interval = parse_value(vals[3], "up interval")?;
         let down_interval = parse_value(vals[4], "down interval")?;
 
-        let feature_args = if vals.len() == 5 {
+        let num_feature_args = if vals.len() == 5 {
+            0
+        } else {
+            parse_value::<usize>(vals[5], "number of feature args")?
+        };
+
+        let feature_args = if num_feature_args == 0 {
             vec![]
+        } else if vals.as_slice().get(5 + num_feature_args).is_some() {
+            parse_feature_args(&vals[6..6 + num_feature_args])?
         } else {
-            parse_feature_args(
-                &vals[6..6 + parse_value::<usize>(vals[5], "number of feature args")?],
-            )?
+            let err_msg = format!(
+                "Expected {} feature arguments but found {}",
+                vals[5],
+                vals.len() - 6
+            );
+            return Err(DmError::Dm(ErrorEnum::Invalid, err_msg));
         };
 
         Ok(FlakeyTargetParams::new(
@@ -918,6 +929,24 @@ mod tests {
         assert_eq!(result.feature_args, expected);
     }
 
+    #[test]
+    fn test_flakey_correct_feature_args_input() {
+        let result = "flakey 8:32 0 16 2 2 error_writes drop_writes"
+            .parse::<FlakeyTargetParams>()
+            .unwrap();
+        let expected = [FeatureArg::ErrorWrites, FeatureArg::DropWrites]
+            .iter()
+            .cloned()
+            .collect::<HashSet<_>>();
+        assert_eq!(result.feature_args, expected);
+    }
+
+    #[test]
+    fn test_flakey_incorrect_feature_args_input() {
+        let result = "flakey 8:32 0 16 2 3 error_writes drop_writes".parse::<FlakeyTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
+
     #[test]
     fn loop_test_duplicate_segments() {
         test_with_spec(1, test_duplicate_segments);

src/thinpooldev.rs

@@ -86,13 +86,26 @@ impl FromStr for ThinPoolTargetParams {
         let data_block_size = Sectors(parse_value(vals[3], "data block size")?);
         let low_water_mark = DataBlocks(parse_value(vals[4], "low water mark")?);
 
-        let feature_args = if vals.len() == 5 {
-            vec![]
+        let num_feature_args = if vals.len() == 5 {
+            0
         } else {
-            vals[6..6 + parse_value::<usize>(vals[5], "number of feature args")?]
+            parse_value::<usize>(vals[5], "number of feature args")?
+        };
+
+        let feature_args = if num_feature_args == 0 {
+            vec![]
+        } else if vals.as_slice().get(5 + num_feature_args).is_some() {
+            vals[6..6 + num_feature_args]
                 .iter()
                 .map(|x| (*x).to_string())
                 .collect()
+        } else {
+            let err_msg = format!(
+                "Expected {} feature arguments but found {}",
+                vals[5],
+                vals.len() - 6
+            );
+            return Err(DmError::Dm(ErrorEnum::Invalid, err_msg));
         };
 
         Ok(ThinPoolTargetParams::new(
@@ -887,4 +900,26 @@ mod tests {
             .unwrap();
         assert_eq!(result.feature_args, HashSet::new());
     }
+
+    #[test]
+    fn test_thinpool_target_params_correct_feature_args() {
+        let result = "thin-pool 42:42 42:43 16 2 2 error_if_no_space skip_block_zeroing"
+            .parse::<ThinPoolTargetParams>()
+            .unwrap();
+        let expected = vec![
+            "error_if_no_space".to_owned(),
+            "skip_block_zeroing".to_owned(),
+        ]
+        .iter()
+        .cloned()
+        .collect::<HashSet<_>>();
+        assert_eq!(result.feature_args, expected);
+    }
+
+    #[test]
+    fn test_thinpool_target_params_incorrect_feature_args() {
+        let result = "thin-pool 42:42 42:43 16 2 3 error_if_no_space skip_block_zeroing"
+            .parse::<ThinPoolTargetParams>();
+        assert_matches!(result, Err(DmError::Dm(ErrorEnum::Invalid, _)));
+    }
 }