Re: [PATCH 2/2] test-lib: exhaustively insert non-alnum ASCII into the TRASH_DIRECTORY name

[Jeff King <peff@peff.net>]

On Tue, Apr 11, 2017 at 01:23:32AM +0200, Ævar Arnfjörð Bjarmason wrote:

> * Most of the tests fail because git clone can't deal with cloning a
> repo with a \r in the path. The error we produce when we try is quite
> bad and doesn't indicate what went wrong:
> 
> $ rm -rf /tmp/git.*; mkdir /tmp/git.$(perl -e 'print chr 13') && cd
> /tmp/git.* && git init --bare b && file b && git clone b c
> /b/tialized empty Git repository in /tmp/git.
> b: directory
> Cloning into 'c'...
> fatal: '/tmp/git. /b' does not appear to be a git repository
> fatal: Could not read from remote repository.
> 
> Please make sure you have the correct access rights
> and the repository exists.
> 
> So file(1) shows it exists, a strace shows that git knows it exists at
> some point, but something gets lost along the way.

It's the round-trip through the config. It writes:

  [remote "origin"]
  url = /tmp/git.^M/b

into the config (where ^M is a literal CR). And then on reading it back,
unquoted whitespace in config values is converted to spaces, which is
why you see "git. /b" in the error message (the other ones are ugly
because it's writing the raw CR in via printf, and your terminal
respects it. Try "2>&1 | cat -A" when debugging this sort of thing (our
error and warning messages clean up cruft like this already, but
informational messages don't always go through vreportf()).

Anyway, something like this would fix it (it actually adds "\r" to the
config format, which is how we handle \n and \t; we could do it without
touching the parsing side if we taught store_write_pair() to recognize
that \r needs to go in double-quotes).

diff --git a/config.c b/config.c
index 1a4d85537..4a36a37ba 100644
--- a/config.c
+++ b/config.c
@@ -526,6 +526,9 @@ static char *parse_value(void)
 			case 'n':
 				c = '\n';
 				break;
+			case 'r':
+				c = '\r';
+				break;
 			/* Some characters escape as themselves */
 			case '\\': case '"':
 				break;
@@ -2172,6 +2175,9 @@ static int store_write_pair(int fd, const char *key, const char *value)
 		case '\t':
 			strbuf_addstr(&sb, "\\t");
 			break;
+		case '\r':
+			strbuf_addstr(&sb, "\\r");
+			break;
 		case '"':
 		case '\\':
 			strbuf_addch(&sb, '\\');

-Peff