Re: [PATCH] git-p4: avoid shell interpretation of commit ids in applyCommit
From: Junio C Hamano <hidden>
Date: 2026-09-23 19:11:09
"Anupam Mediratta via GitGitGadget" [off-list ref] writes:
quoted hunk ↗ jump to hunk
@@ -2279,7 +2290,7 @@ class P4Submit(Command, P4UserMap): if fixed_rcs_keywords: print("Retrying the patch with RCS keywords cleaned up") - if os.system(tryPatchCmd) == 0: + if diffTreeApply(id, tryPatchArgs) == 0: patch_succeeded = True print("Patch succeesed this time with RCS keywords cleaned")
Both of these check the result of running diff|apply pipeline and react to a failure.
quoted hunk ↗ jump to hunk
@@ -2291,7 +2302,7 @@ class P4Submit(Command, P4UserMap): # # Apply the patch for real, and do add/delete/+x handling. # - system(applyPatchCmd, shell=True) + diffTreeApply(id, applyPatchArgs)
It is a bit hard to discover, but the original code catches a failed "diff|apply" pipeline invocation, because the "system()" used here is what git-p4.py defines for itself. When the pipeline fails, this system() raises subprocess.CalledProcessError(). The new one ignores the exit status from the pipeline, so even after a failure to apply the change, the program continues. Which may not be what you want to see.
quoted hunk ↗ jump to hunk
for f in filesToChangeType: p4_edit(f, "-t", "auto")diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh index 2913277013..ef8fd6e094 100755 --- a/t/t9803-git-p4-shell-metachars.sh +++ b/t/t9803-git-p4-shell-metachars.sh@@ -105,4 +105,20 @@ test_expect_success 'branch with shell char' ' ) ' +test_expect_success 'git p4 submit --commit does not execute shell metachars in commit id' ' + git p4 clone --dest="$git" //depot && + test_when_finished cleanup_git && + ( + cd "$git" && + git config git-p4.skipSubmitEditCheck true && + echo f3 >file3 && + git add file3 && + git commit -m "add file3" && + name='"'"'$(touch${IFS}injection-marker)'"'"' && + git branch "$name" HEAD && + P4EDITOR="test-tool chmtime +5" git p4 submit --commit "$name" + ) && + test_path_is_missing "$cli/injection-marker" +' + test_donebase-commit: d38352cd43ab9745686d697872408bc3249a153f