git.cmd.Git.execute(..): fix with_stdout=False

In the event the end-user called one of the APIs with
`with_stdout=False`, i.e., they didn't want to capture stdout, the code
would crash with an AttributeError or ValueError when trying to
dereference the stdout/stderr streams attached to `Popen(..)` objects.

Be more defensive by checking the streams first to make sure they're not
`None` before trying to access their corresponding attributes.

Add myself to AUTHORS and add corresponding regression tests for the
change.

Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

Author: ngie-eign

AUTHORS

@@ -56,5 +56,6 @@ Contributors are:
 -Ethan Lin <et.repositories _at_ gmail.com>
 -Jonas Scharpf <jonas.scharpf _at_ checkmk.com>
 -Gordon Marx
+-Enji Cooper
 
 Portions derived from other open source works and are clearly marked.

git/cmd.py

@@ -1364,25 +1364,30 @@ def communicate() -> Tuple[AnyStr, AnyStr]:
             if output_stream is None:
                 stdout_value, stderr_value = communicate()
                 # Strip trailing "\n".
-                if stdout_value.endswith(newline) and strip_newline_in_stdout:  # type: ignore[arg-type]
+                if (stdout_value is not None and stdout_value.endswith(newline) and
+                    strip_newline_in_stdout):  # type: ignore[arg-type]
                     stdout_value = stdout_value[:-1]
-                if stderr_value.endswith(newline):  # type: ignore[arg-type]
+                if stderr_value is not None and stderr_value.endswith(newline):  # type: ignore[arg-type]
                     stderr_value = stderr_value[:-1]
 
                 status = proc.returncode
             else:
                 max_chunk_size = max_chunk_size if max_chunk_size and max_chunk_size > 0 else io.DEFAULT_BUFFER_SIZE
-                stream_copy(proc.stdout, output_stream, max_chunk_size)
-                stdout_value = proc.stdout.read()
-                stderr_value = proc.stderr.read()
+                if proc.stdout is not None:
+                    stream_copy(proc.stdout, output_stream, max_chunk_size)
+                    stdout_value = proc.stdout.read()
+                if proc.stderr is not None:
+                    stderr_value = proc.stderr.read()
                 # Strip trailing "\n".
-                if stderr_value.endswith(newline):  # type: ignore[arg-type]
+                if stderr_value is not None and stderr_value.endswith(newline):  # type: ignore[arg-type]
                     stderr_value = stderr_value[:-1]
                 status = proc.wait()
             # END stdout handling
         finally:
-            proc.stdout.close()
-            proc.stderr.close()
+            if proc.stdout is not None:
+                proc.stdout.close()
+            if proc.stderr is not None:
+                proc.stderr.close()
 
         if self.GIT_PYTHON_TRACE == "full":
             cmdstr = " ".join(redacted_command)

test/test_git.py

@@ -6,6 +6,7 @@
 import contextlib
 import gc
 import inspect
+import io
 import logging
 import os
 import os.path as osp
@@ -201,6 +202,25 @@ def test_it_logs_istream_summary_for_stdin(self, case):
     def test_it_executes_git_and_returns_result(self):
         self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")
 
+    def test_it_output_stream_with_stdout_is_false(self):
+        temp_stream = io.BytesIO()
+        self.git.execute(
+            ["git", "version"],
+            output_stream=temp_stream,
+            with_stdout=False,
+        )
+        self.assertEqual(temp_stream.tell(), 0)
+
+    def test_it_executes_git_without_stdout_redirect(self):
+        returncode, stdout, stderr = self.git.execute(
+            ["git", "version"],
+            with_extended_output=True,
+            with_stdout=False,
+        )
+        self.assertEqual(returncode, 0)
+        self.assertIsNone(stdout)
+        self.assertIsNotNone(stderr)
+
     @ddt.data(
         # chdir_to_repo, shell, command, use_shell_impostor
         (False, False, ["git", "version"], False),