From 47fae673cfa7762682d71417365ac1a2d940b70b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Eriksson?= Date: Mon, 28 Oct 2024 11:17:26 +0100 Subject: [PATCH] runtimes/core: fix validation of nested objects in unions (#1529) --- Cargo.lock | 5 +- runtimes/core/Cargo.toml | 1 + runtimes/core/src/api/error.rs | 4 +- runtimes/core/src/api/jsonschema/de.rs | 140 ++++++++++++++++++++++-- runtimes/core/src/api/jsonschema/mod.rs | 13 ++- 5 files changed, 145 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfc1d48d40..bbd5bd348e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1450,6 +1450,7 @@ dependencies = [ "rsa", "serde", "serde_json", + "serde_path_to_error", "serde_urlencoded", "serde_with", "sha2", @@ -4533,9 +4534,9 @@ dependencies = [ [[package]] name = "serde_path_to_error" -version = "0.1.14" +version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4beec8bce849d58d06238cb50db2e1c417cfeafa4c63f692b15c82b7c80f8335" +checksum = "af99884400da37c88f5e9146b7f1fd0fbcae8f6eec4e9da38b67d05486f814a6" dependencies = [ "itoa", "serde", diff --git a/runtimes/core/Cargo.toml b/runtimes/core/Cargo.toml index a4ce0a5a2c..bb7edb933a 100644 --- a/runtimes/core/Cargo.toml +++ b/runtimes/core/Cargo.toml @@ -79,6 +79,7 @@ rsa = { version = "0.9.6", features = ["pem"] } flate2 = "1.0.30" urlencoding = "2.1.3" tower-http = { version = "0.5.2", features = ["fs"] } +serde_path_to_error = "0.1.16" [build-dependencies] prost-build = "0.12.3" diff --git a/runtimes/core/src/api/error.rs b/runtimes/core/src/api/error.rs index 08e8753bea..f1766f5e52 100644 --- a/runtimes/core/src/api/error.rs +++ b/runtimes/core/src/api/error.rs @@ -74,8 +74,8 @@ impl Error { { Self { code: ErrCode::InvalidArgument, - message: public_msg.into(), - internal_message: Some(format!("{:?}", cause.into())), + message: format!("{}: {:?}", public_msg.into(), cause.into()), + internal_message: None, stack: None, details: None, } diff --git a/runtimes/core/src/api/jsonschema/de.rs b/runtimes/core/src/api/jsonschema/de.rs index 89312fa996..3e425e20c9 100644 --- a/runtimes/core/src/api/jsonschema/de.rs +++ b/runtimes/core/src/api/jsonschema/de.rs @@ -159,6 +159,15 @@ impl Literal { Literal::Float(lit) => format!("{:#?}", lit), } } + + pub fn expecting_type(&self) -> &'static str { + match self { + Literal::Str(_) => "string", + Literal::Bool(_) => "boolean", + Literal::Int(_) => "integer", + Literal::Float(_) => "number", + } + } } #[derive(Debug, Copy, Clone)] @@ -1159,20 +1168,127 @@ impl<'a> DecodeValue<'a> { JVal::Null => PValue::Null, JVal::Bool(val) => PValue::Bool(val), JVal::Number(num) => PValue::Number(num), - JVal::Array(vals) => { - let mut new_vals = Vec::with_capacity(vals.len()); - for val in vals { - new_vals.push(self.transform(val)?); + JVal::Array(vals) => match self.value { + Value::Ref(idx) => return recurse_ref!(self, idx, transform, JVal::Array(vals)), + Value::Option(bov) => return recurse!(self, bov, transform, JVal::Array(vals)), + Value::Basic(Basic::Any) => { + let mut new_vals = Vec::with_capacity(vals.len()); + for val in vals { + new_vals.push(self.transform(val)?); + } + PValue::Array(new_vals) } - PValue::Array(new_vals) - } - JVal::Object(obj) => { - let mut new_obj = BTreeMap::new(); - for (key, val) in obj { - new_obj.insert(key, self.transform(val)?); + Value::Array(bov) => { + let mut new_vals = Vec::with_capacity(vals.len()); + for val in vals { + let val = recurse!(self, bov, transform, val)?; + new_vals.push(val); + } + PValue::Array(new_vals) } - PValue::Object(new_obj) - } + Value::Union(candidates) => { + // First transform with the first candidate. + // If it fails validation afterwards, we need to start over. + 'CandidateLoop: for c in candidates { + let mut new_vals = Vec::with_capacity(vals.len()); + let vals = vals.clone(); + for val in vals { + let res: Result<_, E> = recurse!(self, c, transform, val); + match res { + Ok(val) => new_vals.push(val), + Err(_) => continue 'CandidateLoop, + } + } + return Ok(PValue::Array(new_vals)); + } + + return Err(serde::de::Error::invalid_type(Unexpected::Seq, self)); + } + Value::Basic(basic) => { + return Err(serde::de::Error::invalid_type( + Unexpected::Other(basic.expecting()), + self, + )) + } + Value::Literal(lit) => { + return Err(serde::de::Error::invalid_type( + Unexpected::Other(lit.expecting_type()), + self, + )) + } + Value::Map(_) | Value::Struct(_) => { + return Err(serde::de::Error::invalid_type(Unexpected::Map, self)) + } + }, + JVal::Object(obj) => match self.value { + Value::Ref(idx) => return recurse_ref!(self, idx, transform, JVal::Object(obj)), + Value::Option(bov) => return recurse!(self, bov, transform, JVal::Object(obj)), + Value::Basic(Basic::Any) => { + let mut new_obj = BTreeMap::new(); + for (key, val) in obj { + new_obj.insert(key, self.transform(val)?); + } + PValue::Object(new_obj) + } + Value::Map(bov) => { + let mut new_obj = BTreeMap::new(); + for (key, val) in obj { + let val = recurse!(self, bov, transform, val)?; + new_obj.insert(key, val); + } + PValue::Object(new_obj) + } + Value::Struct(Struct { fields }) => { + let mut new_obj = BTreeMap::new(); + for (key, value) in obj { + match fields.get(key.as_str()) { + Some(entry) => { + let val = recurse!(self, &entry.value, transform, value)?; + new_obj.insert(key, val); + } + None => { + // Unknown field; ignore it. + } + } + } + PValue::Object(new_obj) + } + Value::Union(candidates) => { + // First transform with the first candidate. + // If it fails validation afterwards, we need to start over. + 'CandidateLoop: for c in candidates { + let mut new_obj = BTreeMap::new(); + let obj = obj.clone(); + for (key, val) in obj { + let res: Result<_, E> = recurse!(self, c, transform, val); + match res { + Ok(val) => { + new_obj.insert(key, val); + } + Err(_) => continue 'CandidateLoop, + } + } + return Ok(PValue::Object(new_obj)); + } + + return Err(serde::de::Error::invalid_type(Unexpected::Map, self)); + } + Value::Basic(basic) => { + return Err(serde::de::Error::invalid_type( + Unexpected::Other(basic.expecting()), + self, + )) + } + Value::Literal(lit) => { + return Err(serde::de::Error::invalid_type( + Unexpected::Other(lit.expecting_type()), + self, + )) + } + Value::Array(_) => { + return Err(serde::de::Error::invalid_type(Unexpected::Seq, self)) + } + }, JVal::String(str) => match self.value { Value::Ref(idx) => return recurse_ref!(self, idx, transform, JVal::String(str)), Value::Option(bov) => return recurse!(self, bov, transform, JVal::String(str)), diff --git a/runtimes/core/src/api/jsonschema/mod.rs b/runtimes/core/src/api/jsonschema/mod.rs index 67ebba4f65..8c3e7e94ca 100644 --- a/runtimes/core/src/api/jsonschema/mod.rs +++ b/runtimes/core/src/api/jsonschema/mod.rs @@ -76,12 +76,21 @@ impl JSONSchema { SchemaDeserializer { cfg, schema: self } } - pub fn deserialize<'de, T>(&self, de: T, cfg: DecodeConfig) -> Result + pub fn deserialize<'de, T>( + &self, + de: T, + cfg: DecodeConfig, + ) -> Result> where T: Deserializer<'de>, { let seed = SchemaDeserializer { cfg, schema: self }; - seed.deserialize(de) + let mut track = serde_path_to_error::Track::new(); + let de = serde_path_to_error::Deserializer::new(de, &mut track); + match seed.deserialize(de) { + Ok(t) => Ok(t), + Err(err) => Err(serde_path_to_error::Error::new(track.path(), err)), + } } pub fn null() -> Self {