diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 7e316d07a..8d8302dd2 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -63,6 +63,7 @@ from .helpers import ProgressIndicatorPercent from .helpers import basic_json_data, json_print from .helpers import replace_placeholders from .helpers import ChunkIteratorFileWrapper +from .helpers import popen_with_error_handling from .patterns import ArgparsePatternAction, ArgparseExcludeFileAction, ArgparsePatternFileAction, parse_exclude_pattern from .patterns import PatternMatcher from .item import Item @@ -747,9 +748,9 @@ class Archiver: # There is no deadlock potential here (the subprocess docs warn about this), because # communication with the process is a one-way road, i.e. the process can never block # for us to do something while we block on the process for something different. - filtercmd = shlex.split(filter) - logger.debug('--tar-filter command line: %s', filtercmd) - filterproc = subprocess.Popen(filtercmd, stdin=subprocess.PIPE, stdout=filterout) + filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout, log_prefix='--tar-filter: ') + if not filterproc: + return EXIT_ERROR # Always close the pipe, otherwise the filter process would not notice when we are done. tarstream = filterproc.stdin tarstream_close = True diff --git a/src/borg/helpers.py b/src/borg/helpers.py index b67250ebc..ec06946ee 100644 --- a/src/borg/helpers.py +++ b/src/borg/helpers.py @@ -11,9 +11,11 @@ import os.path import platform import pwd import re +import shlex import signal import socket import stat +import subprocess import sys import textwrap import threading @@ -1962,3 +1964,34 @@ def secure_erase(path): fd.flush() os.fsync(fd.fileno()) os.unlink(path) + + +def popen_with_error_handling(cmd_line: str, log_prefix='', **kwargs): + """ + Handle typical errors raised by subprocess.Popen. Return None if an error occurred, + otherwise return the Popen object. + + *cmd_line* is split using shlex (e.g. 'gzip -9' => ['gzip', '-9']). + + Log messages will be prefixed with *log_prefix*; if set, it should end with a space + (e.g. log_prefix='--some-option: '). + + Does not change the exit code. + """ + assert not kwargs.get('shell'), 'Sorry pal, shell mode is a no-no' + try: + command = shlex.split(cmd_line) + if not command: + raise ValueError('an empty command line is not permitted') + except ValueError as ve: + logger.error('%s%s', log_prefix, ve) + return + logger.debug('%scommand line: %s', log_prefix, command) + try: + return subprocess.Popen(command, **kwargs) + except FileNotFoundError: + logger.error('%sexecutable not found: %s', log_prefix, command[0]) + return + except PermissionError: + logger.error('%spermission denied: %s', log_prefix, command[0]) + return diff --git a/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py index 7eb421168..ff6b5efe6 100644 --- a/src/borg/testsuite/helpers.py +++ b/src/borg/testsuite/helpers.py @@ -2,6 +2,7 @@ import argparse import hashlib import io import os +import shutil import sys from datetime import datetime, timezone, timedelta from time import mktime, strptime, sleep @@ -26,6 +27,7 @@ from ..helpers import ProgressIndicatorPercent, ProgressIndicatorEndless from ..helpers import swidth_slice from ..helpers import chunkit from ..helpers import safe_ns, safe_s, SUPPORT_32BIT_PLATFORMS +from ..helpers import popen_with_error_handling from . import BaseTestCase, FakeInputs @@ -816,3 +818,28 @@ def test_safe_timestamps(): datetime.utcfromtimestamp(beyond_y10k) assert datetime.utcfromtimestamp(safe_s(beyond_y10k)) > datetime(2262, 1, 1) assert datetime.utcfromtimestamp(safe_ns(beyond_y10k) / 1000000000) > datetime(2262, 1, 1) + + +class TestPopenWithErrorHandling: + @pytest.mark.skipif(not shutil.which('test'), reason='"test" binary is needed') + def test_simple(self): + proc = popen_with_error_handling('test 1') + assert proc.wait() == 0 + + @pytest.mark.skipif(shutil.which('borg-foobar-test-notexist'), reason='"borg-foobar-test-notexist" binary exists (somehow?)') + def test_not_found(self): + proc = popen_with_error_handling('borg-foobar-test-notexist 1234') + assert proc is None + + @pytest.mark.parametrize('cmd', ( + 'mismatched "quote', + 'foo --bar="baz', + '' + )) + def test_bad_syntax(self, cmd): + proc = popen_with_error_handling(cmd) + assert proc is None + + def test_shell(self): + with pytest.raises(AssertionError): + popen_with_error_handling('', shell=True)