fix(winrm): descend into child for a tag whose value is a tag#34
Merged
irvingouj@Devolutions (irvingoujAtDevolution) merged 4 commits intoJun 25, 2026
Merged
Conversation
…sitor) 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.
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<Self>`, 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.
- rustfmt: import ordering/grouping on the touched files. - clippy (-D warnings): drop the redundant `continue` from the FromXml derive output; merge the identical default-namespace match arms in the Element writer; use `Self` in the hand-written FromXml impls.
`Tag<Tag<W, M>, N>` (e.g. `command_response: <CommandResponse><CommandId/>`,
`signal: <Signal><SignalCode/>`) models an N element wrapping a single M child.
The new FromXml passed the parent node straight to the inner tag's from_xml,
which then checked the wrapper's name and failed
("Invalid tag: expected 'CommandId', found 'CommandResponse'").
Tag::from_xml now descends to the single N-named child when handed a node that
isn't itself N — restoring the old from_children semantics for nested tags.
This only surfaced on SSPI auth (Negotiate/Kerberos/NTLM), which exchanges
CommandId; Basic never hits that path, so unit fixtures missed it. Added a
unit regression test for the nested CommandResponse/CommandId shape.
Verified: real-server transport×auth×sealing matrix now 10/10 (was 4/10 on
this branch; master is 10/10). fmt + clippy + workspace tests all clean.
This was referenced Jun 25, 2026
irvingouj@Devolutions (irvingoujAtDevolution)
changed the base branch from
stack/xml-fromxml-core-migrate
to
master
irvingouj@Devolutions (irvingoujAtDevolution)
deleted the
stack/xml-fromxml-nested-fix
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2/6. Base:
stack/xml-fromxml-core-migrate(#33).Fixes the
Tag<Tag<..>, _>wrapper case (e.g.<CommandResponse>wrapping<CommandId>):from_xmlmust descend to the single named child rather than parse the wrapper as the inner tag. This was the SSPI e2e regression (matrix 4/10 → 10/10).FromXml stack (review/merge bottom-up):
tag!macro + aliases