git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [RFC] shell: local x=$1 may need to quote the RHS
Date: Mon, 24 Jan 2022 12:11:33 -0800	[thread overview]
Message-ID: <xmqqsftc3nd6.fsf@gitster.g> (raw)

Even though this message ends with a patch that could be applied to
your tree, it is not for application at all.  It is to demonstrate a
potential problem in the code in our tree. 

While I was playing with the "Linux development environment (beta)"
on one of my Chromebooks, I say its default /bin/sh (which is Dash
0.5.10) mishandled this construct:

	func () {
		local x=$1
		clobber x and nothing else and feel safe
		message="I expect this to be visible by the caller"
	}

	func "a message"
	use "$message"

It assigned 'a' to $x in the function, DECLARED the varilable
$message as local to the function, hence the caller after func
returned did not see what I intended to see in $message.

The breakage is subtle; unless you have a character in $1 that would
not make a valid variable name, you won't get any error message yet
the program would behave in an unexpected way.

In other words, all of these hits are suspect and may misbehave with
such a shell.

    $ git grep -e '^[	 ]*local.* [a-z0-9_]*=\$' t
    t/lib-parallel-checkout.sh:	local expected_workers=$1 &&
    t/t0000-basic.sh:	local x=$1
    t/t4011-diff-symlink.sh:	local oid=$(printf "%s" "$1" | git hash-object --stdin) &&
    t/t4011-diff-symlink.sh:	local oid=$(git hash-object "$1") &&
    t/t4210-log-i18n.sh:	local engine=$1
    t/t4210-log-i18n.sh:	local pattern=$1
    t/test-lib-functions.sh:	local basename=${1#??}
    t/test-lib-functions.sh:	local var=$1 port
    t/test-lib-functions.sh:	local expr=$(printf '"%s",' "$@")
    t/test-lib-functions.sh:	local expr=$(printf '"%s".*' "$@")

Outside t/, I didn't find anything that may be run with dash and
can be fed problematic input, so as far as I can tell, it is a
problem that only affects the current set of tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 t/t0000-basic.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git c/t/t0000-basic.sh w/t/t0000-basic.sh
index b007f0efef..915eb2384f 100755
--- c/t/t0000-basic.sh
+++ w/t/t0000-basic.sh
@@ -45,6 +45,19 @@ test_expect_success 'verify that the running shell supports "local"' '
 	test_cmp expected2 actual2
 '
 
+try_local_unquoted () {
+	local x=$1
+	y="newvalue"
+}
+
+test_expect_success 'verify that "local x=$1" do not quoting' '
+	y=oldvalue &&
+	echo "newvalue" >expect &&
+	try_local_unquoted "x y" &&
+	echo "$y" >actual &&
+	test_cmp expect actual
+'
+
 ################################################################
 # git init has been done in an empty repository.
 # make sure it is empty.

             reply	other threads:[~2022-01-24 20:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-24 20:11 Junio C Hamano [this message]
2022-01-25  9:24 ` [RFC] shell: local x=$1 may need to quote the RHS Konstantin Khomoutov
2022-01-25 19:00   ` Junio C Hamano
2022-01-25 20:19     ` Taylor Blau
2022-01-26  5:53       ` Junio C Hamano
2022-01-26 11:39         ` Konstantin Khomoutov
2022-01-26 21:48         ` SZEDER Gábor
2022-01-26 21:52         ` Taylor Blau
2022-01-27  0:17           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsftc3nd6.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).