From: Jeff King <peff@peff.net>
Date: Thu, 14 May 2020 22:03:46 GMT
Subject: Re: [PATCH] t0300: workaround bug in FreeBSD < 10 sh
Message-ID: <20200514220346.GA3074610@coredump.intra.peff.net>
In-Reply-To: <20200514210518.56101-1-carenas@gmail.com>

On Thu, May 14, 2020 at 02:05:18PM -0700, Carlo Marcelo Arenas Belón wrote:

> 4c5971e18a (credential: treat "?" and "#" in URLs as end of host,
> 2020-04-14) introduces check_host_and_path to t0300 and some tests that
> use it, but fail in at least FreeBSD 9.3.
> 
> The variables in the here-doc fail to be expanded until they are used as
> part of the eval in check(), resulting in (ex: url=fill) instead of what
> was expected.

Wow, that's very surprising.

Just to be clear, if you run:

foo() {
  for i in "$@"; do
    echo "arg:$i"
  done
  sed s/^/stdin:/
}
set -- outer
foo inner <<EOF
$1
EOF

do you get:

arg:inner
stdin:inner

? (on dash and bash, I get stdin:outer as expected). I don't think the
fact that check() uses eval() should matter, because we'd be
interpreting that here-doc earlier as part of read_chunk().

> While at it, make sure all of the parameters which potentially sensitive
> characters (ex: ?#), are quote protected.

I don't mind more quoting, but...

> -test_expect_success 'url parser handles bare query marker' '
> -	check_host_and_path https://example.com?foo.git example.com ?foo.git
> -'
> +test_expect_success 'url parser handles bare query marker' "
> +	check_host_and_path 'https://example.com?foo.git' \
> +		example.com '?foo.git'
> +"

...please don't invert the double and single quotes. In either case the
metacharacter is subject to double-quote expansion, and by putting
the double-quotes on the outside, you've left a trap for somebody adding
more lines to the test (the shell snippet will now be interpolated
before being eval'd, which is contrary to how most of our tests run).

-Peff