[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Severe Bash Bug with Arrays
From: |
Maarten Billemont |
Subject: |
Re: Severe Bash Bug with Arrays |
Date: |
Sat, 5 May 2012 11:04:09 +0200 |
This mail has gone slightly off-topic. Hit mark as read if you don't care for
that sort of thing.
On 27 Apr 2012, at 21:02, Linda Walsh wrote:
> Greg Wooledge wrote:
>
>> NEVER use eval plus a brace expansion to generate a list. That's just
>> freakin' evil. And unsafe:
>
> But I _like_ evil sometimes! >:->
I don't care if you like evil in your sexual play or in your vendettas. When
you write code, evil is nothing worth liking. In programming, evil is defined
as something that is stupid and not thought through. Doing evil has harmful
side effects. It's not cool to be evil, it's short-sighted.
> But reality is things like that save me time in typing in stuff or writing
> maint scripts on my machine -- they aren't meant for handling user input.
>
> They take hand editing to gen the paths I want and don't take user input,
> so it's a specialized usage in that case....
It's fine to do short-sighted hacks on the prompt, where you know your input
well and will notice breakage so that you can recover.
It's not fine to do said things in scripts that run on unpredictable input and
where you don't always notice breakage until it's caused irreparable harm. I
don't really care what lame justifications you come up with to try and justify
broken system code to yourself, just don't try to convince us with them.
> Maarten Billemont wrote:
>
>> On 26 Apr 2012, at 01:18, Linda Walsh wrote:
>>> Ishtar:> echo "${b[*]}"
>> Please note that expanding array elements using [*] is usually NOT what
>> anyone wants.
>
> It was exactly what I wanted for my example to work. Please don't suggest
> non-working solutions to the example code I provided. ${b[@]} Will NOT
> work as a replacement for [*]. It isn't not BETTER, it is DIFFERENT.
> That's why there are both.
I didn't say never to use "${b[*]}". I said it's usually not what anyone
wants. It's mostly only useful for merging arrays into a single string such as
for user display of said array. I explained that. You are correct, of course,
there are two syntaxes, they both do something different (when quoted), and
that's fine. They each have their use case.
What I *did* say, is never to use ${b[*]}. And it also so happens that ${b[@]}
is not different from ${b[*]}. And I stick with that point. Never use
unquoted expansion of arrays, not with [*], not with [@].
>> your array like above, you then follow up by re-splitting the whole thing
>> using standard wordsplitting, not to mention pathname expanding the
>> resulting words.
>
> I followed up? I don't understand this.
When you leave ${b[*]} unquoted, you expand each element of the array, then ask
bash to split the whole thing, and then pathname expand the result. Not only
did you just throw away any distinction between the different array elements,
you also made an entirely new distinction (whitespace), and then you're asking
bash to search the filesystems for any pathnames that match each of the words.
Seriously, nobody, ever, wants, this. If they did, they'd have used a string
instead. And even then I question the sanity behind it. Word-splitting is
useful in a primitive POSIX shell that has no concept like arrays etc. to
compensate. In bash, it is the major cause of bugs, and best avoided.
> I would assume they'd have the
> intelligence to know when to use * or @ and I wouldn't have to talk down to
> them
> and cover basics.
I can't help notice the slight irony in my having to explain to you the
necessity of quotes with array expansion.
>> You should always recommend the "${b[@]}" variant.
>
> I shouldn't ALWAYS do anything. It's dangerous.
> You can do so for your examples... It won't work in the next
> two any more than the last one of the previous...
What?
Anyway, think about this: An array holds elements in a way that each is
separated from the other. To put these elements in an array is to want each
element to be distinct from the others. Therefore, the default way of
expanding these elements should preserve that distinction.
Only in rare cases is the actual INTENT of the author to MERGE these elements
into a single unit, a single string, thus throwing away the distinction between
the elements. And in THOSE cases and those cases ALONE should the author use
"${array[*]}". So yes, always recommend "${b[@]}", unless you know for a fact
that the author is looking to merge the elements.
> sort an arg list:
>
> > args="-dpi -xinerama -clipboard -unixkill -nowinkill -ac +bs -fp
> > -multiwindow"
>
> > readarray -t dflts< <( a=( $args ); IFS=$'\n'; echo "${a[*]#?}"|sort -k1.2 )
>
> > printf " defaults=(%s)" "${dflts[*]}"
>
> defaults=(-ac +bs -clipboard -dpi -fp -multiwindow -nowinkill -unixkill
> -xinerama)
... What are you trying to prove? That with a very specific sent of input
values you can do some ugly and hacky mangling involving arrays and
wordsplitting to sort a word-list? Why are you even USING arrays here at all?
There's no point. Just to replace spaces with newlines?
Also, you're doing a lot of really bad stuff here. The point of the array
structure is to be able to differentiate elements that can hold things like
spaces and newlines. Building an array from wordsplitting is senseless: you
might as well just wordsplit the string wherever you were planning to use the
array instead.
This snippet completely breaks any arguments that contain spaces and newlines
in several places. That is unacceptable. It works for conveniently chosen
demo arguments. But it's still broken code. Arguments are by definition
c-strings. Any code that doesn't treat them as such violates that definition.
Might I point out that you're stripping the leading - and + from your arguments
and not putting them back? I don't know where you got the output you claim
from, but the real output has lost the first character of each word.
> Want to create pathnames using bash's /path/{}/ feature and have the paths
> in an array (for simple paths):
>
> > a=(lib tmp bin share)
> > (export IFS=,;eval "echo /usr/{${a[*]}}")
> /usr/lib /usr/tmp /usr/bin /usr/share
Don't do this. You're injecting strings into bash code, and then evaluating
that code. You're basically injecting data into code. Given certain crafted
filenames, this makes for arbitrary code execution. I really don't care about
what kind of justifications you're going to try to come up with for this.
Ignoring the security aspect, this code will break for elements that contain
commas or braces or quotes or newlines. And you know what, yes, it's perfectly
fine for filenames to contain those. It's not fine for your code to break on
them.
Grow up, use a loop. It really doesn't hurt that much.
for dir in "${dirs[@]}"; do
whatever "/usr/$dir"
done
If you're really a fan of short, there's still options:
whatever "${dirs[@]/#//usr/}"
> But I was pointing out that [*] has its place ... I use [@] alot
> more often than [*], but will use [*]
I agree.
> , though a bit irregularly,
> when I want to get the # of items in an array (where either would work)..
> though I could make a case if it was worth anything to me that #..@ return
> # items and #...[*] return total length of all items...
Which is why I always use {#array[@]} and not ${#array[*]}. While they do the
same, the former gives me a warm fuzzy feeling, the latter makes me feel guilty.
> C'est la vi...
vie ;-)
smime.p7s
Description: S/MIME cryptographic signature
- Re: Severe Bash Bug with Arrays,
Maarten Billemont <=