From e023fd2d61b9c438b1992e5f90739b75c82f35b9 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 28 Oct 2022 18:26:03 -0400 Subject: [PATCH] remove any pipe files before zip/unzipping (#1133) * Write test to demonstrate job hang on pipes * Remove any pipe files before zip/unzipping - Prevents hangs when zipping, unzipping artifacts dir. If python attempts to open a fifo pipe, it will block indefinitely until it can read data from the pipe. * move fifo check below symlink Co-authored-by: Alan Rominger (cherry picked from commit 8f39752dc7e6d271bb3e3b11d507050489a9ad02) --- ansible_runner/utils/streaming.py | 11 ++++++++ test/unit/utils/test_utils.py | 45 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/ansible_runner/utils/streaming.py b/ansible_runner/utils/streaming.py index 7715e9576..92a708fbc 100644 --- a/ansible_runner/utils/streaming.py +++ b/ansible_runner/utils/streaming.py @@ -32,6 +32,11 @@ def stream_dir(source_directory, stream): permissions |= 0xA000 zip_info.external_attr = permissions << 16 archive.writestr(zip_info, os.readlink(full_path)) + elif stat.S_ISFIFO(os.stat(full_path).st_mode): + # skip any pipes, as python hangs when attempting + # to open them. + # i.e. ssh_key_data that was never cleaned up + continue else: archive.write( os.path.join(dirpath, fname), arcname=os.path.join(relpath, fname) @@ -81,6 +86,12 @@ def unstream_dir(stream, length, target_directory): if os.path.exists(out_path): if is_symlink: os.remove(out_path) + elif stat.S_ISFIFO(os.stat(out_path).st_mode): + # remove any pipes, as python hangs when attempting + # to open them. + # i.e. ssh_key_data that was never cleaned up + os.remove(out_path) + continue elif os.path.isdir(out_path): # Special case, the important dirs were pre-created so don't try to chmod them continue diff --git a/test/unit/utils/test_utils.py b/test/unit/utils/test_utils.py index b83441e6c..5e165e7ed 100644 --- a/test/unit/utils/test_utils.py +++ b/test/unit/utils/test_utils.py @@ -4,6 +4,7 @@ import os import signal import time +import stat from pathlib import Path @@ -113,6 +114,50 @@ def test_transmit_symlink(tmp_path, symlink_dest, check_content): assert f.read() == 'hello world' +@pytest.mark.timeout(timeout=3) +def test_stream_dir_no_hang_on_pipe(tmp_path): + # prepare the input private_data_dir directory to zip + pdd = tmp_path / 'timeout_test' + pdd.mkdir() + + with open(pdd / 'ordinary_file.txt', 'w') as f: + f.write('hello world') + + # make pipe, similar to open_fifo_write + os.mkfifo(pdd / 'my_pipe', stat.S_IRUSR | stat.S_IWUSR) + + # zip and stream the data into the in-memory buffer outgoing_buffer + outgoing_buffer = io.BytesIO() + outgoing_buffer.name = 'not_stdout' + stream_dir(pdd, outgoing_buffer) + + +@pytest.mark.timeout(timeout=3) +def test_unstream_dir_no_hang_on_pipe(tmp_path): + # prepare the input private_data_dir directory to zip + pdd = tmp_path / 'timeout_test_source_dir' + pdd.mkdir() + + with open(pdd / 'ordinary_file.txt', 'w') as f: + f.write('hello world') + + # zip and stream the data into the in-memory buffer outgoing_buffer + outgoing_buffer = io.BytesIO() + outgoing_buffer.name = 'not_stdout' + stream_dir(pdd, outgoing_buffer) + + dest_dir = tmp_path / 'timeout_test_dest' + dest_dir.mkdir() + + # We create the pipe in the same location as an archived file to trigger the bug + os.mkfifo(dest_dir / 'ordinary_file.txt', stat.S_IRUSR | stat.S_IWUSR) + + outgoing_buffer.seek(0) + first_line = outgoing_buffer.readline() + size_data = json.loads(first_line.strip()) + unstream_dir(outgoing_buffer, size_data['zipfile'], dest_dir) + + @pytest.mark.parametrize('fperm', [ 0o777, 0o666,