diff --git a/crates/ironposh-client-tokio/tests/e2e/command_matrix.rs b/crates/ironposh-client-tokio/tests/e2e/command_matrix.rs index c080f8f..c4a37d0 100644 --- a/crates/ironposh-client-tokio/tests/e2e/command_matrix.rs +++ b/crates/ironposh-client-tokio/tests/e2e/command_matrix.rs @@ -100,6 +100,23 @@ fn real_server_command_matrix_all_auths() { "C3 retry marker missing for auth={auth}. stdout={stdout} stderr={stderr}" ); } + + // C4: a non-existent command. PowerShell reports CommandNotFound as an + // error record (not a process-fatal failure), so the client prints it via + // `render_concise` to stdout and still exits 0. The message echoes the + // command name, which is what we assert on (locale-independent). + { + let bogus = "Get-IronPoshDefinitelyNotARealCommand"; + let (ok, stdout, stderr) = run_noninteractive(auth, bogus); + assert!( + ok, + "C4 process should exit 0 (error record is non-fatal) for auth={auth}. stdout={stdout} stderr={stderr}" + ); + assert!( + stdout.contains(bogus), + "C4 command-not-found error missing the command name for auth={auth}. stdout={stdout} stderr={stderr}" + ); + } } let _ = Instant::now(); // keep `Instant` import used without adding behavior } diff --git a/crates/ironposh-macros/src/lib.rs b/crates/ironposh-macros/src/lib.rs index d58f532..2d6a8aa 100644 --- a/crates/ironposh-macros/src/lib.rs +++ b/crates/ironposh-macros/src/lib.rs @@ -1364,6 +1364,7 @@ fn impl_from_xml(input: &DeriveInput) -> Result { node: ironposh_xml::parser::Node<'a, 'a>, ) -> Result { use ironposh_xml::mapping::NodeExt; + ironposh_xml::mapping::reject_mixed_content(node)?; #(#inits)* for child in node.children() { if !child.is_element() { diff --git a/crates/ironposh-winrm/Cargo.toml b/crates/ironposh-winrm/Cargo.toml index a28fbc8..f61e54b 100644 --- a/crates/ironposh-winrm/Cargo.toml +++ b/crates/ironposh-winrm/Cargo.toml @@ -13,7 +13,7 @@ ironposh-xml = { path = "../ironposh-xml" } ironposh-macros = { path = "../ironposh-macros" } byteorder = "1.5.0" base64 = "0.22.1" -paste = "1.0.15" +paste = "1.0" uuid = { version = "1.0", features = ["v4"] } [dev-dependencies] diff --git a/crates/ironposh-winrm/src/cores/attribute.rs b/crates/ironposh-winrm/src/cores/attribute.rs index 34fec05..2367fd0 100644 --- a/crates/ironposh-winrm/src/cores/attribute.rs +++ b/crates/ironposh-winrm/src/cores/attribute.rs @@ -19,20 +19,33 @@ macro_rules! define_attributes { impl<'a> Attribute<'a> { /// Convert an attribute name to the corresponding enum variant type /// This is automatically generated to match all enum variants - pub fn from_name_and_value(name: &str, value: &'a str) -> Result, ironposh_xml::XmlError> { - match name { - $( - $attr_name => { - match $parser(value) { + pub fn from_name_and_value(namespace: Option<&str>, name: &str, value: &'a str) -> Result, ironposh_xml::XmlError> { + // The reserved `xml:` prefix is modeled literally (e.g. "xml:lang", + // no declared namespace); fold roxmltree's expanded + // (xml-namespace, local) form back to that spelling so it matches. + let xml_prefixed; + let (namespace, name) = if namespace == Some("http://www.w3.org/XML/1998/namespace") { + xml_prefixed = format!("xml:{name}"); + (None, xml_prefixed.as_str()) + } else { + (namespace, name) + }; + // Identity is the (namespace-URI, local-name) pair, like elements: + // a known attribute in the wrong namespace is not that attribute. + $( + { + let expected_ns: Option = $namespace; + if name == $attr_name && namespace == expected_ns.map(|ns| ns.uri()) { + return match $parser(value) { Ok(val) => Ok(Some(Attribute::$variant(val))), Err(e) => Err(ironposh_xml::XmlError::InvalidXml( format!("Invalid value for {}: {}", $attr_name, e) )), - } + }; } - )* - _ => Ok(None), // Unknown attribute, ignore - } + } + )* + Ok(None) // Unknown attribute, ignore } /// Get the attribute name for this enum variant @@ -82,16 +95,27 @@ macro_rules! define_attributes { }; } +/// XSD boolean: `true`/`false` and their `1`/`0` aliases (MS-WSMV/SOAP send both). +fn parse_xml_bool(v: &str) -> Result { + match v { + "true" | "1" => Ok(true), + "false" | "0" => Ok(false), + other => Err(format!( + "expected xs:boolean (true/false/1/0), got {other:?}" + )), + } +} + // Define all attributes here - adding a new one automatically updates ALL related code define_attributes!( MustUnderstand(bool) => (Some(crate::cores::namespace::Namespace::SoapEnvelope2003), "mustUnderstand"), - |v: &str| v.parse::().map_err(|e| e.to_string()), + parse_xml_bool, |v: bool| v.to_string(), Name(Cow<'a, str>) => (None, "Name"), |v: &str| -> Result, String> { Ok(Cow::Owned(v.to_string())) }, |v: Cow<'a, str>| v.into_owned(), MustComply(bool) => (None, "MustComply"), - |v: &str| v.parse::().map_err(|e| e.to_string()), + parse_xml_bool, |v: bool| v.to_string(), ShellId(Cow<'a, str>) => (None, "ShellId"), |v: &str| -> Result, String> { Ok(Cow::Owned(v.to_string())) }, @@ -114,13 +138,13 @@ define_attributes!( |v: &str| -> Result, String> { Ok(Cow::Owned(v.to_string())) }, |v: Cow<'a, str>| v.into_owned(), End(bool) => (None, "End"), - |v: &str| v.parse::().map_err(|e| e.to_string()), + parse_xml_bool, |v: bool| v.to_string(), Unit(Cow<'a, str>) => (None, "Unit"), |v: &str| -> Result, String> { Ok(Cow::Owned(v.to_string())) }, |v: Cow<'a, str>| v.into_owned(), EndUnit(bool) => (None, "EndUnit"), - |v: &str| v.parse::().map_err(|e| e.to_string()), + parse_xml_bool, |v: bool| v.to_string(), SequenceID(u64) => (None, "SequenceID"), |v: &str| v.parse::().map_err(|e| e.to_string()), @@ -151,13 +175,18 @@ mod tests { #[test] fn test_parsing_round_trip() { let test_cases = [ - ("mustUnderstand", "true"), - ("Name", "test-name"), - ("MustComply", "false"), + ( + Some(crate::cores::namespace::Namespace::SoapEnvelope2003.uri()), + "mustUnderstand", + "true", + ), + (None, "Name", "test-name"), + (None, "MustComply", "false"), ]; - for (attr_name, attr_value) in test_cases { - if let Some(parsed) = Attribute::from_name_and_value(attr_name, attr_value).unwrap() { + for (ns, attr_name, attr_value) in test_cases { + if let Some(parsed) = Attribute::from_name_and_value(ns, attr_name, attr_value).unwrap() + { // Test that we can get the name back assert_eq!(parsed.attribute_name(), attr_name); @@ -166,4 +195,47 @@ mod tests { } } } + + #[test] + fn known_attribute_in_wrong_namespace_is_unmatched() { + // `mustUnderstand` lives in the SOAP namespace; unqualified it is unknown. + assert!( + Attribute::from_name_and_value(None, "mustUnderstand", "true") + .unwrap() + .is_none() + ); + } + + #[test] + fn known_attribute_with_bad_value_propagates_error() { + assert!(Attribute::from_name_and_value(None, "MustComply", "notabool").is_err()); + } + + #[test] + fn xsd_boolean_accepts_one_and_zero() { + assert!(matches!( + Attribute::from_name_and_value(None, "MustComply", "1").unwrap(), + Some(Attribute::MustComply(true)) + )); + assert!(matches!( + Attribute::from_name_and_value( + Some(crate::cores::namespace::Namespace::SoapEnvelope2003.uri()), + "mustUnderstand", + "0" + ) + .unwrap(), + Some(Attribute::MustUnderstand(false)) + )); + } + + #[test] + fn xml_lang_matches_via_expanded_namespace() { + let parsed = Attribute::from_name_and_value( + Some("http://www.w3.org/XML/1998/namespace"), + "lang", + "en-US", + ) + .unwrap(); + assert!(matches!(parsed, Some(Attribute::XmlLang(_)))); + } } diff --git a/crates/ironposh-winrm/src/cores/tag.rs b/crates/ironposh-winrm/src/cores/tag.rs index e9efbba..2f874c8 100644 --- a/crates/ironposh-winrm/src/cores/tag.rs +++ b/crates/ironposh-winrm/src/cores/tag.rs @@ -174,17 +174,20 @@ where node } else { // Wrapper case (`Tag, _>`): exactly one child element, which - // must be N. Reject zero (wrong wrapper), >1 (malformed), or a single - // child of the wrong name rather than silently picking one. + // must be N. Reject zero (wrong wrapper), >1 (malformed), a single + // child of the wrong name, or stray text rather than silently + // picking one. + ironposh_xml::mapping::reject_mixed_content(node)?; let mut elements = node .children() .filter(ironposh_xml::parser::Node::is_element); - let only = elements - .next() - .ok_or_else(|| ironposh_xml::XmlError::XmlInvalidTag { - expected: N::TAG_NAME.to_string(), - found: node.tag_name().name().to_string(), - })?; + let only = elements.next().ok_or_else(|| { + ironposh_xml::XmlError::InvalidXml(format!( + "expected a <{}> child in <{}>, found none", + N::TAG_NAME, + node.tag_name().name() + )) + })?; if elements.next().is_some() { return Err(ironposh_xml::XmlError::InvalidXml(format!( "expected exactly one child element in <{}>", @@ -201,14 +204,16 @@ where }; let value = V::from_xml(element)?; - let attributes = element - .attributes() - .filter_map(|attr| { - Attribute::from_name_and_value(attr.name(), attr.value()) - .ok() - .flatten() - }) - .collect(); + // Identity is (namespace-URI, local-name); a parse error on a *known* + // attribute is propagated, while a truly unknown attribute is ignored. + let mut attributes = Vec::new(); + for attr in element.attributes() { + if let Some(parsed) = + Attribute::from_name_and_value(attr.namespace(), attr.name(), attr.value())? + { + attributes.push(parsed); + } + } let namespaces_declaration = NamespaceDeclaration::from_xml(element)?; Ok(Tag { @@ -298,4 +303,32 @@ mod tests { .expect("nested CommandResponse/CommandId should parse"); assert_eq!(tag.value.value.0.to_string().to_uppercase(), uuid); } + + /// A wrapper that yields no usable child must be rejected, not defaulted. + #[test] + fn nested_tag_rejects_no_child() { + let xml = format!(r#""#); + let doc = parse(&xml).unwrap(); + assert!(CommandResponse::from_xml(doc.root_element()).is_err()); + } + + /// More than one child element is ambiguous; the descend branch must reject + /// it rather than silently picking the first. + #[test] + fn nested_tag_rejects_multiple_children() { + let uuid = "2D6534D0-6B12-40E3-B773-CBA26459CFA8"; + let xml = format!( + r#"{uuid}{uuid}"# + ); + let doc = parse(&xml).unwrap(); + assert!(CommandResponse::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn nested_tag_rejects_wrong_child_name() { + let xml = + format!(r#""#); + let doc = parse(&xml).unwrap(); + assert!(CommandResponse::from_xml(doc.root_element()).is_err()); + } } diff --git a/crates/ironposh-winrm/src/cores/tag_name.rs b/crates/ironposh-winrm/src/cores/tag_name.rs index 69d9f73..3eab122 100644 --- a/crates/ironposh-winrm/src/cores/tag_name.rs +++ b/crates/ironposh-winrm/src/cores/tag_name.rs @@ -43,14 +43,14 @@ tag!(DesiredStream = Text<'a> => WsmanShell); tag!(Stream = Text<'a> => WsmanShell); tag!(ExitCode = I32 => WsmanShell); tag!(CommandId = WsUuid => WsmanShell); -tag!(CommandResponse = CommandId<'a> => WsmanShell); // wraps a single CommandId child +tag!(CommandResponse = CommandId<'a> => WsmanShell); tag!(Command = Text<'a> => WsmanShell); tag!(Arguments = Text<'a> => WsmanShell); tag!(DisconnectResponse = Empty => WsmanShell); tag!(Reconnect = Empty => WsmanShell); tag!(ReconnectResponse = Empty => WsmanShell); tag!(SignalCode = "Code": Text<'a> => WsmanShell); -tag!(Signal = SignalCode<'a> => WsmanShell); // wraps a single Code child +tag!(Signal = SignalCode<'a> => WsmanShell); tag!(SignalResponse = Empty => WsmanShell); tag!(CreationXml = "creationXml": Text<'a> => PowerShellRemoting); diff --git a/crates/ironposh-winrm/src/cores/tag_value.rs b/crates/ironposh-winrm/src/cores/tag_value.rs index 39ea8ce..5bbb689 100644 --- a/crates/ironposh-winrm/src/cores/tag_value.rs +++ b/crates/ironposh-winrm/src/cores/tag_value.rs @@ -11,17 +11,34 @@ pub trait TagValue<'a> { /// The text content of a leaf element, rejecting mixed content. A text-valued /// element (`Text`, `WsUuid`, `Time`, numerics) must not contain child elements; /// silently truncating such malformed input would let it slip through. -pub(crate) fn leaf_text<'a>(node: Node<'a, 'a>) -> Result<&'a str, XmlError> { - // Only text children are allowed. Any element (mixed content), comment, or PI - // is rejected — otherwise `node.text()` (which yields the first child only when - // it is a text node) could silently shadow or drop the real value. - if node.children().any(|child| !child.is_text()) { +pub(crate) fn leaf_text<'a>(node: Node<'a, 'a>) -> Result, XmlError> { + // A text leaf carries no child elements; an element child is mixed content + // that would corrupt the value. Comments/PIs are tolerated, and text runs are + // concatenated — a comment between two runs splits one logical value into two + // text nodes, so reading only the first would silently truncate it. + if node.children().any(|child| child.is_element()) { return Err(XmlError::InvalidXml(format!( - "<{}> is a text leaf but contains non-text content", + "<{}> is a text leaf but contains a child element", node.tag_name().name() ))); } - Ok(node.text().unwrap_or("").trim()) + // Only text nodes contribute — a comment node's own text would otherwise leak in. + let mut runs = node + .children() + .filter(Node::is_text) + .filter_map(|c| c.text()); + let first = runs.next().unwrap_or(""); + runs.next().map_or_else( + || Ok(Cow::Borrowed(first.trim())), + |second| { + let mut combined = String::from(first); + combined.push_str(second); + for run in runs { + combined.push_str(run); + } + Ok(Cow::Owned(combined.trim().to_string())) + }, + ) } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -65,7 +82,7 @@ impl<'a> TagValue<'a> for Text<'a> { impl<'a> FromXml<'a> for Text<'a> { fn from_xml(node: Node<'a, 'a>) -> Result { - Ok(Self(leaf_text(node)?.into())) + Ok(Self(leaf_text(node)?)) } } @@ -86,8 +103,8 @@ impl<'a> TagValue<'a> for Empty { impl<'a> FromXml<'a> for Empty { fn from_xml(node: Node<'a, 'a>) -> Result { - // `leaf_text` rejects any non-text child (element/comment/PI); an empty - // tag additionally must have no non-whitespace text. + // `leaf_text` rejects child elements; an empty tag additionally must have + // no non-whitespace text. if !leaf_text(node)?.is_empty() { return Err(XmlError::InvalidXml(format!( "<{}> must be empty but has text content", @@ -121,6 +138,7 @@ impl<'a> TagValue<'a> for WsUuid { impl<'a> FromXml<'a> for WsUuid { fn from_xml(node: Node<'a, 'a>) -> Result { let text = leaf_text(node)?; + let text = text.as_ref(); // WS-Management prefixes UUIDs with "uuid:" — strip it if present. let raw = text.strip_prefix("uuid:").unwrap_or(text); uuid::Uuid::parse_str(raw) @@ -211,3 +229,57 @@ impl<'a> FromXml<'a> for ReadOnlyUnParsed<'a> { Ok(ReadOnlyUnParsed::Node(node)) } } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::parser::parse; + + #[test] + fn text_leaf_rejects_mixed_content() { + let doc = parse("text").unwrap(); + assert!(Text::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn text_leaf_trims_surrounding_whitespace() { + let doc = parse(" hello ").unwrap(); + assert_eq!( + Text::from_xml(doc.root_element()).unwrap().as_ref(), + "hello" + ); + } + + #[test] + fn text_leaf_tolerates_comment_and_concatenates_runs() { + let doc = parse("foobar").unwrap(); + assert_eq!( + Text::from_xml(doc.root_element()).unwrap().as_ref(), + "foobar" + ); + } + + #[test] + fn numeric_leaf_rejects_mixed_content() { + let doc = parse("1").unwrap(); + assert!(U32::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn wsuuid_leaf_rejects_mixed_content() { + let doc = parse("uuid:2d6534d0-6b12-40e3-b773-cba26459cfa8").unwrap(); + assert!(WsUuid::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn empty_rejects_text_content() { + let doc = parse("nope").unwrap(); + assert!(Empty::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn empty_accepts_whitespace_only() { + let doc = parse(" ").unwrap(); + assert!(Empty::from_xml(doc.root_element()).is_ok()); + } +} diff --git a/crates/ironposh-winrm/src/macros.rs b/crates/ironposh-winrm/src/macros.rs index fa495c5..381a732 100644 --- a/crates/ironposh-winrm/src/macros.rs +++ b/crates/ironposh-winrm/src/macros.rs @@ -3,7 +3,8 @@ /// Generates a zero-sized `TagName` marker (`Tag`) that pins the element's /// wire name + namespace at the type level, plus an ergonomic type alias /// `Alias<'a> = Tag<'a, Value, AliasTag>` used in struct fields and construction. -/// The marker stays internal; everything else writes the alias. +/// The alias is the normal spelling; the `Tag` marker is also public, +/// written directly at the few `Tag::from_name` call sites. /// /// Forms: /// - `tag!(Get = Text<'a> => DmtfWsmanSchema)` — wire name is the alias ident. diff --git a/crates/ironposh-winrm/src/rsp/commandline.rs b/crates/ironposh-winrm/src/rsp/commandline.rs index edb2894..aaefed3 100644 --- a/crates/ironposh-winrm/src/rsp/commandline.rs +++ b/crates/ironposh-winrm/src/rsp/commandline.rs @@ -43,6 +43,7 @@ impl TagValue<'_> for CommandLineValue { impl<'a> FromXml<'a> for CommandLineValue { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut command = None; let mut seen_command = false; let mut arguments = Vec::new(); @@ -65,3 +66,28 @@ impl<'a> FromXml<'a> for CommandLineValue { Ok(Self { command, arguments }) } } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::parser::parse; + + const RSP: &str = "http://schemas.microsoft.com/wbem/wsman/1/windows/shell"; + + #[test] + fn rejects_duplicate_command() { + let xml = format!( + r#"ab"# + ); + let doc = parse(&xml).unwrap(); + assert!(CommandLineValue::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn empty_command_element_is_none() { + let xml = format!(r#""#); + let doc = parse(&xml).unwrap(); + let value = CommandLineValue::from_xml(doc.root_element()).unwrap(); + assert!(value.command.is_none()); + } +} diff --git a/crates/ironposh-winrm/src/rsp/receive.rs b/crates/ironposh-winrm/src/rsp/receive.rs index aa39cd8..da3bf64 100644 --- a/crates/ironposh-winrm/src/rsp/receive.rs +++ b/crates/ironposh-winrm/src/rsp/receive.rs @@ -30,6 +30,7 @@ impl<'a> TagValue<'a> for ReceiveValue<'a> { impl<'a> FromXml<'a> for ReceiveValue<'a> { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut desired_streams = Vec::new(); for child in node.children() { if child.is_element_named(DesiredStreamTag::NAMESPACE, DesiredStreamTag::TAG_NAME) { @@ -108,6 +109,7 @@ impl<'a> TagValue<'a> for ReceiveResponseValue<'a> { impl<'a> FromXml<'a> for ReceiveResponseValue<'a> { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut streams = Vec::new(); let mut command_state = None; for child in node.children() { @@ -129,3 +131,30 @@ impl<'a> FromXml<'a> for ReceiveResponseValue<'a> { }) } } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::parser::parse; + + const RSP: &str = "http://schemas.microsoft.com/wbem/wsman/1/windows/shell"; + + #[test] + fn rejects_duplicate_command_state() { + let xml = format!( + r#""# + ); + let doc = parse(&xml).unwrap(); + assert!(ReceiveResponseValue::from_xml(doc.root_element()).is_err()); + } + + /// The `#[derive(FromXml)]` singleton field must reject a second binding. + #[test] + fn derive_rejects_duplicate_exit_code() { + let xml = format!( + r#"01"# + ); + let doc = parse(&xml).unwrap(); + assert!(CommandStateValue::from_xml(doc.root_element()).is_err()); + } +} diff --git a/crates/ironposh-winrm/src/rsp/send.rs b/crates/ironposh-winrm/src/rsp/send.rs index 3606e63..304bf7c 100644 --- a/crates/ironposh-winrm/src/rsp/send.rs +++ b/crates/ironposh-winrm/src/rsp/send.rs @@ -6,10 +6,8 @@ use ironposh_xml::{ mapping::{FromXml, NodeExt}, }; -// NOTE: this `Send` type alias shares its spelling with `std::marker::Send`. -// They live in different namespaces (type alias vs trait), so they don't -// collide here — but avoid writing a bare `Send` trait bound in modules that -// import this alias. +// `Send` here is this type alias, not `std::marker::Send` — don't write a bare +// `Send` trait bound in modules that import it. tag!(Send = SendValue<'a> => WsmanShell); /// Value for Send element containing multiple Stream elements @@ -31,6 +29,7 @@ impl<'a> TagValue<'a> for SendValue<'a> { impl<'a> FromXml<'a> for SendValue<'a> { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut streams = Vec::new(); for child in node.children() { if child.is_element_named(StreamTag::NAMESPACE, StreamTag::TAG_NAME) { diff --git a/crates/ironposh-winrm/src/soap/fault.rs b/crates/ironposh-winrm/src/soap/fault.rs index f2527d8..e311a57 100644 --- a/crates/ironposh-winrm/src/soap/fault.rs +++ b/crates/ironposh-winrm/src/soap/fault.rs @@ -33,12 +33,35 @@ pub struct SoapFaultSubcodeValue<'a> { pub value: Option>, } -#[derive(Debug, Clone, typed_builder::TypedBuilder, SimpleTagValue, FromXml)] +#[derive(Debug, Clone, typed_builder::TypedBuilder, SimpleTagValue)] pub struct SoapFaultReasonValue<'a> { #[builder(default, setter(into, strip_option))] pub text: Option>, } +impl<'a> ironposh_xml::mapping::FromXml<'a> for SoapFaultReasonValue<'a> { + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + use ironposh_xml::mapping::NodeExt; + ironposh_xml::mapping::reject_mixed_content(node)?; + // SOAP 1.2 permits several reason entries; keep the + // first rather than rejecting a valid multilingual fault as a duplicate. + let mut text = None; + for child in node.children() { + if text.is_none() + && child.is_element_named( + ::NAMESPACE, + ::TAG_NAME, + ) + { + text = Some(::from_xml( + child, + )?); + } + } + Ok(Self { text }) + } +} + impl SoapFaultValue<'_> { /// Check if this SOAP fault represents a WS-Management operation timeout. /// @@ -79,3 +102,23 @@ impl SoapFaultValue<'_> { .map(|t| <&str>::from(t.as_ref())) } } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::mapping::FromXml as _; + use ironposh_xml::parser::parse; + + const S: &str = "http://www.w3.org/2003/05/soap-envelope"; + + #[test] + fn reason_accepts_multiple_text_entries() { + let xml = format!( + r#"EnglishFrench"# + ); + let doc = parse(&xml).unwrap(); + let reason = + Reason::from_xml(doc.root_element()).expect("multilingual reason should parse"); + assert!(reason.as_ref().text.is_some()); + } +} diff --git a/crates/ironposh-winrm/src/soap/mod.rs b/crates/ironposh-winrm/src/soap/mod.rs index 1d2c261..1a5db19 100644 --- a/crates/ironposh-winrm/src/soap/mod.rs +++ b/crates/ironposh-winrm/src/soap/mod.rs @@ -3,15 +3,14 @@ pub mod fault; pub mod header; pub mod parsing; -use ironposh_macros::FromXml; - use crate::cores::TagValue; use crate::tag; use crate::{soap::body::Body, soap::header::Header}; +use ironposh_xml::mapping::{FromXml, NodeExt}; tag!(Envelope = SoapEnvelope<'a> => SoapEnvelope2003); -#[derive(Debug, Clone, typed_builder::TypedBuilder, FromXml)] +#[derive(Debug, Clone, typed_builder::TypedBuilder)] pub struct SoapEnvelope<'a> { #[builder(default, setter(into, strip_option))] pub header: Option>, @@ -19,6 +18,58 @@ pub struct SoapEnvelope<'a> { pub body: Body<'a>, } +impl<'a> FromXml<'a> for SoapEnvelope<'a> { + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + // Public parse entry point: validate the root really is {SOAP}Envelope. + // The derived value parser alone validates only children, so it would + // accept any wrapper that happens to carry a Body. + if !node.is_element_named( + ::NAMESPACE, + ::TAG_NAME, + ) { + return Err(ironposh_xml::XmlError::XmlInvalidTag { + expected: ::TAG_NAME.to_string(), + found: node.tag_name().name().to_string(), + }); + } + ironposh_xml::mapping::reject_mixed_content(node)?; + let mut header = None; + let mut body = None; + for child in node.children() { + if !child.is_element() { + continue; + } + if child.is_element_named( +
::NAMESPACE, +
::TAG_NAME, + ) { + if header.is_some() { + return Err(ironposh_xml::XmlError::InvalidXml( + "duplicate
in Envelope".into(), + )); + } + header = Some(Header::from_xml(child)?); + } else if child.is_element_named( + ::NAMESPACE, + ::TAG_NAME, + ) { + if body.is_some() { + return Err(ironposh_xml::XmlError::InvalidXml( + "duplicate in Envelope".into(), + )); + } + body = Some(Body::from_xml(child)?); + } + } + Ok(SoapEnvelope { + header, + body: body.ok_or_else(|| { + ironposh_xml::XmlError::InvalidXml("Missing body in SoapEnvelope".into()) + })?, + }) + } +} + impl<'a> TagValue<'a> for SoapEnvelope<'a> { fn append_to_element( self, @@ -34,3 +85,27 @@ impl<'a> TagValue<'a> for SoapEnvelope<'a> { .add_child(self.body.into_element()) } } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::parser::parse; + + const S: &str = "http://www.w3.org/2003/05/soap-envelope"; + + /// A non-Envelope root carrying a valid Body must be rejected, not accepted + /// as an envelope. + #[test] + fn rejects_non_envelope_root_with_body() { + let xml = format!(r#""#); + let doc = parse(&xml).unwrap(); + assert!(SoapEnvelope::from_xml(doc.root_element()).is_err()); + } + + #[test] + fn accepts_envelope_root_regardless_of_prefix() { + let xml = format!(r#""#); + let doc = parse(&xml).unwrap(); + assert!(SoapEnvelope::from_xml(doc.root_element()).is_ok()); + } +} diff --git a/crates/ironposh-winrm/src/test_macro.rs b/crates/ironposh-winrm/src/test_macro.rs index 048ccb6..4a23473 100644 --- a/crates/ironposh-winrm/src/test_macro.rs +++ b/crates/ironposh-winrm/src/test_macro.rs @@ -51,6 +51,25 @@ mod tests { assert!(parsed.opt_b.is_none()); } + #[test] + fn test_deserialize_optional_permutations() { + // Both optionals present. + let xml = format!( + r#"aboaob"# + ); + let doc = ironposh_xml::parser::parse(&xml).expect("parse"); + let parsed = TestStruct::from_xml(doc.root_element()).expect("deserialize"); + assert_eq!(parsed.opt_a.unwrap().value.as_ref(), "oa"); + assert_eq!(parsed.opt_b.unwrap().value.as_ref(), "ob"); + + // Neither optional present. + let xml = format!(r#"ab"#); + let doc = ironposh_xml::parser::parse(&xml).expect("parse"); + let parsed = TestStruct::from_xml(doc.root_element()).expect("deserialize"); + assert!(parsed.opt_a.is_none()); + assert!(parsed.opt_b.is_none()); + } + #[test] fn test_deserialize_missing_required_field() { let xml = format!(r#"only-a"#); diff --git a/crates/ironposh-winrm/src/ws_addressing/mod.rs b/crates/ironposh-winrm/src/ws_addressing/mod.rs index 4ab1396..4d733f0 100644 --- a/crates/ironposh-winrm/src/ws_addressing/mod.rs +++ b/crates/ironposh-winrm/src/ws_addressing/mod.rs @@ -9,62 +9,3 @@ tag!(ReplyTo = AddressValue<'a> => WsAddressing2004); pub struct AddressValue<'a> { pub url: Address<'a>, } - -// impl<'a> TagValue<'a> for AddressValue<'a> { -// fn append_to_element(self, element: ironposh_xml::builder::Element<'a>) -> ironposh_xml::builder::Element<'a> { -// let inner_element = self.url.into_element(); -// element.add_child(inner_element) -// } -// } - -// pub struct AddressVisitor<'a> { -// address: Option>, -// } - -// impl<'a> XmlVisitor<'a> for AddressVisitor<'a> { -// type Value = AddressValue<'a>; - -// fn visit_children( -// &mut self, -// children: impl Iterator>, -// ) -> Result<(), ironposh_xml::XmlError> { -// for child in children { -// if !child.is_element() { -// continue; -// } - -// match (child.tag_name().name(), child.tag_name().namespace()) { -// (Address::TAG_NAME, Address::NAMESPACE) => { -// let tag = Tag::from_node(child)?; -// self.address = Some(AddressValue { url: tag }); -// } -// _ => { -// warn!( -// "Unexpected child element in AddressValue: {}", -// child.tag_name().name() -// ); -// } -// } -// } - -// Ok(()) -// } - -// fn finish(self) -> Result { -// Ok(AddressValue { -// url: self.address.ok_or_else(|| { -// ironposh_xml::XmlError::NotSupposeToBeCalled { -// extra_info: "AddressValue must contain an Address element".to_string(), -// } -// }?), -// }) -// } -// } - -// impl<'a> XmlDeserialize<'a> for AddressValue<'a> { -// type Visitor = AddressVisitor<'a>; - -// fn visitor() -> Self::Visitor { -// AddressVisitor { address: None } -// } -// } diff --git a/crates/ironposh-winrm/src/ws_management/header.rs b/crates/ironposh-winrm/src/ws_management/header.rs index 738cd50..f4b6ed5 100644 --- a/crates/ironposh-winrm/src/ws_management/header.rs +++ b/crates/ironposh-winrm/src/ws_management/header.rs @@ -58,14 +58,19 @@ impl<'a> TagValue<'a> for SelectorSetValue { impl<'a> FromXml<'a> for SelectorSetValue { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut selectors = HashMap::new(); for child in node.children() { - if child.is_element_named(SelectorTag::NAMESPACE, SelectorTag::TAG_NAME) - && let Some(name) = child + if child.is_element_named(SelectorTag::NAMESPACE, SelectorTag::TAG_NAME) { + let name = child .attributes() - .find(|attr| attr.name() == "Name") + .find(|attr| attr.namespace().is_none() && attr.name() == "Name") .map(|attr| attr.value().to_string()) - { + .ok_or_else(|| { + ironposh_xml::XmlError::InvalidXml( + " missing Name attribute".into(), + ) + })?; if selectors.contains_key(&name) { return Err(ironposh_xml::XmlError::InvalidXml(format!( "duplicate selector {name:?}" @@ -122,14 +127,17 @@ impl<'a> TagValue<'a> for OptionSetValue { impl<'a> FromXml<'a> for OptionSetValue { fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + ironposh_xml::mapping::reject_mixed_content(node)?; let mut options = HashMap::new(); for child in node.children() { - if child.is_element_named(OptionTagNameTag::NAMESPACE, OptionTagNameTag::TAG_NAME) - && let Some(name) = child + if child.is_element_named(OptionTagNameTag::NAMESPACE, OptionTagNameTag::TAG_NAME) { + let name = child .attributes() - .find(|attr| attr.name() == "Name") + .find(|attr| attr.namespace().is_none() && attr.name() == "Name") .map(|attr| attr.value().to_string()) - { + .ok_or_else(|| { + ironposh_xml::XmlError::InvalidXml("