From daf71c51e0879b9e5dda57fdca0465df7d0c3b33 Mon Sep 17 00:00:00 2001
From: Jordan Borean <jborean93@gmail.com>
Date: Thu, 11 Apr 2024 05:27:26 +1000
Subject: [PATCH] winrm - Handle task timeout (#82784) (#82864)

When using winrm over HTTP with message encryption enabled and a task
has timed out the connection plugin will fail to cleanup the WinRM
command. This will change that exception into a warning as a timeout is
already an exception event and a failure to clean the operation should
not override the timeout error shown.

(cherry picked from commit 8aecd1f9b22f028feaf2cb4aba3ce6baca548853)
---
 changelogs/fragments/winrm-task-timeout.yml      |  2 ++
 lib/ansible/plugins/connection/winrm.py          | 16 ++++++++++++++--
 .../targets/connection_windows_ssh/tests.yml     | 12 ++++++++++++
 .../targets/connection_winrm/tests.yml           | 12 ++++++++++++
 4 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/winrm-task-timeout.yml

diff --git a/changelogs/fragments/winrm-task-timeout.yml b/changelogs/fragments/winrm-task-timeout.yml
new file mode 100644
index 00000000000..305957bf8a6
--- /dev/null
+++ b/changelogs/fragments/winrm-task-timeout.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- winrm - Do not raise another exception during cleanup when a task is timed out - https://github.com/ansible/ansible/issues/81095
diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py
index 7104369ab47..b2974959bbc 100644
--- a/lib/ansible/plugins/connection/winrm.py
+++ b/lib/ansible/plugins/connection/winrm.py
@@ -199,7 +199,7 @@ from ansible.utils.display import Display
 
 try:
     import winrm
-    from winrm.exceptions import WinRMError, WinRMOperationTimeoutError
+    from winrm.exceptions import WinRMError, WinRMOperationTimeoutError, WinRMTransportError
     from winrm.protocol import Protocol
     import requests.exceptions
     HAS_WINRM = True
@@ -684,7 +684,19 @@ class Connection(ConnectionBase):
             raise AnsibleConnectionFailure('winrm connection error: %s' % to_native(exc))
         finally:
             if command_id:
-                self.protocol.cleanup_command(self.shell_id, command_id)
+                # Due to a bug in how pywinrm works with message encryption we
+                # ignore a 400 error which can occur when a task timeout is
+                # set and the code tries to clean up the command. This happens
+                # as the cleanup msg is sent over a new socket but still uses
+                # the already encrypted payload bound to the other socket
+                # causing the server to reply with 400 Bad Request.
+                try:
+                    self.protocol.cleanup_command(self.shell_id, command_id)
+                except WinRMTransportError as e:
+                    if e.code != 400:
+                        raise
+
+                    display.warning("Failed to cleanup running WinRM command, resources might still be in use on the target server")
 
     def _connect(self) -> Connection:
 
diff --git a/test/integration/targets/connection_windows_ssh/tests.yml b/test/integration/targets/connection_windows_ssh/tests.yml
index e9b538b40a2..3b09f620367 100644
--- a/test/integration/targets/connection_windows_ssh/tests.yml
+++ b/test/integration/targets/connection_windows_ssh/tests.yml
@@ -30,3 +30,15 @@
       - win_ssh_async.rc == 0
       - win_ssh_async.stdout == "café\n"
       - win_ssh_async.stderr == ""
+
+  # Ensures the connection plugin can handle a timeout
+  # without raising another error.
+  - name: run command with timeout
+    win_shell: Start-Sleep -Seconds 10
+    timeout: 5
+    register: timeout_cmd
+    ignore_errors: true
+
+  - assert:
+      that:
+      - timeout_cmd.msg == 'The win_shell action failed to execute in the expected time frame (5) and was terminated'
diff --git a/test/integration/targets/connection_winrm/tests.yml b/test/integration/targets/connection_winrm/tests.yml
index b086a3ad1a1..cf109a8c6cd 100644
--- a/test/integration/targets/connection_winrm/tests.yml
+++ b/test/integration/targets/connection_winrm/tests.yml
@@ -29,3 +29,15 @@
       that:
       - winrm_copy_empty is changed
       - winrm_copy_empty_actual.stat.size == 0
+
+  # Ensures the connection plugin can handle a timeout
+  # without raising another error.
+  - name: run command with timeout
+    win_shell: Start-Sleep -Seconds 10
+    timeout: 5
+    register: timeout_cmd
+    ignore_errors: true
+
+  - assert:
+      that:
+      - timeout_cmd.msg == 'The win_shell action failed to execute in the expected time frame (5) and was terminated'