Skip to content

Stack 4/5: Session Disconnect / Reconnect (same client)#17

Closed
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 5 commits into
stack/3-jea-configfrom
stack/4-disconnect-reconnect
Closed

Stack 4/5: Session Disconnect / Reconnect (same client)#17
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 5 commits into
stack/3-jea-configfrom
stack/4-disconnect-reconnect

Conversation

@irvingoujAtDevolution

Copy link
Copy Markdown
Collaborator

Stack 4/5 — Session Disconnect / Reconnect (same client)

Base: stack/3-jea-config · Review PRs 1–3 first.

What this PR does

PSRP sessions survive client disconnection (MS-WSMV Disconnect/Reconnect). The state enums existed; the operations did not.

  • ironposh-winrm: WsAction::Disconnect/Reconnect + body types + response parsing.
  • ironposh-client-core: RunspacePool fire/accept state transitions with guards (Disconnecting = 10), fault surfacing, connection-id fault correlation.
  • ironposh-async + tokio REPL: disconnect()/reconnect() client methods, a PoolLifecycleEvent channel, :disconnect/:reconnect REPL commands.
  • Review-driven hardening: REPL no longer soft-locks after disconnect; mistimed :reconnect is non-fatal instead of killing the session; faulted Disconnect reverts to Opened instead of hanging; mid-pipeline reconnect resumes the pipeline.

Core / web exposure

  • client-core: new disconnect/reconnect methods + routing that only activates in disconnect-related pool states; normal Opened-state handling untouched.
  • web: none (web reattach deferred).

Verification

fmt / clippy / workspace tests green. Disconnect→reconnect validated live ($x = 42 survives a disconnect/reconnect cycle).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings:

  • [P1] Accept header-only Disconnect/Reconnect responses in this PR. The new accept_disconnect_response / accept_reconnect_response code requires <rsp:DisconnectResponse/> and <rsp:ReconnectResponse/> body elements, but real Windows WinRM responses can have an empty SOAP Body and identify the operation only through the a:Action header. In this branch, the empty-body DisconnectResponse is classified as InvalidResponse and ignored as “non-disconnect traffic,” leaving the pool stuck in Disconnecting; the same shape for Reconnect fails outright. The header-action fallback that appears later in the stack should move down here, with tests, because this PR is the one introducing disconnect/reconnect behavior.

@irvingoujAtDevolution

Copy link
Copy Markdown
Collaborator Author

Addressed in 27895e3 (now this PR's tip). The header-action fallback was moved down here: accept_disconnect_response / accept_reconnect_response now accept a response when either the rsp:*Response body element is present or the a:Action header matches the corresponding response action, so real Windows WinRM empty-body responses no longer leave the pool stuck in Disconnecting. Tests: accept_disconnect_response_accepts_empty_body_with_action_header, the reconnect equivalent, and accept_disconnect_response_rejects_unrelated_action_with_empty_body.

…l servers

Live Windows WinRM answers shell Disconnect/Reconnect with an empty
s:Body and identifies the operation solely via the a:Action header
(.../shell/DisconnectResponse, .../shell/ReconnectResponse), unlike the
documented rsp:*Response body element our unit fixtures assumed. The
parser required the body element, so the real response was classified
as tolerated non-disconnect traffic and the runspace pool stayed stuck
in Disconnecting forever.

- add WsAction::DisconnectResponse / ReconnectResponse URIs
- accept_disconnect_response / accept_reconnect_response now also match
  on the Action header (body element still accepted)
- log the tolerated body in the disconnecting tolerance path for future
  wire-level diagnosis
- unit tests encode the captured live-server envelope shape, plus a
  negative test keeping unrelated traffic rejected

Validated live: disconnect_reconnect_e2e and reattach_e2e both pass
against a real WinRM server.
@irvingoujAtDevolution

Copy link
Copy Markdown
Collaborator Author

Superseded by #18, retargeted to master to land the full stack as a single merge now that review is complete. Same commits; no code lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant