On 12/31/24 6:30 PM, Greg Wooledge wrote:
On Tue, Dec 31, 2024 at 12:15:27 -0500, Chet Ramey wrote:On 12/29/24 4:34 PM, Lawrence Velázquez wrote:I'm aware of the pitfalls described therein, and more. I don't think anyone thinks multiple evaluation in indexes is good per se, but many find the side effects of the attempted cures worse than the disease (e.g., https://mywiki.wooledge.org/BashPitfalls#pf62).In its zeal to demonstrate how bad bash is, that little code snippet ignores the obvious solution: declare -A hash key=\'\] hash[$key]=17 (( hash[key]++ )) declare -p hash Since arithmetic evaluation performs its own expansion of identifiers. assoc_expand_once also works. But that wasn't really the point, was it?What's obvious to you is not obvious to the rest of us. Also, the main point of the BashPitfalls page isn't just to show how "bad" bash is; it's to tell users how to safely and correctly do the thing they want to do.
I think we have different goals here. You want to show users how to cope with the (incorrect) behavior in bash versions before 5.2. I want to figure out what the right behavior is and make bash do that. I'm willing to make incompatible changes because of how bitterly people have complained about the historical bash behavior. These aren't perfectly aligned, but they're not necessarily incompatible. There was a long discussion about this and what ended up being in bash-5.2 in March and April, 2021, starting at https://lists.gnu.org/archive/html/bug-bash/2021-03/msg00056.html The upshot is that bash-5.2 does the right thing, with or without assoc_expand_once, in all of these test cases. But since it no longer needs the workarounds that were required to avoid double expansion of subscripts in previous versions, those workarounds do not work as they did before. The bash-5.1 behavior is available if you set BASH_COMPAT=51, so the workarounds will act as they did previously. I obviously don't recommend that.
hobbit:~$ cat x61 declare -A hash key=\'\] hash[$key]=foo if [[ -v hash[$key] ]]; then echo yes; else echo no; fi It should say "yes".
This works in bash-5.2, with or without assoc_expand_once. That was kind of the point of the changes.
hobbit:~$ cat x61 declare -A hash key=\'\] hash[$key]=foo if [[ -v 'hash[$key]' ]]; then echo yes; else echo no; fi
Same.
Now, the second failure mode: if key contains a command substitution, the evaluation may execute it. hobbit:~$ cat x61 declare -A hash key='x$(date >&2)' hash[$key]=foo if [[ -v hash[$key] ]]; then echo yes; else echo no; fi
This works in bash-5.2, with or without assoc_expand_once. This was the other thing the changes intended to address.
With the single quotes: hobbit:~$ cat x61 declare -A hash key='x$(date >&2)' hash[$key]=foo if [[ -v 'hash[$key]' ]]; then echo yes; else echo no; fi
hobbit:~$ for v in 4.0 4.1 4.2 4.3 4.4 5.0 5.1 5.2; do printf '%s: ' "$v"; bash-"$v" x61; done; for v in 5.0 5.1 5.2; do printf '%s with shopt: ' "$v"; bash-"$v" y61; done 4.0: x61: line 4: conditional binary operator expected x61: line 4: syntax error near `'hash[$key]'' x61: line 4: `if [[ -v 'hash[$key]' ]]; then echo yes; else echo no; fi' 4.1: x61: line 4: conditional binary operator expected x61: line 4: syntax error near `'hash[$key]'' x61: line 4: `if [[ -v 'hash[$key]' ]]; then echo yes; else echo no; fi' 4.2: no 4.3: yes 4.4: yes 5.0: yes 5.1: yes 5.2: yes 5.0 with shopt: yes 5.1 with shopt: no 5.2 with shopt: yes The single quotes successfully stop the code injection in all versions, which is good. The conclusion here is that this feature simply doesn't exist before version 4.2, has no known way to make it work correctly in 4.2, can be made to work correctly with a single-quote workaround in versions 4.3 to 5.1 (as long as assoc_expand_once is NOT set), and works out of the box only in 5.2.
Kind of. I had hoped using an option that was off by default would have
been a less intrusive way to improve the behavior, but finally concluded
that correct behavior was more important. The issue in bash-5.1 was the
result of an unrelated change making the (now not-needed) workaround
unnecessary. The test is for ${hash[$key]}.
Next, we have pitfall 62: (( hash[$key]++ ))
hobbit:~$ cat x62
declare -A hash
key=\'\]
hash[$key]=3
(( hash[$key]++ ))
printf '%s\n' "${hash[$key]}"
This one should print "4".
This works in bash-5.2 with or without assoc_expand_once.
The single-quote workaround from pitfall 61 only works in some versions:
Yes. If you make the workaround unnecessary, a script that defeats those changes won't produce the same results as in previous versions.
This gives the right answer in 4.0 through 5.0, gives the wrong answer with NO error message in 5.0 with shopt, and gives the wrong answer with an error message in 5.1 and 5.2 (regardless of shopt).
Again.
The second known workaround is a backslash:
hobbit:~$ cat x62
declare -A hash
key=\'\]
hash[$key]=3
(( hash[\$key]++ ))
printf '%s\n' "${hash[$key]}"
hobbit:~$ for v in 4.0 4.1 4.2 4.3 4.4 5.0 5.1 5.2; do printf '%s: ' "$v"; bash-"$v" x62; done; for
v in 5.0 5.1 5.2; do printf '%s with shopt: ' "$v"; bash-"$v" y62; done
4.0: 4
4.1: 4
4.2: 4
4.3: 4
4.4: 4
5.0: 4
5.1: 4
5.2: 3
5.0 with shopt: 3
5.1 with shopt: 3
5.2 with shopt: 3
This one gives the right answer in 4.0 through 5.1 (without shopt),
gives the wrong answer in 5.0 and 5.1 with shopt, and gives the wrong
answer in 5.2 regardless of the shopt.
I know you understand that you're changing the key you're testing if you don't have the double expansion behavior.
Pitfall 62 gives a few workarounds which are purported to work across all versions. The first is using a temporary string variable:
The third is to use "let" with a single-quoted expression instead of "((":
Those are not equivalent, of course.
hobbit:~$ cat x62
declare -A hash
key=\'\]
hash[$key]=3
let 'hash[$key]++'
printf '%s\n' "${hash[$key]}"
hobbit:~$ for v in 4.0 4.1 4.2 4.3 4.4 5.0 5.1 5.2; do printf '%s: ' "$v"; bash-"$v" x62; done; for
v in 5.0 5.1 5.2; do printf '%s with shopt: ' "$v"; bash-"$v" y62; done
4.0: 4
4.1: 4
4.2: 4
4.3: 4
4.4: 4
5.0: 4
5.1: 4
5.2: 4
5.0 with shopt: 3
5.1 with shopt: 3
5.2 with shopt: 3
(Note: gotta mention that the shopt breaks this one!)
I'm actually torn about this one. `let' is a builtin, not a compound command, so its arguments undergo the standard simple command word expansions. This is unchangeable. Since the index that's actually evaluated is `$key', assoc_expand_once forces `let' to increment the element with key '$key'. This would be clearer if you displayed the contents of the array with `declare -p'. Leaving assoc_expand_once unset allows the array expansion to expand $key to "']" and you get different results. Now, I think that's probably wrong, and now that I think about it, I should change it to be consistent with other builtins. If bash-5.2 is going to act like assoc_expand_once is set in most contexts, it should have done so here. But still, you don't need this workaround.
Now, Chet has proposed a fifth one:
hobbit:~$ cat x62
declare -A hash
key=\'\]
hash[$key]=3
(( hash[key]++ ))
printf '%s\n' "${hash[$key]}"
No, that was my mistake. I pasted the wrong script into my message. I
thought I said that in a message to the list, but I can't find it now.
Chet
--
``The lyf so short, the craft so long to lerne.'' - Chaucer
``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU c...@case.edu http://tiswww.cwru.edu/~chet/
OpenPGP_signature.asc
Description: OpenPGP digital signature
