Skip to contentSkip to conversation

t5563: prevent "ambiguous redirect"

PATCH In review Johannes Schindelin via GitGitGadget 2 participants 2 messages Mar 31, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t5563: prevent "ambiguous redirect"

From: Johannes Schindelin <[email protected]>

When I ran this test using TEST_SHELL_PATH=/bin/bash in my Ubuntu setup (where Bash is at version 5.0.17(1)-release), I was greeted with this error message:

./test-lib.sh: line 1072: $CHALLENGE: ambiguous redirect

This commit fixes that error by quoting the CHALLENGE variable (which has as value a path containing spaces), and by avoiding to cuddle the empty string parameter in the printf call with the redirect character (in fact, the printf ''>$CHALLENGE is removed because the next line overwrites the file anyway because it _also_ uses a single > to redirect the output).

---
t5563: prevent "ambiguous redirect"

I ran into this issue while running the tests with TEST_SHELL_PATH=/bin/bash.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1507%2Fdscho%2Ffix-ambiguous-redirect-in-t5563-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1507/dscho/fix-ambiguous-redirect-in-t5563-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1507

 t/t5563-simple-http-auth.sh | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)
t/t5563-simple-http-auth.sh +12 -14
index ccf7e54b073..f45a43b4b5e 100755
@@ -252,15 +252,14 @@ test_expect_success 'access using basic auth with wwwauth header empty continuat
 
 	# Note that leading and trailing whitespace is important to correctly
 	# simulate a continuation/folded header.
-	printf "">$CHALLENGE &&
-	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >$CHALLENGE &&
-	printf " \r\n" >>$CHALLENGE &&
-	printf " param2=\"value2\"\r\n" >>$CHALLENGE &&
-	printf "WWW-Authenticate: Bearer authorize_uri=\"id.example.com\"\r\n" >>$CHALLENGE &&
-	printf " p=1\r\n" >>$CHALLENGE &&
-	printf " \r\n" >>$CHALLENGE &&
-	printf " q=0\r\n" >>$CHALLENGE &&
-	printf "WWW-Authenticate: Basic realm=\"example.com\"\r\n" >>$CHALLENGE &&
+	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >"$CHALLENGE" &&
+	printf " \r\n" >>"$CHALLENGE" &&
+	printf " param2=\"value2\"\r\n" >>"$CHALLENGE" &&
+	printf "WWW-Authenticate: Bearer authorize_uri=\"id.example.com\"\r\n" >>"$CHALLENGE" &&
+	printf " p=1\r\n" >>"$CHALLENGE" &&
+	printf " \r\n" >>"$CHALLENGE" &&
+	printf " q=0\r\n" >>"$CHALLENGE" &&
+	printf "WWW-Authenticate: Basic realm=\"example.com\"\r\n" >>"$CHALLENGE" &&
 
 	test_config_global credential.helper test-helper &&
 	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
@@ -298,11 +297,10 @@ test_expect_success 'access using basic auth with wwwauth header mixed line-endi
 
 	# Note that leading and trailing whitespace is important to correctly
 	# simulate a continuation/folded header.
-	printf "">$CHALLENGE &&
-	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >$CHALLENGE &&
-	printf " \r\n" >>$CHALLENGE &&
-	printf "\tparam2=\"value2\"\r\n" >>$CHALLENGE &&
-	printf "WWW-Authenticate: Basic realm=\"example.com\"" >>$CHALLENGE &&
+	printf "WWW-Authenticate: FooBar param1=\"value1\"\r\n" >"$CHALLENGE" &&
+	printf " \r\n" >>"$CHALLENGE" &&
+	printf "\tparam2=\"value2\"\r\n" >>"$CHALLENGE" &&
+	printf "WWW-Authenticate: Basic realm=\"example.com\"" >>"$CHALLENGE" &&
 
 	test_config_global credential.helper test-helper &&
 	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&

base-commit: 5f2117b24f568ecc789c677748d70ccd538b16ba

Signature · 1 line
gitgitgadget
#1 lore raw

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 3 lines · writes:

(in fact, the printf ''>$CHALLENGE is removed because the next line overwrites the file anyway because it _also_ uses a single > to redirect the output).

Good eyes. I also wondered what that empty printf was doing. While I suspect the original intention was to start from an empty file and keep appending contents with any meaning so that the redirection on subsequent lines would look identical, I do not think it is necessary in this case, primarily because it is unlikely that any future change will swap the first line with any subsequent lines.

Thanks for spotting. I was hoping that we could soon retire the "quote the redirection target if it has parameter substitution to help older bash" rule in our coding guidelines, but this example shows that the world is not quite ready yet.