Re: git rebase -i error message interprets \t in commit message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David Kastrup <dak@xxxxxxx> writes:

> Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
>
>>>From 7962ac8d8f2cbc556f669fd97487f9d70edc4ea1 Mon Sep 17 00:00:00 2001
>> From: Matthieu Moy <Matthieu.Moy@xxxxxxx>
>> Date: Tue, 6 Aug 2013 19:13:03 +0200
>> Subject: [PATCH] die_with_status: use "printf '%s\n'", not "echo"
>>
>> At least GNU echo interprets backslashes in its arguments.
>
> I think that's incorrect in several respects.  For one thing, echo is
> never called for most Bourne shells since echo is a builtin (might have
> been different for UNIX version 7 or so).  For another, GNU echo would
> behave like Bash:
>
> And GNU Bash does not interpret escapes unless you do echo -e.  Escape
> sequence interpretation, however, happens for Dash:
>
> dak@lola:/usr/local/tmp/lilypond$ dash -c 'echo "x\tx"'
> x	x
> dak@lola:/usr/local/tmp/lilypond$ bash -c 'echo "x\tx"'
> x\tx
> dak@lola:/usr/local/tmp/lilypond$ /bin/echo "x\tx"
> x\tx
>
> So replace "GNU echo" in your commit message with "Dash's echo builtin"
> and you get closer.

I'll queue the attached.

POSIX makes it an implementation defined behaviour when the first
argument is "-n", or any argument contains a backslas (X/Open System
Interfaces wants to treat "-n" as literal and always interpret the
backslash sequence), so it is definitely safer to avoid running
'echo' on any random string.

Thanks.

Author: Matthieu Moy <Matthieu.Moy@xxxxxxx>
Date:   Tue Aug 6 20:26:44 2013 +0200

    die_with_status: use "printf '%s\n'", not "echo"
    
    Some implementations of 'echo' (e.g. dash's built-in) interpret
    backslash sequences in their arguments.
    
    This triggered at least one bug: the error message of "rebase -i" was
    turning \t in commit messages into actual tabulations. There may be
    others.
    
    Using "printf '%s\n'" instead avoids this bad behavior, and is the form
    used by the "say" function.
    
    Noticed-by: David Kastrup <dak@xxxxxxx>
    Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
    Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
 	status=$1
 	shift
-	echo >&2 "$*"
+	printf >&2 '%s\n' "$*"
 	exit "$status"
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 49ccb38..074deb1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' '
 	test $(cat file1) = Z
 '
 
+test_expect_success 'rebase -i error on commits with \ in message' '
+	current_head=$(git rev-parse HEAD)
+	test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" &&
+	test_commit TO-REMOVE will-conflict old-content &&
+	test_commit "\temp" will-conflict new-content dummy &&
+	(
+	EDITOR=true &&
+	export EDITOR &&
+	test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error
+	) &&
+	grep -v "	" error
+'
+
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]