diff --git a/CHANGES.md b/CHANGES.md index f4a04320d66c..7c971698913d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -83,6 +83,7 @@ ## Bugfixes +* Fixed IndexError in `apache_beam.utils.processes` when pip subprocess fails with short command (e.g. `pip install pkg`) (Python) ([#37515](https://github.com/apache/beam/issues/37515)). * Fixed X (Java/Python) ([#X](https://github.com/apache/beam/issues/X)). ## Security Fixes diff --git a/sdks/python/apache_beam/utils/processes.py b/sdks/python/apache_beam/utils/processes.py index f6daecea2125..f34b4a48085f 100644 --- a/sdks/python/apache_beam/utils/processes.py +++ b/sdks/python/apache_beam/utils/processes.py @@ -52,15 +52,12 @@ def call(*args, **kwargs): except OSError as e: raise RuntimeError("Executable {} not found".format(args[0])) from e except subprocess.CalledProcessError as error: - if isinstance(args, tuple) and (args[0][2] == "pip"): - raise RuntimeError( \ - "Full traceback: {}\n Pip install failed for package: {} \ - \n Output from execution of subprocess: {}" \ - .format(traceback.format_exc(), args[0][6], error. output)) from error - else: - raise RuntimeError("Full trace: {}\ - \n Output of the failed child process: {} " \ - .format(traceback.format_exc(), error.output)) from error + raise RuntimeError( + "Output from execution of subprocess: {}\n" + "Command that failed: {}\nFull trace: {}".format( + error.output if error.output is not None else "(not captured)", + args, + traceback.format_exc())) from error return out def check_call(*args, **kwargs): @@ -71,15 +68,12 @@ def check_call(*args, **kwargs): except OSError as e: raise RuntimeError("Executable {} not found".format(args[0])) from e except subprocess.CalledProcessError as error: - if isinstance(args, tuple) and (args[0][2] == "pip"): - raise RuntimeError( \ - "Full traceback: {} \n Pip install failed for package: {} \ - \n Output from execution of subprocess: {}" \ - .format(traceback.format_exc(), args[0][6], error.output)) from error - else: - raise RuntimeError("Full trace: {} \ - \n Output of the failed child process: {}" \ - .format(traceback.format_exc(), error.output)) from error + raise RuntimeError( + "Output from execution of subprocess: {}\n" + "Command that failed: {}\nFull trace: {}".format( + error.output if error.output is not None else "(not captured)", + args, + traceback.format_exc())) from error return out def check_output(*args, **kwargs): @@ -90,15 +84,12 @@ def check_output(*args, **kwargs): except OSError as e: raise RuntimeError("Executable {} not found".format(args[0])) from e except subprocess.CalledProcessError as error: - if isinstance(args, tuple) and (args[0][2] == "pip"): - raise RuntimeError( \ - "Full traceback: {} \n Pip install failed for package: {} \ - \n Output from execution of subprocess: {}" \ - .format(traceback.format_exc(), args[0][6], error.output)) from error - else: - raise RuntimeError("Full trace: {}, \ - output of the failed child process {} "\ - .format(traceback.format_exc(), error.output)) from error + raise RuntimeError( + "Output from execution of subprocess: {}\n" + "Command that failed: {}\nFull trace: {}".format( + error.output if error.output is not None else "(not captured)", + args, + traceback.format_exc())) from error return out def Popen(*args, **kwargs): diff --git a/sdks/python/apache_beam/utils/processes_test.py b/sdks/python/apache_beam/utils/processes_test.py index 13425550dbbe..6330631afdeb 100644 --- a/sdks/python/apache_beam/utils/processes_test.py +++ b/sdks/python/apache_beam/utils/processes_test.py @@ -121,15 +121,25 @@ def test_check_call_pip_install_non_existing_package(self): self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\ cmd, output=output) try: - output = processes.check_call(cmd) + processes.check_call(cmd) self.fail( - "The test failed due to that\ - no error was raised when calling process.check_call") + "The test failed due to that " + "no error was raised when calling process.check_call") except RuntimeError as error: - self.assertIn("Output from execution of subprocess: {}".format(output),\ - error.args[0]) - self.assertIn("Pip install failed for package: {}".format(package),\ - error.args[0]) + self.assertIn("Output from execution of subprocess:", error.args[0]) + self.assertIn(output, error.args[0]) + + def test_check_call_pip_short_command_no_index_error(self): + """Short pip command (e.g. pip install pkg) must not raise IndexError.""" + returncode = 1 + cmd = ['python', '-m', 'pip', 'install', 'nonexistent-package-xyz'] + output = "ERROR: Could not find a version that satisfies" + self.mock_get.side_effect = subprocess.CalledProcessError( + returncode, cmd, output=output) + with self.assertRaises(RuntimeError) as ctx: + processes.check_call(cmd) + self.assertIn("Output from execution of subprocess:", ctx.exception.args[0]) + self.assertIn(output, ctx.exception.args[0]) class TestErrorHandlingCheckOutput(unittest.TestCase): @@ -162,15 +172,25 @@ def test_check_output_pip_install_non_existing_package(self): self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\ cmd, output=output) try: - output = processes.check_output(cmd) + processes.check_output(cmd) self.fail( - "The test failed due to that\ - no error was raised when calling process.check_call") + "The test failed due to that " + "no error was raised when calling process.check_output") except RuntimeError as error: - self.assertIn("Output from execution of subprocess: {}".format(output),\ - error.args[0]) - self.assertIn("Pip install failed for package: {}".format(package),\ - error.args[0]) + self.assertIn("Output from execution of subprocess:", error.args[0]) + self.assertIn(output, error.args[0]) + + def test_check_output_pip_short_command_no_index_error(self): + """Short pip command must not raise IndexError.""" + returncode = 1 + cmd = ['python', '-m', 'pip', 'install', 'nonexistent-package-xyz'] + output = "ERROR: Could not find a version" + self.mock_get.side_effect = subprocess.CalledProcessError( + returncode, cmd, output=output) + with self.assertRaises(RuntimeError) as ctx: + processes.check_output(cmd) + self.assertIn("Output from execution of subprocess:", ctx.exception.args[0]) + self.assertIn(output, ctx.exception.args[0]) class TestErrorHandlingCall(unittest.TestCase): @@ -193,7 +213,7 @@ def test_oserror_check_output_message(self): self.assertIn('Executable {} not found'.format(str(cmd)),\ error.args[0]) - def test_check_output_pip_install_non_existing_package(self): + def test_call_pip_install_non_existing_package(self): returncode = 1 package = "non-exsisting-package" cmd = ['python', '-m', 'pip', 'download', '--dest', '/var',\ @@ -203,15 +223,25 @@ def test_check_output_pip_install_non_existing_package(self): self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\ cmd, output=output) try: - output = processes.call(cmd) + processes.call(cmd) self.fail( - "The test failed due to that\ - no error was raised when calling process.check_call") + "The test failed due to that " + "no error was raised when calling process.call") except RuntimeError as error: - self.assertIn("Output from execution of subprocess: {}".format(output),\ - error.args[0]) - self.assertIn("Pip install failed for package: {}".format(package),\ - error.args[0]) + self.assertIn("Output from execution of subprocess:", error.args[0]) + self.assertIn(output, error.args[0]) + + def test_call_pip_short_command_no_index_error(self): + """Short pip command must not raise IndexError.""" + returncode = 1 + cmd = ['python', '-m', 'pip', 'install', 'nonexistent-package-xyz'] + output = "ERROR: Could not find a version" + self.mock_get.side_effect = subprocess.CalledProcessError( + returncode, cmd, output=output) + with self.assertRaises(RuntimeError) as ctx: + processes.call(cmd) + self.assertIn("Output from execution of subprocess:", ctx.exception.args[0]) + self.assertIn(output, ctx.exception.args[0]) if __name__ == '__main__':