diff --git a/src/error.rs b/src/error.rs index 13e13b82..2d738b94 100644 --- a/src/error.rs +++ b/src/error.rs @@ -174,7 +174,7 @@ impl ConfigError { } /// Alias for a `Result` with the error type set to `ConfigError`. -pub(crate) type Result = result::Result; +pub(crate) type Result = result::Result; // Forward Debug to Display for readable panic! messages impl fmt::Debug for ConfigError { diff --git a/src/path/mod.rs b/src/path/mod.rs index 3c0b4213..e98e92a3 100644 --- a/src/path/mod.rs +++ b/src/path/mod.rs @@ -40,11 +40,14 @@ impl std::fmt::Display for ParseError { impl std::error::Error for ParseError {} -fn sindex_to_uindex(index: isize, len: usize) -> usize { +/// Convert a relative index into an absolute index +fn abs_index(index: isize, len: usize) -> Result { if index >= 0 { - index as usize + Ok(index as usize) + } else if let Some(index) = len.checked_sub(index.unsigned_abs()) { + Ok(index) } else { - len - index.unsigned_abs() + Err((len as isize + index).unsigned_abs()) } } @@ -80,13 +83,8 @@ impl Expression { Self::Subscript(expr, index) => match expr.get(root) { Some(value) => match value.kind { ValueKind::Array(ref array) => { - let index = sindex_to_uindex(index, array.len()); - - if index >= array.len() { - None - } else { - Some(&array[index]) - } + let index = abs_index(index, array.len()).ok()?; + array.get(index) } _ => None, @@ -141,7 +139,7 @@ impl Expression { match value.kind { ValueKind::Array(ref mut array) => { - let index = sindex_to_uindex(index, array.len()); + let index = abs_index(index, array.len()).ok()?; if index >= array.len() { array.resize(index + 1, Value::new(None, ValueKind::Nil)); @@ -216,10 +214,21 @@ impl Expression { } if let ValueKind::Array(ref mut array) = parent.kind { - let uindex = sindex_to_uindex(index, array.len()); - if uindex >= array.len() { - array.resize(uindex + 1, Value::new(None, ValueKind::Nil)); - } + let uindex = match abs_index(index, array.len()) { + Ok(uindex) => { + if uindex >= array.len() { + array.resize(uindex + 1, Value::new(None, ValueKind::Nil)); + } + uindex + } + Err(insertion) => { + array.splice( + 0..0, + (0..insertion).map(|_| Value::new(None, ValueKind::Nil)), + ); + 0 + } + }; array[uindex] = value; } diff --git a/tests/testsuite/errors.rs b/tests/testsuite/errors.rs index 983a3c61..74edd41a 100644 --- a/tests/testsuite/errors.rs +++ b/tests/testsuite/errors.rs @@ -3,6 +3,52 @@ use snapbox::{assert_data_eq, str}; use config::{Config, ConfigError, File, FileFormat, Map, Value}; +#[test] +#[cfg(feature = "json")] +fn test_error_path_index_bounds() { + let c = Config::builder() + .add_source(File::from_str( + r#" +{ + "arr": [1] +} +"#, + FileFormat::Json, + )) + .build() + .unwrap(); + + let res = c.get::("arr[2]"); + assert!(res.is_err()); + assert_data_eq!( + res.unwrap_err().to_string(), + str![[r#"configuration property "arr[2]" not found"#]] + ); +} + +#[test] +#[cfg(feature = "json")] +fn test_error_path_index_negative_bounds() { + let c = Config::builder() + .add_source(File::from_str( + r#" +{ + "arr": [] +} +"#, + FileFormat::Json, + )) + .build() + .unwrap(); + + let res = c.get::("arr[-1]"); + assert!(res.is_err()); + assert_data_eq!( + res.unwrap_err().to_string(), + str![[r#"configuration property "arr[-1]" not found"#]] + ); +} + #[test] #[cfg(feature = "json")] fn test_error_parse() { diff --git a/tests/testsuite/set.rs b/tests/testsuite/set.rs index 4a02b5c0..c5d29de0 100644 --- a/tests/testsuite/set.rs +++ b/tests/testsuite/set.rs @@ -65,27 +65,44 @@ fn test_set_scalar_path() { #[cfg(feature = "json")] fn test_set_arr_path() { let config = Config::builder() - .set_override("items[0].name", "Ivan") + .set_override("present[0].name", "Ivan") .unwrap() - .set_override("data[0].things[1].name", "foo") + .set_override("absent[0].things[1].name", "foo") .unwrap() - .set_override("data[0].things[1].value", 42) + .set_override("absent[0].things[1].value", 42) .unwrap() - .set_override("data[1]", 0) + .set_override("absent[1]", 0) .unwrap() - .set_override("items[2]", "George") + .set_override("present[2]", "George") + .unwrap() + .set_override("reverse[-1]", "Bob") + .unwrap() + .set_override("reverse[-2]", "Alice") + .unwrap() + .set_override("empty[-1]", "Bob") + .unwrap() + .set_override("empty[-2]", "Alice") .unwrap() .add_source(File::from_str( r#" { - "items": [ + "present": [ { "name": "1" }, { "name": "2" } - ] + ], + "reverse": [ + { + "name": "l1" + }, + { + "name": "l2" + } + ], + "empty": [] } "#, FileFormat::Json, @@ -93,14 +110,18 @@ fn test_set_arr_path() { .build() .unwrap(); - assert_eq!(config.get("items[0].name").ok(), Some("Ivan".to_owned())); + assert_eq!(config.get("present[0].name").ok(), Some("Ivan".to_owned())); assert_eq!( - config.get("data[0].things[1].name").ok(), + config.get("absent[0].things[1].name").ok(), Some("foo".to_owned()) ); - assert_eq!(config.get("data[0].things[1].value").ok(), Some(42)); - assert_eq!(config.get("data[1]").ok(), Some(0)); - assert_eq!(config.get("items[2]").ok(), Some("George".to_owned())); + assert_eq!(config.get("absent[0].things[1].value").ok(), Some(42)); + assert_eq!(config.get("absent[1]").ok(), Some(0)); + assert_eq!(config.get("present[2]").ok(), Some("George".to_owned())); + assert_eq!(config.get("reverse[1]").ok(), Some("Bob".to_owned())); + assert_eq!(config.get("reverse[0]").ok(), Some("Alice".to_owned())); + assert_eq!(config.get("empty[1]").ok(), Some("Bob".to_owned())); + assert_eq!(config.get("empty[0]").ok(), Some("Alice".to_owned())); } #[test]