From 0c87e7e512a5795efcb2c67f49b3ffdb079b5471 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 23 Jun 2026 15:37:23 -0400 Subject: [PATCH 1/3] feat(xml): namespace-correct FromXml derive (kills the deserialize visitor) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deserialize side is where the visitor ceremony lived (accumulator structs, finish(), NotSupposeToBeCalled). Replace it with a direct, attribute-free derive: - ironposh-xml `mapping`: `FromXml`/`ToXml` + `NodeExt` — element identity is the (namespace-URI, local-name) pair; the prefix is never compared. - `#[derive(FromXml)]`: generates a flat `from_xml(node)` that matches each child by (URI, name) sourced from the field's `Tag<_, _, N>` type. No visitor struct, no finish(). Reuses the existing Tag/TagName types, so migrating a struct is a one-line derive add. Proven on `DisconnectValue`: parses IdleTimeOut regardless of prefix, and ignores a same-local-name child in the wrong namespace (the bug the old visit_node had — it matched on local name only). Additive: the existing XmlVisitor/SimpleXmlDeserialize path is untouched; removing it across the winrm structs is the mechanical follow-up. --- crates/ironposh-macros/src/lib.rs | 96 +++++++++++ crates/ironposh-winrm/src/rsp/disconnect.rs | 37 +++- crates/ironposh-xml/src/lib.rs | 1 + crates/ironposh-xml/src/mapping.rs | 176 ++++++++++++++++++++ 4 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 crates/ironposh-xml/src/mapping.rs diff --git a/crates/ironposh-macros/src/lib.rs b/crates/ironposh-macros/src/lib.rs index 8dc3ee8..57c56c1 100644 --- a/crates/ironposh-macros/src/lib.rs +++ b/crates/ironposh-macros/src/lib.rs @@ -1271,6 +1271,102 @@ pub fn derive_simple_xml_deserialize(input: TokenStream) -> TokenStream { TokenStream::from(expanded) } +/// Derives [`ironposh_xml::mapping::FromXml`] for a WinRM struct whose fields +/// are `Tag<'a, V, N>` / `Option>`. +/// +/// Generates a direct, namespace-correct `from_xml(node)` — no visitor struct, +/// no `finish()`. Each child element is matched by its `(namespace-URI, +/// local-name)` pair, sourced from the field's `N: TagName` (`NAMESPACE` + +/// `TAG_NAME`); the prefix is never compared. `Option<_>` fields stay `None` +/// when absent; required fields error. This is the deserialize-side replacement +/// for the visitor that `SimpleXmlDeserialize` generates. +#[proc_macro_derive(FromXml)] +pub fn derive_from_xml(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + TokenStream::from(impl_from_xml(&input)) +} + +fn impl_from_xml(input: &DeriveInput) -> TokenStream2 { + let name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + + let fields = match &input.data { + Data::Struct(data) => match &data.fields { + Fields::Named(fields) => &fields.named, + _ => panic!("FromXml can only be derived for structs with named fields"), + }, + _ => panic!("FromXml can only be derived for structs"), + }; + + let entries: Vec = fields + .iter() + .map(|field| { + let field_name = field.ident.as_ref().unwrap().clone(); + let field_type = field.ty.clone(); + let is_optional = is_option_type(&field_type); + let tag_name_type = extract_tag_name_type(&field_type); + SimpleFieldEntry { + field_name, + field_type, + tag_name_type, + is_optional, + } + }) + .collect(); + + let inits = entries.iter().map(|e| { + let f = &e.field_name; + quote! { let mut #f = None; } + }); + + // One namespace-correct match per field: identity is (URI, local-name). + let matchers = entries.iter().filter_map(|e| { + let f = &e.field_name; + e.tag_name_type.as_ref().map(|n| { + quote! { + if child.is_element_named( + ::NAMESPACE, + ::TAG_NAME, + ) { + #f = Some(ironposh_xml::parser::XmlDeserialize::from_node(child)?); + continue; + } + } + }) + }); + + let construct = entries.iter().map(|e| { + let f = &e.field_name; + if e.is_optional { + quote! { #f } + } else { + quote! { + #f: #f.ok_or_else(|| ironposh_xml::XmlError::InvalidXml( + format!("Missing {} in {}", stringify!(#f), stringify!(#name)) + ))? + } + } + }); + + quote! { + impl #impl_generics ironposh_xml::mapping::FromXml<'a> for #name #ty_generics #where_clause { + fn from_xml( + node: ironposh_xml::parser::Node<'a, 'a>, + ) -> Result { + use ironposh_xml::mapping::NodeExt; + #(#inits)* + for child in node.children() { + if !child.is_element() { + continue; + } + #(#matchers)* + } + Ok(#name { #(#construct),* }) + } + } + } +} + fn impl_simple_tag_value(input: &DeriveInput) -> TokenStream2 { let name = &input.ident; let generics = &input.generics; diff --git a/crates/ironposh-winrm/src/rsp/disconnect.rs b/crates/ironposh-winrm/src/rsp/disconnect.rs index 0adcedd..f450ab0 100644 --- a/crates/ironposh-winrm/src/rsp/disconnect.rs +++ b/crates/ironposh-winrm/src/rsp/disconnect.rs @@ -2,12 +2,45 @@ use crate::cores::{ Tag, Time, tag_name::{IdleTimeOut, TagName}, }; -use ironposh_macros::{SimpleTagValue, SimpleXmlDeserialize}; +use ironposh_macros::{FromXml, SimpleTagValue, SimpleXmlDeserialize}; /// Value for the Disconnect element (MS-WSMV 3.1.4.13). /// Optionally carries an IdleTimeOut serialized as `PT{seconds}S`. -#[derive(Debug, Clone, typed_builder::TypedBuilder, SimpleTagValue, SimpleXmlDeserialize)] +#[derive(Debug, Clone, typed_builder::TypedBuilder, SimpleTagValue, SimpleXmlDeserialize, FromXml)] pub struct DisconnectValue<'a> { #[builder(default, setter(strip_option, into))] pub idle_time_out: Option>, } + +#[cfg(test)] +mod tests { + use super::*; + use ironposh_xml::mapping::FromXml as _; + use ironposh_xml::parser::parse; + + const RSP: &str = "http://schemas.microsoft.com/wbem/wsman/1/windows/shell"; + + #[test] + fn from_xml_reads_idle_timeout_namespace_correctly() { + // IdleTimeOut under the correct (rsp) namespace — and with a *different* + // prefix than the canonical one, to prove identity is by URI, not prefix. + let xml = format!( + r#"PT60.000S"# + ); + let doc = parse(&xml).unwrap(); + let value = DisconnectValue::from_xml(doc.root_element()).unwrap(); + + let secs = value.idle_time_out.expect("idle_time_out present").value.0; + assert!((secs - 60.0).abs() < 1e-9); + } + + #[test] + fn from_xml_ignores_idle_timeout_in_wrong_namespace() { + // Same local name, wrong namespace URI — must not bind. + let xml = r#"PT60.000S"#; + let doc = parse(xml).unwrap(); + let value = DisconnectValue::from_xml(doc.root_element()).unwrap(); + + assert!(value.idle_time_out.is_none()); + } +} diff --git a/crates/ironposh-xml/src/lib.rs b/crates/ironposh-xml/src/lib.rs index b4044f5..1f8c8dd 100644 --- a/crates/ironposh-xml/src/lib.rs +++ b/crates/ironposh-xml/src/lib.rs @@ -1,6 +1,7 @@ use roxmltree::NodeType; pub mod builder; +pub mod mapping; pub mod parser; #[derive(Debug, thiserror::Error)] diff --git a/crates/ironposh-xml/src/mapping.rs b/crates/ironposh-xml/src/mapping.rs new file mode 100644 index 0000000..a1e8f75 --- /dev/null +++ b/crates/ironposh-xml/src/mapping.rs @@ -0,0 +1,176 @@ +//! Generic, namespace-aware mapping traits — the use-case-agnostic core. +//! +//! `ironposh-xml` owns the XML *mechanism*: parse (roxmltree), build +//! ([`Element`](crate::builder::Element)), and namespace matching/emission **by +//! URI**. It owns no *vocabulary*: concrete namespace URIs, tag names, and +//! schema rules all live in consumer crates. The two traits below are the +//! entire mapping contract — one entry point each, no visitor split. +//! +//! The invariant that makes namespaces correct: an element's identity is the +//! pair `(namespace-URI, local-name)`. The prefix (`s:`, `soap:`, …) is an +//! arbitrary, document-local alias and is never compared. roxmltree resolves +//! prefixes to URIs for us; [`NodeExt`] is the single matching primitive built +//! on top of that. + +use crate::XmlError; +use crate::builder::Element; +use crate::parser::Node; + +/// Namespace-aware identity helpers for a parsed node. +/// +/// Matching is always against the namespace **URI** (caller-supplied), never +/// the prefix. `None` denotes "no namespace" (an unprefixed element under no +/// default namespace, or an unqualified attribute). +pub trait NodeExt<'a> { + /// The element's expanded name: `(namespace_uri, local_name)`. + fn expanded_name(&self) -> (Option<&'a str>, &'a str); + + /// `true` iff this node is an element whose expanded name equals + /// `(ns, local)`. The prefix is irrelevant. + fn is_element_named(&self, ns: Option<&str>, local: &str) -> bool; +} + +impl<'a> NodeExt<'a> for Node<'a, 'a> { + fn expanded_name(&self) -> (Option<&'a str>, &'a str) { + let name = self.tag_name(); + (name.namespace(), name.name()) + } + + fn is_element_named(&self, ns: Option<&str>, local: &str) -> bool { + self.is_element() + && self.tag_name().name() == local + && self.tag_name().namespace() == ns + } +} + +/// Build `Self` from a parsed XML element node. +/// +/// One entry point. A consumer type (or, later, a `#[derive(FromXml)]`) walks +/// the node's attributes/children using [`NodeExt`] to match by `(URI, name)`. +pub trait FromXml<'a>: Sized { + fn from_xml(node: Node<'a, 'a>) -> Result; +} + +/// Render `Self` into an XML [`Element`], borrowing from `self`. +/// +/// The symmetric counterpart to [`FromXml`]: the same field shape that +/// `from_xml` reads, `to_xml` writes — declaring each namespace by URI with a +/// chosen alias. +pub trait ToXml { + fn to_xml(&self) -> Element<'_>; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::parser::parse; + + // ── Vocabulary lives in the *consumer*, never in ironposh-xml. ── + // These are plain `&str` URIs a downstream crate would own (e.g. the WinRM + // `Namespace` registry). The traits above never hard-code them. + const SOAP_ENVELOPE: &str = "http://www.w3.org/2003/05/soap-envelope"; + const WS_ADDRESSING: &str = "http://schemas.xmlsoap.org/ws/2004/08/addressing"; + const OTHER_NS: &str = "http://example.com/other"; + + /// A trivial consumer-side message type. This hand-written impl is exactly + /// the shape a future `#[derive(FromXml, ToXml)]` would generate — it is the + /// macro's expansion target, written out so we can prove the foundation. + #[derive(Debug, PartialEq, Eq)] + struct SoapBody { + action: Option, + } + + impl<'a> FromXml<'a> for SoapBody { + fn from_xml(node: Node<'a, 'a>) -> Result { + // Identity check by (URI, local) — prefix-blind. + if !node.is_element_named(Some(SOAP_ENVELOPE), "Body") { + let (ns, local) = node.expanded_name(); + return Err(XmlError::XmlInvalidNamespace { + expected: format!("({SOAP_ENVELOPE}, Body)"), + found: Some(format!("({ns:?}, {local})")), + }); + } + + let mut action = None; + for child in node.children() { + // Same-local-name in a different namespace must NOT match. + if child.is_element_named(Some(WS_ADDRESSING), "Action") { + action = child.text().map(str::to_owned); + } + } + Ok(SoapBody { action }) + } + } + + impl ToXml for SoapBody { + fn to_xml(&self) -> Element<'_> { + let mut body = Element::new("Body") + .set_namespace(SOAP_ENVELOPE) + .add_namespace_declaration(SOAP_ENVELOPE, Some("s")) + .add_namespace_declaration(WS_ADDRESSING, Some("a")); + + if let Some(action) = &self.action { + body = body.add_child( + Element::new("Action") + .set_namespace(WS_ADDRESSING) + .set_text(action.as_str()), + ); + } + body + } + } + + fn root_of<'a>(doc: &'a roxmltree::Document<'a>) -> Node<'a, 'a> { + doc.root_element() + } + + #[test] + fn matches_regardless_of_prefix() { + // Same document, three encodings of the *same* element. A correct + // namespace-aware reader treats them identically. + let with_s = r#"cmd"#; + let with_soap = r#"cmd"#; + let default_ns = r#"cmd"#; + + let expected = SoapBody { + action: Some("cmd".to_owned()), + }; + + for xml in [with_s, with_soap, default_ns] { + let doc = parse(xml).unwrap(); + let parsed = SoapBody::from_xml(root_of(&doc)).unwrap(); + assert_eq!(parsed, expected, "prefix should not affect identity: {xml}"); + } + } + + #[test] + fn rejects_same_local_name_in_wrong_namespace() { + // Local name "Body" but the wrong namespace URI: must not be accepted. + let xml = format!(r#""#); + let doc = parse(&xml).unwrap(); + let err = SoapBody::from_xml(root_of(&doc)).unwrap_err(); + assert!(matches!(err, XmlError::XmlInvalidNamespace { .. })); + } + + #[test] + fn child_in_wrong_namespace_is_ignored() { + // "Action" exists but under SOAP, not WS-Addressing — it must not bind. + let xml = r#"cmd"#; + let doc = parse(xml).unwrap(); + let parsed = SoapBody::from_xml(root_of(&doc)).unwrap(); + assert_eq!(parsed, SoapBody { action: None }); + } + + #[test] + fn round_trips_through_to_xml() { + let original = SoapBody { + action: Some("cmd".to_owned()), + }; + + let xml = original.to_xml().to_xml_string().unwrap(); + let doc = parse(&xml).unwrap(); + let reparsed = SoapBody::from_xml(root_of(&doc)).unwrap(); + + assert_eq!(reparsed, original); + } +} From 543de7dc9ee290eade24779af3e17ea6d43a6c98 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 23 Jun 2026 16:14:55 -0400 Subject: [PATCH 2/3] refactor(winrm): replace the XML visitor framework with direct FromXml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deserialization across the WinRM/SOAP/WS-Management layer no longer uses the accumulator-visitor pattern. Every type now implements a single direct `ironposh_xml::mapping::FromXml::from_xml(node) -> Result`, and matching is by `(namespace-URI, local-name)` via `NodeExt` — the prefix is never compared, fixing the long-standing local-name-only matching bug. Removed (no longer needed): - `SimpleXmlDeserialize` derive + the `impl_xml_deserialize!` / `impl_tag_value!` macro_rules (the latter two were already dead). - Every hand-written `XmlVisitor` impl: TagVisitor, TagList, AnyTag, the leaf value visitors (Text/Empty/WsUuid/Time/numerics/Unparsed), SoapEnvelope, Receive/ReceiveResponse, Send, SelectorSet/OptionSet, CommandLine, namespace visitors. - The duplicate NodeDeserializer in winrm. Converted: - Structs whose fields are `Tag<_, _, N>` now `#[derive(FromXml)]` (one line); the value/leaf types and the irregular hand-written ones implement `FromXml` directly. Serialization (TagValue / Element builder) is unchanged. Correctness fixes surfaced by namespace-aware matching: - `creationXml` / `connectXml` / `connectResponseXml` carry the PowerShell namespace on the wire; their TagName definitions said `None`. Set to the real URI and taught the builder to emit a default-namespace element unprefixed (instead of erroring), keeping serialized output byte-identical. - Unknown extension namespaces/elements are now ignored (SOAP must-ignore) rather than failing the whole envelope. `ironposh-xml` keeps the legacy `XmlVisitor`/`XmlDeserialize` traits only because ps_value's CLIXML primitive layer still rides on them; ps_value's ComplexObject tree is untouched. Net: 310 insertions, 1804 deletions. --- .../runspace_pool/expect_shell_connected.rs | 4 +- .../src/runspace_pool/expect_shell_created.rs | 4 +- .../src/runspace_pool/incoming.rs | 8 +- crates/ironposh-macros/src/lib.rs | 249 +------------ crates/ironposh-winrm/src/cores/anytag.rs | 68 +--- crates/ironposh-winrm/src/cores/attribute.rs | 52 +-- crates/ironposh-winrm/src/cores/namespace.rs | 84 +---- crates/ironposh-winrm/src/cores/tag.rs | 174 ++------- crates/ironposh-winrm/src/cores/tag_list.rs | 52 +-- crates/ironposh-winrm/src/cores/tag_name.rs | 18 +- crates/ironposh-winrm/src/cores/tag_value.rs | 340 ++---------------- crates/ironposh-winrm/src/macros.rs | 233 +----------- crates/ironposh-winrm/src/rsp/commandline.rs | 62 +--- crates/ironposh-winrm/src/rsp/connect.rs | 8 +- crates/ironposh-winrm/src/rsp/disconnect.rs | 6 +- crates/ironposh-winrm/src/rsp/receive.rs | 107 ++---- crates/ironposh-winrm/src/rsp/send.rs | 45 +-- crates/ironposh-winrm/src/rsp/shell_value.rs | 6 +- crates/ironposh-winrm/src/soap/body.rs | 4 +- crates/ironposh-winrm/src/soap/fault.rs | 10 +- crates/ironposh-winrm/src/soap/header.rs | 2 +- crates/ironposh-winrm/src/soap/mod.rs | 79 +--- crates/ironposh-winrm/src/test_macro.rs | 27 +- .../ironposh-winrm/src/ws_addressing/mod.rs | 4 +- .../ironposh-winrm/src/ws_management/body.rs | 10 +- .../src/ws_management/header.rs | 161 ++------- .../tests/test_connect_request.rs | 6 +- .../tests/test_disconnect_request.rs | 6 +- .../tests/test_initial_deserialize_request.rs | 10 +- .../tests/test_malformed_soap.rs | 50 ++- .../tests/test_parse_error_response.rs | 4 +- .../tests/test_parse_receive_response.rs | 4 +- .../tests/test_parse_resource_created.rs | 4 +- .../ironposh-winrm/tests/test_receive_fix.rs | 4 +- crates/ironposh-xml/src/builder/element.rs | 19 +- crates/ironposh-xml/src/mapping.rs | 148 ++------ crates/ironposh-xml/src/parser/mod.rs | 42 +-- 37 files changed, 310 insertions(+), 1804 deletions(-) diff --git a/crates/ironposh-client-core/src/runspace_pool/expect_shell_connected.rs b/crates/ironposh-client-core/src/runspace_pool/expect_shell_connected.rs index 3c32862..cf3cd00 100644 --- a/crates/ironposh-client-core/src/runspace_pool/expect_shell_connected.rs +++ b/crates/ironposh-client-core/src/runspace_pool/expect_shell_connected.rs @@ -1,7 +1,7 @@ use base64::Engine; use ironposh_psrp::{MessageType, PsrpMessage, fragmentation}; use ironposh_winrm::soap::SoapEnvelope; -use ironposh_xml::parser::XmlDeserialize; +use ironposh_xml::mapping::FromXml; use tracing::{debug, info, trace, warn}; use super::enums::RunspacePoolState; @@ -23,7 +23,7 @@ impl ExpectShellConnected { let parsed = ironposh_xml::parser::parse(response)?; - let soap_response = SoapEnvelope::from_node(parsed.root_element()) + let soap_response = SoapEnvelope::from_xml(parsed.root_element()) .map_err(crate::PwshCoreError::XmlParsingError)?; RunspacePool::fault_to_error(&soap_response)?; diff --git a/crates/ironposh-client-core/src/runspace_pool/expect_shell_created.rs b/crates/ironposh-client-core/src/runspace_pool/expect_shell_created.rs index 9f6668f..72a6953 100644 --- a/crates/ironposh-client-core/src/runspace_pool/expect_shell_created.rs +++ b/crates/ironposh-client-core/src/runspace_pool/expect_shell_created.rs @@ -1,5 +1,5 @@ use ironposh_winrm::soap::SoapEnvelope; -use ironposh_xml::parser::XmlDeserialize; +use ironposh_xml::mapping::FromXml; use super::pool::RunspacePool; @@ -14,7 +14,7 @@ impl ExpectShellCreated { let parsed = ironposh_xml::parser::parse(response)?; - let soap_response = SoapEnvelope::from_node(parsed.root_element()) + let soap_response = SoapEnvelope::from_xml(parsed.root_element()) .map_err(crate::PwshCoreError::XmlParsingError)?; runspace_pool.shell.accept_create_response(&soap_response)?; diff --git a/crates/ironposh-client-core/src/runspace_pool/incoming.rs b/crates/ironposh-client-core/src/runspace_pool/incoming.rs index 01c0168..730929b 100644 --- a/crates/ironposh-client-core/src/runspace_pool/incoming.rs +++ b/crates/ironposh-client-core/src/runspace_pool/incoming.rs @@ -18,7 +18,7 @@ use ironposh_psrp::{ SessionCapability, fragmentation, }; use ironposh_winrm::{soap::SoapEnvelope, ws_management::WsAction}; -use ironposh_xml::parser::XmlDeserialize; +use ironposh_xml::mapping::FromXml; use rsa::pkcs1v15::Pkcs1v15Encrypt; use tracing::{debug, error, info, instrument, trace, warn}; use uuid::Uuid; @@ -46,7 +46,7 @@ impl RunspacePool { } let parsed = ironposh_xml::parser::parse(soap_envelope)?; - let soap_envelope = SoapEnvelope::from_node(parsed.root_element()) + let soap_envelope = SoapEnvelope::from_xml(parsed.root_element()) .map_err(crate::PwshCoreError::XmlParsingError)?; Self::fault_to_error(&soap_envelope)?; @@ -82,7 +82,7 @@ impl RunspacePool { } let parsed = ironposh_xml::parser::parse(soap_envelope)?; - let soap_envelope = SoapEnvelope::from_node(parsed.root_element()) + let soap_envelope = SoapEnvelope::from_xml(parsed.root_element()) .map_err(crate::PwshCoreError::XmlParsingError)?; Self::fault_to_error(&soap_envelope)?; @@ -147,7 +147,7 @@ impl RunspacePool { e })?; - let soap_envelope = SoapEnvelope::from_node(parsed.root_element()).map_err(|e| { + let soap_envelope = SoapEnvelope::from_xml(parsed.root_element()).map_err(|e| { error!(target: "soap", error = %e, "failed to parse SOAP envelope"); crate::PwshCoreError::XmlParsingError(e) })?; diff --git a/crates/ironposh-macros/src/lib.rs b/crates/ironposh-macros/src/lib.rs index 57c56c1..4e02649 100644 --- a/crates/ironposh-macros/src/lib.rs +++ b/crates/ironposh-macros/src/lib.rs @@ -1,7 +1,7 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, TokenStream as TokenStream2}; -use quote::{format_ident, quote}; -use syn::{parse_macro_input, Data, DeriveInput, Fields, Generics, LitStr, Type, TypePath}; +use quote::quote; +use syn::{parse_macro_input, Data, DeriveInput, Fields, LitStr, Type, TypePath}; /// Derives the CLIXML serialize side of a PSRP object struct (RFC #12, L3). /// @@ -1259,18 +1259,6 @@ pub fn derive_simple_tag_value(input: TokenStream) -> TokenStream { TokenStream::from(expanded) } -/// Derives XmlDeserialize implementation for structs where all fields are `Option>` -/// -/// This macro assumes that all fields in the struct are optional Tag fields and generates -/// a complete XmlDeserialize implementation with visitor pattern that can parse XML nodes -/// into the struct by matching tag names to field names. -#[proc_macro_derive(SimpleXmlDeserialize)] -pub fn derive_simple_xml_deserialize(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as DeriveInput); - let expanded = impl_simple_xml_deserialize(&input); - TokenStream::from(expanded) -} - /// Derives [`ironposh_xml::mapping::FromXml`] for a WinRM struct whose fields /// are `Tag<'a, V, N>` / `Option>`. /// @@ -1302,12 +1290,10 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 { .iter() .map(|field| { let field_name = field.ident.as_ref().unwrap().clone(); - let field_type = field.ty.clone(); - let is_optional = is_option_type(&field_type); - let tag_name_type = extract_tag_name_type(&field_type); + let is_optional = is_option_type(&field.ty); + let tag_name_type = extract_tag_name_type(&field.ty); SimpleFieldEntry { field_name, - field_type, tag_name_type, is_optional, } @@ -1328,7 +1314,7 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 { ::NAMESPACE, ::TAG_NAME, ) { - #f = Some(ironposh_xml::parser::XmlDeserialize::from_node(child)?); + #f = Some(ironposh_xml::mapping::FromXml::from_xml(child)?); continue; } } @@ -1432,58 +1418,8 @@ fn impl_simple_tag_value(input: &DeriveInput) -> TokenStream2 { } } -fn impl_simple_xml_deserialize(input: &DeriveInput) -> TokenStream2 { - let name = &input.ident; - let generics = &input.generics; - - let fields = match &input.data { - Data::Struct(data) => match &data.fields { - Fields::Named(fields) => &fields.named, - _ => panic!("RegularXmlDeserialize can only be derived for structs with named fields"), - }, - _ => panic!("RegularXmlDeserialize can only be derived for structs"), - }; - - let visitor_name = format_ident!("{}Visitor", name); - - // Extract field information - handle both Tag<..> and Option> - let field_entries: Vec = fields - .iter() - .map(|field| { - let field_name = field.ident.as_ref().unwrap().clone(); - let field_type = field.ty.clone(); - let is_optional = is_option_type(&field_type); - let tag_name_type = extract_tag_name_type(&field_type); - - SimpleFieldEntry { - field_name, - field_type, - tag_name_type, - is_optional, - } - }) - .collect(); - - // Generate Visitor struct - let visitor_struct = generate_simple_visitor_struct(&visitor_name, generics, &field_entries); - - // Generate XmlVisitor implementation - let xml_visitor_impl = - generate_simple_xml_visitor_impl(&visitor_name, name, generics, &field_entries); - - // Generate XmlDeserialize implementation - let xml_deserialize_impl = generate_xml_deserialize_impl(name, &visitor_name, generics); - - quote! { - #visitor_struct - #xml_visitor_impl - #xml_deserialize_impl - } -} - struct SimpleFieldEntry { field_name: Ident, - field_type: Type, tag_name_type: Option, is_optional: bool, } @@ -1493,160 +1429,6 @@ struct FieldInfo<'a> { is_optional: bool, } -fn generate_simple_visitor_struct( - visitor_name: &Ident, - generics: &Generics, - field_entries: &[SimpleFieldEntry], -) -> TokenStream2 { - let (impl_generics, _ty_generics, where_clause) = generics.split_for_impl(); - - let visitor_fields: Vec = field_entries - .iter() - .map(|entry| { - let field_name = &entry.field_name; - let field_type = &entry.field_type; - if entry.is_optional { - // Optional fields stay as Option> in visitor - quote! { pub #field_name: #field_type } - } else { - // Required fields are stored as Option> during parsing, then validated - quote! { pub #field_name: Option<#field_type> } - } - }) - .collect(); - - quote! { - #[derive(Debug, Clone, Default)] - pub struct #visitor_name #impl_generics #where_clause { - #(#visitor_fields),* - } - } -} - -fn generate_simple_xml_visitor_impl( - visitor_name: &Ident, - struct_name: &Ident, - generics: &Generics, - field_entries: &[SimpleFieldEntry], -) -> TokenStream2 { - let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - - // Generate match arms for each field - let match_arms: Vec = field_entries - .iter() - .filter_map(|entry| { - entry.tag_name_type.as_ref().map(|tag_name_type| { - let field_name = &entry.field_name; - quote! { - crate::cores::#tag_name_type::TAG_NAME => { - self.#field_name = Some(ironposh_xml::parser::XmlDeserialize::from_node(child)?); - } - } - }) - }) - .collect(); - - // Generate field list for finish method - let field_names: Vec<&Ident> = field_entries.iter().map(|f| &f.field_name).collect(); - let field_list = quote! { #(#field_names),* }; - - // Separate required and optional fields for finish method - let required_fields: Vec<&SimpleFieldEntry> = - field_entries.iter().filter(|f| !f.is_optional).collect(); - let _optional_fields: Vec<&SimpleFieldEntry> = - field_entries.iter().filter(|f| f.is_optional).collect(); - - // Generate required field checks - use different variable names to avoid conflicts - let required_field_checks: Vec = required_fields - .iter() - .map(|entry| { - let field_name = &entry.field_name; - let checked_field_name = format_ident!("{}_checked", field_name); - quote! { - let #checked_field_name = #field_name.ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml(format!( - "Missing {} in {}", - stringify!(#field_name), - stringify!(#struct_name) - )) - })?; - } - }) - .collect(); - - // Generate field assignments for struct construction - let field_assignments: Vec = field_entries - .iter() - .map(|entry| { - let field_name = &entry.field_name; - if entry.is_optional { - // Optional fields use their own value - quote! { #field_name } - } else { - // Required fields use the checked version - let checked_field_name = format_ident!("{}_checked", field_name); - quote! { #field_name: #checked_field_name } - } - }) - .collect(); - - quote! { - impl #impl_generics ironposh_xml::parser::XmlVisitor<'a> for #visitor_name #ty_generics #where_clause { - type Value = #struct_name #ty_generics; - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - for child in children { - if !child.is_element() { - continue; // Skip non-element nodes like text/whitespace - } - - let tag_name = child.tag_name().name(); - let namespace = child.tag_name().namespace(); - - match tag_name { - #(#match_arms)* - _ => { - // Warn about unknown tags instead of erroring - this allows SOAP faults - // with unknown namespaces (like WS-Eventing) to be parsed successfully - tracing::warn!( - target: "xml_parsing", - tag_name = tag_name, - namespace = ?namespace, - struct_name = stringify!(#struct_name), - "Unknown tag encountered during XML parsing, ignoring" - ); - } - } - } - - Ok(()) - } - - fn visit_node(&mut self, node: ironposh_xml::parser::Node<'a, 'a>) -> Result<(), ironposh_xml::XmlError> { - // Get the children and process them - let children: Vec<_> = node.children().collect(); - - self.visit_children(children.into_iter())?; - Ok(()) - } - - fn finish(self) -> Result { - let Self { #field_list } = self; - - // Validate required fields - #(#required_field_checks)* - - Ok(#struct_name { - #(#field_assignments),* - }) - } - } - } -} - fn is_option_type(ty: &Type) -> bool { if let Type::Path(TypePath { path, .. }) = ty { if let Some(segment) = path.segments.first() { @@ -1709,24 +1491,3 @@ fn extract_tag_name_from_tag_type(ty: &Type) -> Option { None } -fn generate_xml_deserialize_impl( - struct_name: &Ident, - visitor_name: &Ident, - generics: &Generics, -) -> TokenStream2 { - let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - - quote! { - impl #impl_generics ironposh_xml::parser::XmlDeserialize<'a> for #struct_name #ty_generics #where_clause { - type Visitor = #visitor_name #ty_generics; - - fn visitor() -> Self::Visitor { - #visitor_name::default() - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) - } - } - } -} diff --git a/crates/ironposh-winrm/src/cores/anytag.rs b/crates/ironposh-winrm/src/cores/anytag.rs index fe0046d..b331095 100644 --- a/crates/ironposh-winrm/src/cores/anytag.rs +++ b/crates/ironposh-winrm/src/cores/anytag.rs @@ -1,5 +1,3 @@ -use ironposh_xml::parser::{XmlDeserialize, XmlVisitor}; - use crate::{ cores::{Tag, TagList, TagName, tag_name::*, tag_value::Text}, rsp::{receive::ReceiveValue, shell_value::ShellValue}, @@ -7,7 +5,7 @@ use crate::{ #[macro_export] macro_rules! define_any_tag { - ($enum_name:ident, $visitor_name:ident, $(($variant:ident, $tag_name:ty, $tag_type:ty)),* $(,)?) => { + ($enum_name:ident, $(($variant:ident, $tag_name:ty, $tag_type:ty)),* $(,)?) => { #[expect(clippy::large_enum_variant)] #[derive(Debug, Clone)] pub enum $enum_name<'a> { @@ -45,58 +43,23 @@ macro_rules! define_any_tag { } } - pub struct $visitor_name<'a> { - tag: Option<$enum_name<'a>>, - } - - impl<'a> XmlVisitor<'a> for $visitor_name<'a> { - type Value = $enum_name<'a>; - - fn visit_children( - &mut self, - node: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { + impl<'a> ironposh_xml::mapping::FromXml<'a> for $enum_name<'a> { + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + use ironposh_xml::mapping::NodeExt; + // Dispatch by (namespace-URI, local-name) — prefix is irrelevant. + $( + if node.is_element_named(<$tag_name>::NAMESPACE, <$tag_name>::TAG_NAME) { + return Ok($enum_name::$variant( + <$tag_type as ironposh_xml::mapping::FromXml>::from_xml(node)?, + )); + } + )* Err(ironposh_xml::XmlError::InvalidXml(format!( - "Expected a single tag, found {} children", - node.count() + "Unknown tag: {} (namespace: {:?})", + node.tag_name().name(), + node.tag_name().namespace() ))) } - - fn visit_node(&mut self, node: ironposh_xml::parser::Node<'a, 'a>) -> Result<(), ironposh_xml::XmlError> { - match node.tag_name().name() { - $( - <$tag_name>::TAG_NAME => { - let tag = <$tag_type>::from_node(node)?; - self.tag = Some($enum_name::$variant(tag)); - } - )* - _ => { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Unknown tag: {}", - node.tag_name().name() - ))); - } - }; - - Ok(()) - } - - fn finish(self) -> Result { - self.tag - .ok_or(ironposh_xml::XmlError::InvalidXml("No valid tag found".to_string())) - } - } - - impl<'a> XmlDeserialize<'a> for $enum_name<'a> { - type Visitor = $visitor_name<'a>; - - fn visitor() -> Self::Visitor { - $visitor_name { tag: None } - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) - } } }; } @@ -108,7 +71,6 @@ macro_rules! define_any_tag { // while perserving the compile-time safety of tag definitions. define_any_tag!( AnyTag, - AnyTagVisitor, // SOAP elements (Envelope, Envelope, Tag<'a, TagList<'a>, Envelope>), (Header, Header, Tag<'a, TagList<'a>, Header>), diff --git a/crates/ironposh-winrm/src/cores/attribute.rs b/crates/ironposh-winrm/src/cores/attribute.rs index 75645d0..34fec05 100644 --- a/crates/ironposh-winrm/src/cores/attribute.rs +++ b/crates/ironposh-winrm/src/cores/attribute.rs @@ -19,7 +19,7 @@ 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 - fn from_name_and_value(name: &str, value: &'a str) -> Result, ironposh_xml::XmlError> { + pub fn from_name_and_value(name: &str, value: &'a str) -> Result, ironposh_xml::XmlError> { match name { $( $attr_name => { @@ -128,56 +128,6 @@ define_attributes!( // Add new attributes here and they automatically get handled everywhere! ); -pub struct AttributeVisitor<'a> { - attribute: Option>, -} - -impl<'a> ironposh_xml::parser::XmlVisitor<'a> for AttributeVisitor<'a> { - type Value = Attribute<'a>; - - fn visit_attribute( - &mut self, - _attribute: ironposh_xml::parser::Attribute<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - Attribute::from_name_and_value(_attribute.name(), _attribute.value()) - .map(|attr| { - if let Some(parsed_attr) = attr { - self.attribute = Some(parsed_attr); - } - }) - .map_err(|e| ironposh_xml::XmlError::InvalidXml(e.to_string())) - } - - fn finish(self) -> Result { - self.attribute - .ok_or_else(|| ironposh_xml::XmlError::InvalidXml("No attribute found".to_string())) - } -} - -impl<'a> ironposh_xml::parser::XmlDeserialize<'a> for Attribute<'a> { - type Visitor = AttributeVisitor<'a>; - - fn visitor() -> Self::Visitor { - AttributeVisitor { attribute: None } - } - - fn from_node( - _node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result { - Err(ironposh_xml::XmlError::InvalidXml( - "Attributes should not be parsed from nodes directly".to_string(), - )) - } - - fn from_children( - _children: impl Iterator>, - ) -> Result { - Err(ironposh_xml::XmlError::InvalidXml( - "Attributes should not be parsed from children directly".to_string(), - )) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/ironposh-winrm/src/cores/namespace.rs b/crates/ironposh-winrm/src/cores/namespace.rs index 73fd0c8..3c835ef 100644 --- a/crates/ironposh-winrm/src/cores/namespace.rs +++ b/crates/ironposh-winrm/src/cores/namespace.rs @@ -46,40 +46,6 @@ macro_rules! define_namespaces { } } - // ---------- XmlDeserialize support ----------------------------------- - pub struct NamespaceVisitor { namespace: Option } - - impl<'a> ironposh_xml::parser::XmlVisitor<'a> for NamespaceVisitor { - type Value = Namespace; - - fn visit_children( - &mut self, - _children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { Ok(()) } - - fn visit_node( - &mut self, - node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - let Some(ns) = node.tag_name().namespace() else { - return Err(ironposh_xml::XmlError::InvalidXml("No namespace found".into())); - }; - self.namespace = Some( - Namespace::try_from(ns) - .map_err(|_| ironposh_xml::XmlError::InvalidXml(format!("Unknown namespace: {ns}")))?, - ); - Ok(()) - } - - fn finish(self) -> Result { - self.namespace.ok_or_else(|| ironposh_xml::XmlError::InvalidXml("No namespace found".into())) - } - } - - impl<'a> ironposh_xml::parser::XmlDeserialize<'a> for Namespace { - type Visitor = NamespaceVisitor; - #[inline] fn visitor() -> Self::Visitor { NamespaceVisitor { namespace: None } } - } }; } @@ -125,46 +91,14 @@ impl IntoIterator for NamespaceDeclaration { } } -pub struct NamespaceDeclarationVisitor { - namespaces: Vec, -} - -impl<'a> ironposh_xml::parser::XmlVisitor<'a> for NamespaceDeclarationVisitor { - type Value = NamespaceDeclaration; - - fn visit_children( - &mut self, - _c: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - Ok(()) - } - - fn visit_node( - &mut self, - node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - for ns in node.namespaces() { - self.namespaces.push(Namespace::try_from(ns).map_err(|_| { - ironposh_xml::XmlError::InvalidXml(format!("Unknown namespace: {ns:?}")) - })?); - } - Ok(()) - } - - fn finish(self) -> Result { - Ok(NamespaceDeclaration(self.namespaces)) - } -} - -impl<'a> ironposh_xml::parser::XmlDeserialize<'a> for NamespaceDeclaration { - type Visitor = NamespaceDeclarationVisitor; - #[inline] - fn visitor() -> Self::Visitor { - NamespaceDeclarationVisitor { - namespaces: Vec::new(), - } - } - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) +impl<'a> ironposh_xml::mapping::FromXml<'a> for NamespaceDeclaration { + /// Captures the namespaces declared on this element. Declarations we don't + /// recognize are skipped — they don't affect matching, which compares URIs. + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + let namespaces = node + .namespaces() + .filter_map(|ns| Namespace::try_from(ns).ok()) + .collect(); + Ok(NamespaceDeclaration(namespaces)) } } diff --git a/crates/ironposh-winrm/src/cores/tag.rs b/crates/ironposh-winrm/src/cores/tag.rs index 05a16c2..1388026 100644 --- a/crates/ironposh-winrm/src/cores/tag.rs +++ b/crates/ironposh-winrm/src/cores/tag.rs @@ -1,10 +1,9 @@ use ironposh_xml::builder::Element; -use ironposh_xml::parser::{XmlDeserialize, XmlVisitor}; -use tracing::{debug, trace, warn}; +use ironposh_xml::mapping::{FromXml, NodeExt}; -use crate::cores::namespace::NamespaceDeclaration; +use crate::cores::WsUuid; +use crate::cores::namespace::{Namespace, NamespaceDeclaration}; use crate::cores::tag_value::{Text, U32}; -use crate::cores::{Namespace, WsUuid}; use crate::impl_tag_from; use super::attribute::Attribute; @@ -160,155 +159,40 @@ where } } -pub struct TagVisitor<'a, V, N> +impl<'a, V, N> FromXml<'a> for Tag<'a, V, N> where - V: TagValue<'a>, - N: TagName, -{ - pub tag: Option, - pub attributes: Vec>, - pub namespaces: NamespaceDeclaration, - pub namespace: Option, - __phantom: std::marker::PhantomData<&'a N>, -} - -pub struct NodeDeserializer<'a> { - root: ironposh_xml::parser::Node<'a, 'a>, -} - -impl<'a> NodeDeserializer<'a> { - pub fn new(root: ironposh_xml::parser::Node<'a, 'a>) -> Self { - Self { root } - } - - /// Drive any visitor over the subtree rooted at `self.root` - pub fn deserialize(self, mut visitor: V) -> Result - where - V: XmlVisitor<'a>, - { - visitor.visit_node(self.root)?; - visitor.finish() - } -} - -impl<'a, V, N> XmlVisitor<'a> for TagVisitor<'a, V, N> -where - V: TagValue<'a> + 'a + XmlDeserialize<'a>, + V: TagValue<'a> + FromXml<'a>, N: TagName, { - type Value = Tag<'a, V, N>; - - fn visit_node( - &mut self, - node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - trace!( - expected_tag_name = N::TAG_NAME, - actual_tag_name = node.tag_name().name(), - namespace = ?node.tag_name().namespace(), - "TagVisitor visiting node", - ); - - if node.is_element() && node.tag_name().name() == N::TAG_NAME { - let value = - V::from_children(node.children().filter(|c| c.is_element() || c.is_text()))?; - self.tag = Some(value); - trace!(tag_name = N::TAG_NAME, "Successfully created tag value"); - } else { - warn!( - actual_tag_name = node.tag_name().name(), - expected_tag_name = N::TAG_NAME, - "Tag name doesn't match or node is not an element" - ); - } - - for attr in node.attributes() { - trace!( - name = attr.name(), - value = attr.value(), - "Processing attribute" - ); - if let Ok(attribute) = Attribute::from_attribute(attr) { - trace!("Successfully parsed attribute: {:?}", attribute); - self.attributes.push(attribute); - } else { - debug!("Failed to parse attribute: {}", attr.name()); - } + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + // Identity is the (namespace-URI, local-name) pair; the prefix is never + // consulted. A dispatcher (a derived struct, AnyTag, or a TagList) has + // usually already matched this, so the check is a cheap self-validation. + if !node.is_element_named(N::NAMESPACE, N::TAG_NAME) { + return Err(ironposh_xml::XmlError::XmlInvalidTag { + expected: N::TAG_NAME.to_string(), + found: node.tag_name().name().to_string(), + }); } - self.namespaces = NamespaceDeclaration::from_node(node)?; - - self.namespace = Namespace::from_node(node).ok(); - - Ok(()) - } - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - for child in children { - if child.is_element() - && child.tag_name().name() == N::TAG_NAME - && child.tag_name().namespace() == N::NAMESPACE - { - debug!("Visiting child node: {}", child.tag_name().name()); - self.visit_node(child)?; - } else { - warn!( - "Skipping child node: {} (namespace: {:?})", - child.tag_name().name(), - child.tag_name().namespace() - ); - - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Unexpected child node: {} (namespace: {:?})", - child.tag_name().name(), - child.tag_name().namespace() - ))); - } - } - - Ok(()) - } - - fn finish(self) -> Result { - self.tag - .map(|value| Tag { - value, - attributes: self.attributes, - namespaces_declaration: self.namespaces, - __phantom: std::marker::PhantomData, - __phantom_name: std::marker::PhantomData, - }) - .ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml(format!( - "Tag visitor cannot built for tag: {}", - N::TAG_NAME - )) + let value = V::from_xml(node)?; + let attributes = node + .attributes() + .filter_map(|attr| { + Attribute::from_name_and_value(attr.name(), attr.value()) + .ok() + .flatten() }) - } -} - -impl<'a, V, N> XmlDeserialize<'a> for Tag<'a, V, N> -where - V: TagValue<'a> + XmlDeserialize<'a>, - N: TagName + 'a, -{ - type Visitor = TagVisitor<'a, V, N>; + .collect(); + let namespaces_declaration = NamespaceDeclaration::from_xml(node)?; - fn visitor() -> Self::Visitor { - TagVisitor { - tag: None, - attributes: Vec::new(), - namespaces: NamespaceDeclaration::new(), - namespace: None, + Ok(Tag { + value, + attributes, + namespaces_declaration, __phantom: std::marker::PhantomData, - } - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - NodeDeserializer::new(node).deserialize(Self::visitor()) + __phantom_name: std::marker::PhantomData, + }) } } diff --git a/crates/ironposh-winrm/src/cores/tag_list.rs b/crates/ironposh-winrm/src/cores/tag_list.rs index 8ec876b..f8d98bb 100644 --- a/crates/ironposh-winrm/src/cores/tag_list.rs +++ b/crates/ironposh-winrm/src/cores/tag_list.rs @@ -1,7 +1,4 @@ -use ironposh_xml::{ - builder::Element, - parser::{XmlDeserialize, XmlVisitor}, -}; +use ironposh_xml::{builder::Element, mapping::FromXml}; use crate::cores::{TagValue, anytag::AnyTag}; @@ -37,49 +34,14 @@ impl<'a> TagValue<'a> for TagList<'a> { } } -pub struct TagListVisitor<'a> { - items: Vec>, -} - -impl<'a> XmlVisitor<'a> for TagListVisitor<'a> { - type Value = TagList<'a>; - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - for child in children { +impl<'a> FromXml<'a> for TagList<'a> { + fn from_xml(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { + let mut items = Vec::new(); + for child in node.children() { if child.is_element() { - let tag = AnyTag::from_node(child)?; - self.items.push(tag); - } else { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Expected element child, found: {:?}", - child.node_type() - ))); + items.push(AnyTag::from_xml(child)?); } } - Ok(()) - } - - fn visit_node( - &mut self, - _node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - Err(ironposh_xml::XmlError::InvalidXml( - "TagListVisitor should not be called with a single node".to_string(), - )) - } - - fn finish(self) -> Result { - Ok(TagList { items: self.items }) - } -} - -impl<'a> XmlDeserialize<'a> for TagList<'a> { - type Visitor = TagListVisitor<'a>; - - fn visitor() -> Self::Visitor { - TagListVisitor { items: Vec::new() } + Ok(TagList { items }) } } diff --git a/crates/ironposh-winrm/src/cores/tag_name.rs b/crates/ironposh-winrm/src/cores/tag_name.rs index bc36867..4d56dad 100644 --- a/crates/ironposh-winrm/src/cores/tag_name.rs +++ b/crates/ironposh-winrm/src/cores/tag_name.rs @@ -37,7 +37,11 @@ define_tagname!(ShellInactivity, Some(Namespace::WsmanShell.uri())); define_tagname!(CompressionType, Some(Namespace::WsmanShell.uri())); define_tagname!(DesiredStream, Some(Namespace::WsmanShell.uri())); -define_custom_tagname!(CreationXml, "creationXml", None); +define_custom_tagname!( + CreationXml, + "creationXml", + Some(Namespace::PowerShellRemoting.uri()) +); define_tagname!(CommandLine, Some(Namespace::WsmanShell.uri())); define_tagname!(Shell, Some(Namespace::WsmanShell.uri())); @@ -56,8 +60,16 @@ define_tagname!(Reconnect, Some(Namespace::WsmanShell.uri())); define_tagname!(ReconnectResponse, Some(Namespace::WsmanShell.uri())); define_tagname!(Connect, Some(Namespace::WsmanShell.uri())); define_tagname!(ConnectResponse, Some(Namespace::WsmanShell.uri())); -define_custom_tagname!(ConnectXml, "connectXml", None); -define_custom_tagname!(ConnectResponseXml, "connectResponseXml", None); +define_custom_tagname!( + ConnectXml, + "connectXml", + Some(Namespace::PowerShellRemoting.uri()) +); +define_custom_tagname!( + ConnectResponseXml, + "connectResponseXml", + Some(Namespace::PowerShellRemoting.uri()) +); define_tagname!(Signal, Some(Namespace::WsmanShell.uri())); define_tagname!(SignalResponse, Some(Namespace::WsmanShell.uri())); define_custom_tagname!(SignalCode, "Code", Some(Namespace::WsmanShell.uri())); diff --git a/crates/ironposh-winrm/src/cores/tag_value.rs b/crates/ironposh-winrm/src/cores/tag_value.rs index ff0bffd..5ee5a09 100644 --- a/crates/ironposh-winrm/src/cores/tag_value.rs +++ b/crates/ironposh-winrm/src/cores/tag_value.rs @@ -1,9 +1,6 @@ use std::borrow::Cow; -use ironposh_xml::{ - builder::Element, - parser::{Node, XmlDeserialize, XmlVisitor}, -}; +use ironposh_xml::{XmlError, builder::Element, mapping::FromXml, parser::Node}; use crate::xml_num_value; @@ -50,130 +47,33 @@ impl<'a> TagValue<'a> for Text<'a> { } } -impl<'a> TagValue<'a> for () { - fn append_to_element(self, element: Element<'a>) -> Element<'a> { - element - } -} - -pub struct TextVisitor<'a> { - value: Option>, -} - -impl<'a> XmlVisitor<'a> for TextVisitor<'a> { - type Value = Text<'a>; - - fn visit_node( - &mut self, - _node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - Ok(()) - } - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - let child_nodes: Vec<_> = children.collect(); - - // Validate there's only one child node - if child_nodes.len() != 1 { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Expected exactly one text node, found {} children", - child_nodes.len() - ))); - } - - let child = child_nodes.first().ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml("Expected at least one child node".to_string()) - })?; - - // Validate that child node is a text node - if !child.is_text() { - return Err(ironposh_xml::XmlError::InvalidXml( - "Expected text node, found non-text child".to_string(), - )); - } - - if let Some(text) = child.text() { - self.value = Some(Text(text.trim().into())); - } - - Ok(()) - } - - fn finish(self) -> Result { - self.value.ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml("No text found in the node".to_string()) - }) - } -} - -impl<'a> XmlDeserialize<'a> for Text<'a> { - type Visitor = TextVisitor<'a>; - - fn visitor() -> Self::Visitor { - TextVisitor { value: None } - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) +impl<'a> FromXml<'a> for Text<'a> { + fn from_xml(node: Node<'a, 'a>) -> Result { + Ok(Text(node.text().unwrap_or("").trim().into())) } } -pub struct EmptyVisitor; - -impl<'a> XmlVisitor<'a> for EmptyVisitor { - type Value = Empty; - - fn visit_node( - &mut self, - _node: ironposh_xml::parser::Node<'a, 'a>, - ) -> Result<(), ironposh_xml::XmlError> { - Ok(()) - } - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - let child_count = children.count(); - - if child_count != 0 { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Expected empty tag with no children, found {child_count} children" - ))); - } - - Ok(()) - } - - fn finish(self) -> Result { - Ok(Empty) +impl<'a> TagValue<'a> for () { + fn append_to_element(self, element: Element<'a>) -> Element<'a> { + element } } #[derive(Debug, Clone, PartialEq, Eq)] pub struct Empty; -impl<'a> XmlDeserialize<'a> for Empty { - type Visitor = EmptyVisitor; - - fn visitor() -> Self::Visitor { - EmptyVisitor - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) - } -} - impl<'a> TagValue<'a> for Empty { fn append_to_element(self, element: Element<'a>) -> Element<'a> { element } } +impl<'a> FromXml<'a> for Empty { + fn from_xml(_node: Node<'a, 'a>) -> Result { + Ok(Empty) + } +} + impl From<()> for Empty { fn from(_value: ()) -> Self { Self @@ -194,74 +94,14 @@ impl<'a> TagValue<'a> for WsUuid { } } -pub struct WsUuidVisitor { - value: Option, -} - -impl<'a> XmlVisitor<'a> for WsUuidVisitor { - type Value = WsUuid; - - fn visit_children( - &mut self, - children: impl Iterator>, - ) -> Result<(), ironposh_xml::XmlError> { - let child_nodes: Vec<_> = children.collect(); - - // Validate there's only one child node - if child_nodes.len() != 1 { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Expected exactly one text node, found {} children", - child_nodes.len() - ))); - } - - let child = child_nodes.first().ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml("Expected at least one child node".to_string()) - })?; - - // Validate that child node is a text node - if !child.is_text() { - return Err(ironposh_xml::XmlError::InvalidXml( - "Expected text node, found non-text child".to_string(), - )); - } - - if let Some(text) = child.text() { - let uuid_str = text.trim(); - // Handle WS-Management format: "uuid:9EC885D6-F5A4-4771-9D47-4BDF7DAAEA8C" - let uuid_part = uuid_str - .strip_prefix("uuid:") - .map_or(uuid_str, |stripped| stripped); - - match uuid::Uuid::parse_str(uuid_part) { - Ok(uuid) => self.value = Some(WsUuid(uuid)), - Err(_) => { - return Err(ironposh_xml::XmlError::InvalidXml(format!( - "Invalid UUID format: {uuid_str}" - ))); - } - } - } - - Ok(()) - } - - fn finish(self) -> Result { - self.value.ok_or_else(|| { - ironposh_xml::XmlError::InvalidXml("No UUID found in the node".to_string()) - }) - } -} - -impl<'a> XmlDeserialize<'a> for WsUuid { - type Visitor = WsUuidVisitor; - - fn visitor() -> Self::Visitor { - WsUuidVisitor { value: None } - } - - fn from_node(node: ironposh_xml::parser::Node<'a, 'a>) -> Result { - ironposh_xml::parser::NodeDeserializer::new(node).deserialize(Self::visitor()) +impl<'a> FromXml<'a> for WsUuid { + fn from_xml(node: Node<'a, 'a>) -> Result { + let text = node.text().unwrap_or("").trim(); + // WS-Management prefixes UUIDs with "uuid:" — strip it if present. + let raw = text.strip_prefix("uuid:").unwrap_or(text); + uuid::Uuid::parse_str(raw) + .map(WsUuid) + .map_err(|_| XmlError::InvalidXml(format!("Invalid UUID format: {text}"))) } } @@ -292,89 +132,17 @@ impl<'a> TagValue<'a> for Time { } } -pub struct TimeVisitor { - value: Option