Bash Pitfalls🔗

Bash is hard to use safely. There's a lot of non-obvious behaviors. This file is reference material for the pitfalls we know of that have already steered this library.

(There's also a whole series of details that have changed across versions, and people have a wild range of bash versions running around in the wild that they expect things to work with. But see the sibling bash versions file for more that.)

Using assoc arrays safely🔗

Assoc arrays are extremely touchy in bash and very easy to use unsafely. There's a shocking number of ways that strings can get evaluated instead of used as strings.

Read all of these sections carefully before writing anything remotely resembling name[$key], especially with any key that isn't entirely a literal.

Broadly, the problem is: given the very spicy string assigned here to k:

	k='`y[$(cmd)]`'

...one must write code which handles an assoc array with such a string as a key without ever interpreting that string.

Handling assoc arrays correctly is absolutely critical if there's any part of their keys that can contain user-supplied input. Even if the key strings are fully controlled by the program, it's wise to use only the safest forms of access so that the code has some defense in depth, can be maintained without fear, and so that safe-only-contextually idioms don't get cargo-culted into unsafe contexts.

assoc arrays must be declared as such🔗

Only assoc arrays -- declared with -A -- treat keys as plain strings. Any other array treats its subscript as arithmetic, and arithmetic expansion mode is the source of most of the "runs code in the key" problems.

And any array syntax used on values that aren't already declared as assoc arrays will silently make indexed arrays instead.

You're at risk whenever an array keyed by data wasn't declared -A before its first use, including when you assign to a name that doesn't exist yet, or write to one through a nameref.

# `put` will assign a spicy key into a given array.
k='y[$(cmd)]' 
put() { local -n arr=$1; arr[$k]=v; }

# this is safe: the key is stored as the literal string,
# because `good` is an associative array.
declare -A good; put good

# **this is unsafe and will execute code**,
# because `bad` becomes an indexed array,
# and then arithmetic expansion runs on its keys,
# which can cause arbitrary execution!
put bad

Never pass an assoc array element to a builtin or to arithmetic🔗

Builtins that take a variable name (printf -v, unset, declare, and the -v test) parse that name themselves, after the shell has already expanded it, and does arithmetic expansion on it. This is yet another place where a key has a shot at getting evaluated!

(The bash man page calls this "multiple evaluation", under the assoc_expand_once shopt. It's fairly insane! But also not reliably disableable; the assoc_expand_once shopt didn't exist until bash 5.x.)

All of these are dangerous: printf -v "a[$k]", unset "a[$k]", [[ -v a[$k] ]], or declare -n r="a[$k]", or any a[$k] expressions inside (( )).

Use the key only in assignment and parameter expansion syntax, which bash parses once:

a[$k]=v         # assign
"${a[$k]}"      # read
"${a[$k]-}"     # read, where it may be missing
${a[$k]+set}    # test whether it exists
"${!a[@]}"      # iterate keys, in no particular order

Note: [[ -v ]] and (( )) only do this on bash 4.4 through 5.1, so using tests on a current bash to check that we're not doing something unsafe isn't enough; see also bash versions.

Never unset a single data-keyed element🔗

No spelling of unset on one associative array element is right on every bash: unset "a[$k]" runs code in the key, and unset 'a[$k]' silently does nothing when assoc_expand_once is on.

Drop the whole array and rebuild it instead.

Reading files🔗

use blocks to open an fd and check properties on it without TOCTOU🔗

read and mapfile report a read error the same way as end of file. This is problematic.

And a directory, for one exciting example, reads as empty input with no error, in both read and mapfile.

Handling this with tests like -f or -r would help, except... if one does that one a path string either before or after opening it, that'd be a time-of-check-time-of-use bug.

So what can we do?

Open the fd first; then check on it.

Use a redirect to get an fd; and send it to a block, so you can do multiple operations on it. Like this:

# A missing or unreadable file fails the redirect itself.
# But the `-d` check also makes sure we didn't open a directory!
{ [[ ! -d /dev/stdin ]] && mapfile lines; } <"$file"

NUL bytes vanish unless you check for them🔗

Bash strings can't hold NUL, and each reader loses it differently: mapfile cuts off the string at the null byte; $(...) drops it with a warning; and read -N drops it silently.

Use IFS= read -r -d '' _ to read such a string: it'll at least tell you if you hit a NUL. Exit code 0 means it stopped at a NUL, while 1 means it read to end of file.

Writing library functions🔗

Library code runs under whatever situation and shopts the caller has set. That makes life tricky.

In this library, we mostly centralized handling of that into _testmark_call and wrap roughly everything with that.

(You can go read _testmark_call to see what it does. It and its docs are their own SSOT.)

There's still two things that have to be known everywhere, because they couldn't be solved in one place with wrappers.

check every exit code🔗

We've been unable to use set -e as a strict mode (unfortunately).

That means an awful lot of cmd || return 1 is required.

treat unset variables as fatal🔗

We turned set -u on in our library code, as a strict mode to catch as many bugs as possible.

Mostly this is good and obvious.

There's a couple pretty frequent "oof" sources, though: reading an element of an array that may be missing, or taking ${#a[@]} of an array that may never have been assigned can both count as reads of unset variables.

Solution: use ${a[$k]-} or ${a[$k]+set} for presence, and iterate with "${!a[@]}" rather than counting (or just... make sure your arrays are initialized, which... you should be doing anyway).

Quadratic operations🔗

Don't do quadratic things on document-sized data. Two innocent-looking ones:

• Slicing an array per item. "${a[@]:i:n}" walks the array from its start every time, so slicing out every hunk is quadratic. Instead, loop over the indexes: for ((j = i; j < i + n; j++)).
• Substituting over a whole document. ${s//pat/rep} is quadratic in the number of matches (CRLF to LF over a 1.2 MB string took 15 s on 4.4). Instead, work per line, as the parse does when it strips CR.