git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] sequencer: fix quoting in write_author_script

From
PWPhillip Wood <phillip.wood@talktalk.net>
Date
Jul 31, 2018, 11:15 UTC
Message-ID
<20180731111532.9358-3-phillip.wood@talktalk.net>
In-Reply-To
<20180731111532.9358-1-phillip.wood@talktalk.net>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Single quotes should be escaped as \' not \\'. Note that this only affects authors that contain a single quote and then only external scripts that read the author script and users whose git is upgraded from the shell version of rebase -i while rebase was stopped. This is because the parsing in read_env_script() expected the broken version and for some reason sq_dequote() called by read_author_ident() seems to handle the broken quoting correctly.

Ideally rebase and am would share the same code for reading and writing the author script, but this commit just fixes the immediate bug.

The test is based on an original by Akinori MUSHA which was designed to check that the author script was parsable by posix shell.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 sequencer.c                   | 23 +++++++++++++++++------
 t/t3404-rebase-interactive.sh | 17 ++++++++++++++++-
 2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 212b912420..b6e17c1c32 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -636,21 +636,21 @@ static int write_author_script(const char *message)
 		else if (*message != '\'')
 			strbuf_addch(&buf, *(message++));
 		else
-			strbuf_addf(&buf, "'\\\\%c'", *(message++));
+			strbuf_addf(&buf, "'\\%c'", *(message++));
 	strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
 	while (*message && *message != '\n' && *message != '\r')
 		if (skip_prefix(message, "> ", &message))
 			break;
 		else if (*message != '\'')
 			strbuf_addch(&buf, *(message++));
 		else
-			strbuf_addf(&buf, "'\\\\%c'", *(message++));
+			strbuf_addf(&buf, "'\\%c'", *(message++));
 	strbuf_addstr(&buf, "'\nGIT_AUTHOR_DATE='@");
 	while (*message && *message != '\n' && *message != '\r')
 		if (*message != '\'')
 			strbuf_addch(&buf, *(message++));
 		else
-			strbuf_addf(&buf, "'\\\\%c'", *(message++));
+			strbuf_addf(&buf, "'\\%c'", *(message++));
 	strbuf_addch(&buf, '\'');
 	res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1);
 	strbuf_release(&buf);
@@ -664,14 +664,25 @@ static int write_author_script(const char *message)
 static int read_env_script(struct argv_array *env)
 {
 	struct strbuf script = STRBUF_INIT;
-	int i, count = 0;
-	char *p, *p2;
+	int i, count = 0, sq_bug;
+	const char *p2;
+	char *p;
 
 	if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
 		return -1;
 
+	/*
+	 * write_author_script() used to fail to terminate the GIT_AUTHOR_DATE
+	 * line with a "'" and also escaped "'" incorrectly as "'\\\\''" rather
+	 * than "'\\''". We check for the terminating "'" on the last line to
+	 * see how "'" has been escaped in case git was upgraded while rebase
+	 * was stopped.
+	 */
+	sq_bug = script.len && script.buf[script.len - 2] != '\'';
 	for (p = script.buf; *p; p++)
-		if (skip_prefix(p, "'\\\\''", (const char **)&p2))
+		if (sq_bug && skip_prefix(p, "'\\\\''", &p2))
+			strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
+		else if (skip_prefix(p, "'\\''", &p2))
 			strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
 		else if (*p == '\'')
 			strbuf_splice(&script, p-- - script.buf, 1, "", 0);
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index b72167ecd5..ae9036d4d9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -75,6 +75,22 @@ test_expect_success 'rebase --keep-empty' '
 	test_line_count = 6 actual
 '
 
+SQ="'"
+test_expect_success 'rebase -i writes correct author-script' '
+	test_when_finished "test_might_fail git rebase --abort" &&
+	git checkout -b author-with-sq master &&
+	GIT_AUTHOR_NAME="Auth O$SQ R" git commit --allow-empty -m with-sq &&
+	set_fake_editor &&
+	FAKE_LINES="edit 1" git rebase -ki HEAD^ &&
+	tail -n1 .git/rebase-merge/author-script | grep "$SQ\$" &&
+	(
+		sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE &&
+		. .git/rebase-merge/author-script &&
+		test "$(git show -s --date=raw --format=%an,%ae,@%ad)" = \
+			"Auth O$SQ R,$GIT_AUTHOR_EMAIL,$GIT_AUTHOR_DATE"
+	)
+'
+
 test_expect_success 'rebase -i with the exec command' '
 	git checkout master &&
 	(
@@ -1361,7 +1377,6 @@ test_expect_success 'editor saves as CR/LF' '
 	)
 '
 
-SQ="'"
 test_expect_success 'rebase -i --gpg-sign=<key-id>' '
 	test_when_finished "test_might_fail git rebase --abort" &&
 	set_fake_editor &&
-- 
2.18.0
Previous: Phillip WoodNext: Eric Sunshine
Message 20 of 57 in “fix "rebase -i --root" corrupting root commit”
  1. 0/4 fix "rebase -i --root" corrupting root commitEric Sunshine, Jul 31, 2018
  2. 1/4 sequencer: fix "rebase -i --root" corrupting author headerEric Sunshine, Jul 31, 2018
  3. 2/4 sequencer: fix "rebase -i --root" corrupting author header timezoneEric Sunshine, Jul 31, 2018
  4. Phillip WoodJul 31, 2018
  5. Eric SunshineJul 31, 2018
  6. 4/4 sequencer: don't die() on bogus user-edited timestampEric Sunshine, Jul 31, 2018
  7. Phillip WoodJul 31, 2018
  8. Eric SunshineJul 31, 2018
  9. 3/4 sequencer: fix "rebase -i --root" corrupting author header timestampEric Sunshine, Jul 31, 2018
  10. Phillip WoodJul 31, 2018
  11. Eric SunshineJul 31, 2018
  12. Phillip WoodJul 31, 2018
  13. Eric SunshineJul 31, 2018
  14. Phillip WoodJul 31, 2018
  15. Eric SunshineJul 31, 2018
  16. 0/2 Fix author script quotingPhillip Wood, Jul 31, 2018
  17. 1/2 sequencer: handle errors in read_author_ident()Phillip Wood, Jul 31, 2018
  18. Eric SunshineJul 31, 2018
  19. Phillip WoodAug 1, 2018
  20. 2/2 sequencer: fix quoting in write_author_scriptPhillip Wood, Jul 31, 2018
  21. Eric SunshineJul 31, 2018
  22. Phillip WoodAug 1, 2018
  23. Junio C HamanoAug 1, 2018
  24. Phillip WoodAug 1, 2018
  25. Eric SunshineAug 1, 2018
  26. Hilco WijbengaAug 1, 2018
  27. Eric SunshineAug 1, 2018
  28. Hilco WijbengaAug 7, 2018
  29. Eric SunshineAug 7, 2018
  30. Junio C HamanoAug 7, 2018
  31. Johannes SchindelinAug 27, 2018
  32. brian m. carlsonAug 1, 2018
  33. Eric SunshineAug 2, 2018
  34. 0/2 Fix author script quotingPhillip Wood, Aug 2, 2018
  35. 1/2 sequencer: handle errors in read_author_ident()Phillip Wood, Aug 2, 2018
  36. Eric SunshineAug 3, 2018
  37. Junio C HamanoAug 3, 2018
  38. 2/2 sequencer: fix quoting in write_author_scriptPhillip Wood, Aug 2, 2018
  39. Junio C HamanoAug 2, 2018
  40. Eric SunshineAug 3, 2018
  41. Phillip WoodAug 3, 2018
  42. Eric SunshineAug 3, 2018
  43. Phillip WoodAug 3, 2018
  44. Junio C HamanoAug 7, 2018
  45. 0/2 fix author-script quotingPhillip Wood, Aug 7, 2018
  46. 1/2 sequencer: handle errors from read_author_ident()Phillip Wood, Aug 7, 2018
  47. Eric SunshineAug 8, 2018
  48. 2/2 sequencer: fix quoting in write_author_scriptPhillip Wood, Aug 7, 2018
  49. Eric SunshineAug 7, 2018
  50. Phillip WoodAug 7, 2018
  51. Eric SunshineAug 8, 2018
  52. Junio C HamanoAug 8, 2018
  53. Phillip WoodAug 9, 2018
  54. Phillip WoodAug 9, 2018
  55. Eric SunshineAug 8, 2018
  56. Phillip WoodAug 9, 2018
  57. Eric SunshineAug 8, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.