test: negative FromXml rejection tests + command-not-found e2e#38
Merged
irvingouj@Devolutions (irvingoujAtDevolution) merged 4 commits intoJun 25, 2026
Merged
Conversation
Review-loop findings (Claude subagents + Codex, converged). The visitor→FromXml migration had relaxed several validations the old visitors enforced; this restores strict rejection of malformed/untrusted input across the parse layer: - `leaf_text()` helper: text-valued leaves (Text/WsUuid/Time/numerics) reject any non-text child (mixed content, comments, PIs) instead of silently truncating via `node.text()`. `Empty` rejects any non-whitespace content. - `Tag::from_xml` wrapper-descend (`Tag<Tag<..>,_>`): requires exactly one child element which must be `N` — rejects extra siblings, wrong child, duplicates. - `#[derive(FromXml)]`: `if … else if …` matcher chain; rejects a duplicate element for a singleton field; reports shape errors via `syn::Error` not `panic!`; doc no longer references the deleted `SimpleXmlDeserialize`. - Hand-written impls made consistent: commandline (leaf_text; empty `<Command/>` → None via a seen flag; duplicate `<Command>` rejected), receive (duplicate `<CommandState>` rejected), SelectorSet/OptionSet (leaf_text; duplicate Name rejected). - Comments: flagged the deliberate `Send` alias / `std::marker::Send` shadow and the `Locale`/`DataLocale` empty-vs-text deserialize-collapse invariant. Trimming in `leaf_text` is intentional and kept (original leaf behavior; strips pretty-printed-XML indentation; WinRM values carry no significant edge whitespace) — a Codex suggestion to drop it was declined as a regression. Verified: fmt + clippy (-D warnings) clean, workspace tests pass, real-server matrix 10/10.
Pin the rejection behavior the hardening round added: mixed-content leaf rejection, Empty non-empty rejection, single-child descend (zero/multiple/ wrong-name), duplicate singleton (Command/CommandState/derived ExitCode), and duplicate selector/option keys.
A non-existent command surfaces as an error record (non-fatal), printed via render_concise to stdout while the process still exits 0. Asserts both, across all auth methods.
Polish from the independent review pass (no behavior change except a clearer error message): remove dead commented-out visitor block in ws_addressing; correct the tag! marker-visibility doc; drop two what/how comments; loosen the paste version pin; clearer 'found none' diagnostic for an empty tag wrapper; restore optional-permutation derive tests and add mixed-content rejection tests for numeric/uuid leaves.
irvingouj@Devolutions (irvingoujAtDevolution)
changed the base branch from
stack/xml-fromxml-harden
to
master
irvingouj@Devolutions (irvingoujAtDevolution)
deleted the
stack/xml-fromxml-tests
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 6/6. Base:
stack/xml-fromxml-harden.Pins the rejection behavior from #5 with negative unit tests (mixed-content leaf, Empty non-empty, single-child descend zero/multiple/wrong-name, duplicate Command/CommandState/derived ExitCode, duplicate selector/option keys) and adds a command-not-found case to the real-server command matrix e2e (asserts the error record prints to stdout and the process still exits 0).
FromXml stack (review/merge bottom-up):
tag!macro + aliases (refactor(winrm): alias-based tags via tag! macro #36)