Re: [PATCH] git-sh-setup.sh: fix missing double quotes variables

2 messages, 2 authors, 2016-06-19 · open the first message on its own page

Re: [PATCH] git-sh-setup.sh: fix missing double quotes variables

From: Junio C Hamano <hidden>
Date: 2016-06-19 18:00:34

Junio C Hamano [off-list ref] writes:
LE Manh Cuong [off-list ref] writes:
quoted
It's not only people shooting their foot, but also from malicious user.
Given that `curl url | sudo sh/bash` is often found in many instructions,
an end user may not be noticed about the environment variable injection
from their side.

IMHO, it's better if  git can protect the end users in this situation.
Huh?  For those who run `curl url | sudo sh`, I do not think the
incoming script setting and exporting LV to an arbitrary value and
runing Git is not the top thing they need worry about.

While I think enclosing the string in dq is an improvement (as I
said already), I still do think your use of the v-word is making a
mountain out of an anthill.
I failed to say why I found the dq is an improvement, but that
should be in the log message of this commit.  Off the top of my
head, something like:

	We often make sure an environment variable is set to
	something, either set by the user (in which case we do not
	molest it) or set it to our default value (otherwise), with

		: ${VAR=default value}

	i.e. running the no-op command ":" with ${VAR} as its
	parameters (or the default value we supply), relying on that
	":" is a no-op.

	This pattern, even though it is no-op from correctness point
	of view, still can be expensive if the existing value in VAR
	has shell glob (because they will be expanded against
	filesystem entities) and IFS whitespaces (because the value
	need to be split into multiple parameters).  Our invocation
	of ":" command does not care if the parameter given to it is
	after the value in VAR goes through these processing.

	Enclosing the whole thing in double-quote, i.e.

		: "${VAR=default value}"

	avoids paying the unnecessary cost, so let's do so.

Re: [PATCH] git-sh-setup.sh: fix missing double quotes variables

From: LE Manh Cuong <hidden>
Date: 2016-06-19 18:11:19

It's really a good commit message, better than mine.

So must I make another patch or you will update the commit message?

PS: This is the first time I submit a patch to Git, so forgive me
if I made any silly questions.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help