From 306be9f23c0c426d16dad3a4e3abee9cdee72ce2 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Mon, 23 Jul 2018 12:20:14 -0400 Subject: [PATCH] Fix pause module so it does not stack trace when redirecting stdout. (#42217) (#42441) * Use separate variables for stdin and stdout file descriptors * Do not set stdout to raw mode when output is not a TTY (cherry picked from commit 1d1595b990641b6a3446c6a94ddf992ef9cfac39) --- .../fragments/pause-stdout-redirection.yaml | 2 + lib/ansible/plugins/action/pause.py | 44 +++++++++++-------- test/integration/targets/pause/runme.sh | 6 +++ 3 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/pause-stdout-redirection.yaml diff --git a/changelogs/fragments/pause-stdout-redirection.yaml b/changelogs/fragments/pause-stdout-redirection.yaml new file mode 100644 index 00000000000..4f420f885bc --- /dev/null +++ b/changelogs/fragments/pause-stdout-redirection.yaml @@ -0,0 +1,2 @@ +bugfixes: + - pause - do not set stdout to raw mode when redirecting to a file (https://github.com/ansible/ansible/issues/41717) diff --git a/lib/ansible/plugins/action/pause.py b/lib/ansible/plugins/action/pause.py index bdf9c9a51c8..abece3adf1b 100644 --- a/lib/ansible/plugins/action/pause.py +++ b/lib/ansible/plugins/action/pause.py @@ -143,7 +143,7 @@ class ActionModule(ActionBase): result['start'] = to_text(datetime.datetime.now()) result['user_input'] = b'' - fd = None + stdin_fd = None old_settings = None try: if seconds is not None: @@ -167,7 +167,8 @@ class ActionModule(ActionBase): # save the attributes on the existing (duped) stdin so # that we can restore them later after we set raw mode - fd = None + stdin_fd = None + stdout_fd = None try: if PY3: stdin = self._connection._new_stdin.buffer @@ -175,52 +176,59 @@ class ActionModule(ActionBase): else: stdin = self._connection._new_stdin stdout = sys.stdout - fd = stdin.fileno() + stdin_fd = stdin.fileno() + stdout_fd = stdout.fileno() except (ValueError, AttributeError): # ValueError: someone is using a closed file descriptor as stdin # AttributeError: someone is using a null file descriptor as stdin on windoez stdin = None - if fd is not None: - if isatty(fd): - + if stdin_fd is not None: + if isatty(stdin_fd): # grab actual Ctrl+C sequence try: - intr = termios.tcgetattr(fd)[6][termios.VINTR] + intr = termios.tcgetattr(stdin_fd)[6][termios.VINTR] except Exception: # unsupported/not present, use default intr = b'\x03' # value for Ctrl+C # get backspace sequences try: - backspace = termios.tcgetattr(fd)[6][termios.VERASE] + backspace = termios.tcgetattr(stdin_fd)[6][termios.VERASE] except Exception: backspace = [b'\x7f', b'\x08'] - old_settings = termios.tcgetattr(fd) - tty.setraw(fd) - tty.setraw(stdout.fileno()) + old_settings = termios.tcgetattr(stdin_fd) + tty.setraw(stdin_fd) + + # Only set stdout to raw mode if it is a TTY. This is needed when redirecting + # stdout to a file since a file cannot be set to raw mode. + if isatty(stdout_fd): + tty.setraw(stdout_fd) # Only echo input if no timeout is specified if not seconds and echo: - new_settings = termios.tcgetattr(fd) + new_settings = termios.tcgetattr(stdin_fd) new_settings[3] = new_settings[3] | termios.ECHO - termios.tcsetattr(fd, termios.TCSANOW, new_settings) + termios.tcsetattr(stdin_fd, termios.TCSANOW, new_settings) # flush the buffer to make sure no previous key presses # are read in below termios.tcflush(stdin, termios.TCIFLUSH) while True: + try: - if fd is not None: + if stdin_fd is not None: + key_pressed = stdin.read(1) + if key_pressed == intr: # value for Ctrl+C clear_line(stdout) raise KeyboardInterrupt if not seconds: - if fd is None or not isatty(fd): + if stdin_fd is None or not isatty(stdin_fd): display.warning("Not waiting for response to prompt as stdin is not interactive") break @@ -255,9 +263,9 @@ class ActionModule(ActionBase): pass finally: # cleanup and save some information - # restore the old settings for the duped stdin fd - if not(None in (fd, old_settings)) and isatty(fd): - termios.tcsetattr(fd, termios.TCSADRAIN, old_settings) + # restore the old settings for the duped stdin stdin_fd + if not(None in (stdin_fd, old_settings)) and isatty(stdin_fd): + termios.tcsetattr(stdin_fd, termios.TCSADRAIN, old_settings) duration = time.time() - start result['stop'] = to_text(datetime.datetime.now()) diff --git a/test/integration/targets/pause/runme.sh b/test/integration/targets/pause/runme.sh index aeda654bd80..51016dd3591 100755 --- a/test/integration/targets/pause/runme.sh +++ b/test/integration/targets/pause/runme.sh @@ -15,6 +15,12 @@ ansible-playbook test-pause-no-tty.yml -i ../../inventory 2>&1 | \ } EOF +# Test redirecting stdout +# Issue #41717 +ansible-playbook pause-3.yml -i ../../inventory > /dev/null \ + && echo "Successfully redirected stdout" \ + || echo "Failure when attempting to redirect stdout" + # Test pause with seconds and minutes specified ansible-playbook test-pause.yml -i ../../inventory "$@"