diff --git a/CHANGELOG.md b/CHANGELOG.md index 6760222fdcdc..4183e616f753 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,13 @@ **Fixes**: +- Deserializing an ident with an empty path now reports an error rather than + panicking. A document containing `{"Ident":[]}` passed to `json::to_pl` or + `json::to_rq` — reachable from the Python and JS bindings — hit + `path.pop().unwrap()`; it now reports + `invalid length 0, expected an ident with at least one part`. (@prql-bot, + #6223) + **Documentation**: **Web**: diff --git a/prqlc/prqlc-parser/src/parser/pr/ident.rs b/prqlc/prqlc-parser/src/parser/pr/ident.rs index 781bf4ccdddc..caddecbd544f 100644 --- a/prqlc/prqlc-parser/src/parser/pr/ident.rs +++ b/prqlc/prqlc-parser/src/parser/pr/ident.rs @@ -21,13 +21,19 @@ impl Ident { /// Creates a new ident from a non-empty path. /// - /// Panics if path is empty. - pub fn from_path(mut path: Vec) -> Self { - let name = path.pop().unwrap().to_string(); - Ident { + /// Panics if path is empty; use [Ident::try_from_path] for paths that + /// originate outside the compiler, such as deserialized input. + pub fn from_path(path: Vec) -> Self { + Ident::try_from_path(path).expect("ident path is empty") + } + + /// Creates a new ident from a path, returning `None` if the path is empty. + pub fn try_from_path(mut path: Vec) -> Option { + let name = path.pop()?.to_string(); + Some(Ident { path: path.into_iter().map(|x| x.to_string()).collect(), name, - } + }) } pub fn len(&self) -> usize { @@ -155,7 +161,9 @@ impl<'de> Deserialize<'de> for Ident { where D: Deserializer<'de>, { - as Deserialize>::deserialize(deserializer).map(Ident::from_path) + let path = as Deserialize>::deserialize(deserializer)?; + Ident::try_from_path(path) + .ok_or_else(|| serde::de::Error::invalid_length(0, &"an ident with at least one part")) } } @@ -187,3 +195,24 @@ pub fn display_ident_part(f: &mut std::fmt::Formatter, s: &str) -> Result<(), st write!(f, "{s}") } } + +#[cfg(test)] +mod test { + use insta::assert_snapshot; + + use super::Ident; + + #[test] + fn deserialize_empty_path() { + // An empty path used to panic in `from_path`, reachable from the public + // `json::to_pl` / `json::to_rq` entry points. + let err = serde_json::from_str::("[]").unwrap_err(); + assert_snapshot!(err, @"invalid length 0, expected an ident with at least one part"); + } + + #[test] + fn deserialize_path() { + let ident: Ident = serde_json::from_str(r#"["a","b","c"]"#).unwrap(); + assert_snapshot!(ident, @"a.b.c"); + } +}