Re: [PATCH (not final)] (take three?) unset "array[$anything]"

[Stephane Chazelas <stephane@xxxxxxxxxxxx>]

2021-06-02 19:04:01 -0700, Bart Schaefer:
> On Wed, Jun 2, 2021 at 8:59 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> >
> > I've just had a hand-slaps-forehead
> > moment ... take 3 to follow in another thread.
> 
> What I realized is that for any unset of an array element, the closing
> bracket must always be the last character of the argument.  There's no
> reason to parse the subscript or skip over matching brackets; if a '['
> is found, just make sure the last character is ']' and the subscript
> must be everything in between.

D'oh, I had assumed that had been like that in the beginning but
changed to allow backslash processing for some reason or other
such as alignment with something else.


[...]
> Given this realization, it's easy to make { unset "hash[$key]" } work
> "like it always should have".  The trouble comes in with (not)
> breaking past workarounds.

I think I'd be in favour of breaking these workarounds on the
ground that it would fix far more existing script (that just do
unset "hash[$key]") than it would fix.

There's also the question of what to do with

read "hash[$key]"
getopts "hash[$key]"

and getln, print[f] -v, sysread, strftime -s...

I've not tested all of them but at least for read and print -v,

key=foo
typeset -A a
print -v 'a[$key]' x

gives:

typeset -A a=( [foo]=x )

and not

typeset -A a=( ['$key']=x )

With those, one can't do:

read "hash[$key]"

(which would be a command injection vulnerability)

And the (unintuitive) work around is:

read 'hash[$key]'

It's the same in bash unless you set the assoc_expand_once
option there, not in ksh93.

My suggestion of making unset a keyword so that unset hash[$key]
works as expected  while unset "hash[$key]" works as before
won't fly as we can't possibly make all builtins that take
lvalues keywords.


[...]
> Therefore I think the best option is to choose one of the latter two,
> possibly depending on which one induces the least damage to any
> workarounds for the current behavior that are known in the wild,
> though aesthetically I'd rather use the literal version.
[...]

Well, the only one that doesn't break workarounds is to leave it
asis (and potentially add support for unset 'hash[]'), or
introduce a new syntax (like unset -k "$key" hash) or an option
like bash's assoc_expand_once to switch to the new behaviour.

We can't really expect people to carry on doing:

() { set -o localoptions +o multibyte -o extendedglob
unset "hash[${key//(#m)[][\\()\`]/\\$MATCH}]"; }

to unset a hash key, so I don't thing it's reasonable to leave
it asis.

Another option would be to align unset with the other builtins,
but that's even more problematic as codes that were doing:

unset "hash[$key]"

would change from being buggy (choking on []()`\ and other
characters containing the encoding of those)  to being command
injection vulnerabilities (like with bash without
assoc_expand_once):

$ echo x | key='$(uname>&2)x' bash -c 'typeset -A a; a[$key]=1; unset "a[$key]"; typeset -p a'
Linux
declare -A a=(["\$(uname>&2)x"]="1" )
$ echo x | key='$(uname>&2)x' bash -O assoc_expand_once -c 'typeset -A a; a[$key]=1; unset "a[$key]"; typeset -p a'
declare -A a=()

(zsh has the same problem with read/print -v...)

And again, there is also the problem of hashes used in
arithmetic expressions, including:

key='evil $(reboot)'
print -v 'array[++hash[\$key]]' x

(here you need both the single quotes and the \ to avoid
evaluations of code in the contents of $key)

I must admit I don't really have an idea of how to get out of
this mess. bash, which must have gone through a similar
exercise, hasn't fixed everything with its assoc_expand_once
(see
https://unix.stackexchange.com/questions/627474/how-to-use-associative-arrays-safely-inside-arithmetic-expressions/627475#627475
again).

-- 
Stephane