Merge pull request #2125 from gitpython-developers/copilot/address-review-comments-2116

Harden commit trailer subprocess handling and align trailer I/O paths

Author: Byron

git/objects/commit.py

@@ -450,14 +450,7 @@ def trailers_list(self) -> List[Tuple[str, str]]:
         :return:
             List containing key-value tuples of whitespace stripped trailer information.
         """
-        cmd = ["git", "interpret-trailers", "--parse"]
-        proc: Git.AutoInterrupt = self.repo.git.execute(  # type: ignore[call-overload]
-            cmd,
-            as_process=True,
-            istream=PIPE,
-        )
-        trailer: str = proc.communicate(str(self.message).encode())[0].decode("utf8")
-        trailer = trailer.strip()
+        trailer = self._interpret_trailers(self.repo, self.message, ["--parse"], encoding=self.encoding).strip()
 
         if not trailer:
             return []
@@ -469,6 +462,27 @@ def trailers_list(self) -> List[Tuple[str, str]]:
 
         return trailer_list
 
+    @classmethod
+    def _interpret_trailers(
+        cls,
+        repo: "Repo",
+        message: Union[str, bytes],
+        trailer_args: Sequence[str],
+        encoding: str = default_encoding,
+    ) -> str:
+        message_bytes = message if isinstance(message, bytes) else message.encode(encoding, errors="strict")
+        cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers", *trailer_args]
+        proc: Git.AutoInterrupt = repo.git.execute(  # type: ignore[call-overload]
+            cmd,
+            as_process=True,
+            istream=PIPE,
+        )
+        try:
+            stdout_bytes, _ = proc.communicate(message_bytes)
+            return stdout_bytes.decode(encoding, errors="strict")
+        finally:
+            finalize_process(proc)
+
     @property
     def trailers_dict(self) -> Dict[str, List[str]]:
         """Get the trailers of the message as a dictionary.
@@ -699,15 +713,7 @@ def create_from_tree(
                     trailer_args.append("--trailer")
                     trailer_args.append(f"{key}: {val}")
 
-            cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers"] + trailer_args
-            proc: Git.AutoInterrupt = repo.git.execute(  # type: ignore[call-overload]
-                cmd,
-                as_process=True,
-                istream=PIPE,
-            )
-            stdout_bytes, _ = proc.communicate(str(message).encode())
-            finalize_process(proc)
-            message = stdout_bytes.decode("utf8")
+            message = cls._interpret_trailers(repo, str(message), trailer_args)
         # END apply trailers
 
         # CREATE NEW COMMIT

test/test_commit.py

@@ -622,6 +622,71 @@ def test_create_from_tree_with_trailers_list(self, rw_dir):
             "Issue": ["456"],
         }
 
+    @with_rw_directory
+    def test_create_from_tree_with_non_utf8_trailers(self, rw_dir):
+        """Test that trailer creation and parsing respect the configured commit encoding."""
+        rw_repo = Repo.init(osp.join(rw_dir, "test_trailers_non_utf8"))
+        with rw_repo.config_writer() as writer:
+            writer.set_value("i18n", "commitencoding", "ISO-8859-1")
+
+        path = osp.join(str(rw_repo.working_tree_dir), "hello.txt")
+        touch(path)
+        rw_repo.index.add([path])
+        tree = rw_repo.index.write_tree()
+
+        commit = Commit.create_from_tree(
+            rw_repo,
+            tree,
+            "Résumé",
+            head=True,
+            trailers={"Reviewed-by": "André <andre@example.com>"},
+        )
+
+        assert commit.encoding == "ISO-8859-1"
+        assert "Résumé" in commit.message
+        assert "Reviewed-by: André <andre@example.com>" in commit.message
+        assert commit.trailers_list == [("Reviewed-by", "André <andre@example.com>")]
+
+    @with_rw_directory
+    def test_trailers_list_with_non_utf8_message_bytes(self, rw_dir):
+        """Test that trailer parsing handles non-UTF-8 commit message bytes."""
+        rw_repo = Repo.init(osp.join(rw_dir, "test_trailers_non_utf8_bytes"))
+        with rw_repo.config_writer() as writer:
+            writer.set_value("i18n", "commitencoding", "ISO-8859-1")
+
+        path = osp.join(str(rw_repo.working_tree_dir), "hello.txt")
+        touch(path)
+        rw_repo.index.add([path])
+        tree = rw_repo.index.write_tree()
+
+        commit = Commit.create_from_tree(
+            rw_repo,
+            tree,
+            "Résumé",
+            head=True,
+            trailers={"Reviewed-by": "André <andre@example.com>"},
+        )
+
+        bytes_commit = Commit(
+            rw_repo,
+            commit.binsha,
+            message=commit.message.encode(commit.encoding),
+            encoding=commit.encoding,
+        )
+
+        assert bytes_commit.trailers_list == [("Reviewed-by", "André <andre@example.com>")]
+
+    def test_interpret_trailers_encodes_before_launching_process(self):
+        """Test that encoding failures happen before spawning interpret-trailers."""
+        repo = Mock()
+        repo.git = Mock()
+        repo.git.GIT_PYTHON_GIT_EXECUTABLE = "git"
+
+        with self.assertRaises(UnicodeEncodeError):
+            Commit._interpret_trailers(repo, "Euro: €", ["--parse"], encoding="ISO-8859-1")
+
+        repo.git.execute.assert_not_called()
+
     @with_rw_directory
     def test_index_commit_with_trailers(self, rw_dir):
         """Test that IndexFile.commit() supports adding trailers."""