search

mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.29k stars 345 forks source link

interp: fix off-by-one bug when printing arrays #751

Closed riacataquian closed 3 years ago

riacataquian commented 3 years ago

prior to this PR, we slice the elements of variable.Str but that's buggy; the package expand expands the bash array arr=(x y z) as "x y z", a string with space between members. when accesing via indexes, in effect we get str[2] == "y"

this PR fixes the bug by making use of the expanded variable.List field instead to access the elements of the bash array

fixes #746

riacataquian commented 3 years ago

Hi @mvdan! This should be ready for review :)

It would be cleaner if, instead, we did the sub-slicing for @ and * indices early on. Lucky for you, we already have a code branch just for that case; see elems = vr.List. In there, you could do the subslicing if needed.

Adjusted the code to do just this! We also don't have to do cfg.VarInd needlessly, as in the case of indexed list.

Also, please see separate commits which fixes different things:

All fixes are towards making the commands portable across different platforms.

riacataquian commented 3 years ago

By the way, did you end up testing that the slicing of ${@} and ${*} works just like bash? because the same bug might have been present with those.

Yup, there's:

{`arr=("foo" "bar" "lala" "foobar"); echo ${arr[@]:2:4}; echo ${arr[*]:1:4}`, "lala foobar\nbar lala foobar\n"},

to test slicing works, and:

{`echo ${arr[@]:1:99}; echo ${arr[*]:1:99}`, "\n\n"},
{`arr=("foo"); echo ${arr[@]:99}`, "\n"},

to test out of range scenarios.