patcht0300: workaround bug in FreeBSD < 10 sh

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.

Transfer the 3 parameters to local variables which will be used instead and that will be in scope without conflicting names.

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

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 t/t0300-credentials.sh | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

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

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().

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

...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

On Thu, May 14, 2020 at 6:03 PM Jeff King <peff@peff.net> wrote:

That is indeed the result with the FreeBSD 9.3 shell. Seems buggy. Assigning $1, $2, ... to variables (local or not) as Carlo's patch does seems to work around the buggy behavior.

On Thu, May 14, 2020 at 06:03:46PM -0400, Jeff King wrote:

And luckily, also a problem that is no longer present with newer versions of the shell and neither NetBSD's or OpenBSD's AFAIK.

correct, so the problem was in the here-doc, not the eval; thanks for your explanation and reproduction, will update it in the reroll.

$ foo() {

$ set -- outer $ foo inner <<EOF

arg:inner stdin:inner

indeed, apologies for the extra quoting which was part of my original debugging and wasn't really needed.

will keep it without the inversion, only because I think it looks also clearer with the added indentantion.

Carlo

On 2020-05-14 at 21:05:18, Carlo Marcelo Arenas Belón wrote:

From FreeBSD's website, it looks like the production releases are 11.3 and 12.1. 9.3 is EOL and has been since 2019. Since FreeBSD is not supporting this release with security updates, nobody should be using it. In light of that, do we need this patch?

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Perfect comment given on the day I am cutting -rc0---the fewer patches I need to worry about, the better ;-).

On Thu, May 14, 2020 at 04:04:39PM -0700, Junio C Hamano wrote:

Not urgently, and my rationale was that it might get reverted and dropped in the near future, like we did for those workarounds for defunct versions of dash.

Which is why it was clearly marked as a bug and the version it affected.

FWIW there are some people still building git in macOS 10.5 (the last with powerpc support) that was EOL in 2011.

I was considering it will be worth pu (in case someone might need to pull it), but definitely not needed in master for this release.

Carlo