News for hackers. Stuff that matters.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by kevan on Tuesday March 24, 2015 @12:59AM (#9254977)
Agreed, I should've elaborated. All it takes is something like this in the init script without checking if the variable is empty:

    rm -rf "$STEAMROOT/"*
Re:Bug 1202858 – Restarting squid results in deleting all fi... by lpsz on Tuesday March 24, 2015 @01:45AM (#9255068)
And this is why it is important to write something like

    set -eu
on top of your bash scripts -- execution will stop on errors (non-zero retvals) and on undefined variables.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by cellularmitosis on Tuesday March 24, 2015 @02:28AM (#9255142)
I also include set -o pipefail (exit if ANY command in a pipeline fails). Had to get bitten and waste an hour before that became a habit.

set -e and set -o pipefail really should have been the default, rather than an opt-in.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by nwalfield on Tuesday March 24, 2015 @07:00AM (#9255830)
set -o pipefail makes common idioms a pain. Consider using head, which simply exits after it has read a few lines. In this case, the input process gets a SIGPIPE and exits with a non-zero exit code:

Consider /tmp/test.sh:

  set -o pipefail
  yes foo | head

  $ bash /tmp/test.sh >/dev/null
  $ echo $?
  141
Re:Bug 1202858 – Restarting squid results in deleting all fi... by pixelbeat on Tuesday March 24, 2015 @07:37AM (#9255925)
That's a bug IMHO which I reported at http://lists.gnu.org/archive/html/bug-bash/2015-02/msg00052....

I've collated other mishandling of closed pipes at: http://www.pixelbeat.org/programming/sigpipe_handling.html

Re:Bug 1202858 – Restarting squid results in deleting all fi... by quotemstr on Tuesday March 24, 2015 @09:56AM (#9256538)
For a while now, I've thought we should change SIGPIPE's SIG_DFL action to _exit(0).
Re:Bug 1202858 – Restarting squid results in deleting all fi... by darklajid on Tuesday March 24, 2015 @05:46AM (#9255609)
That's not as simple or clear as you make it sound though.

http://mywiki.wooledge.org/BashFAQ/105

disagrees and refers to GreyCat's preference not to use -e at the bottom of the list of 'complications'.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by un1xl0ser on Tuesday March 24, 2015 @08:20AM (#9256096)
From the same page:"rking's personal recommendation is to go ahead and use set -e, but beware of possible gotchas. It has useful semantics, so to exclude it from the toolbox is to give into FUD."

You can use set -e, and turn it off (set +e) for code blocks and things that are problematic. He could also add '|| true', and you may be able to use colon to avoid point problems without turning everything off. These are edge cases and you can easily work around them if you an advanced user.

If you are not an advanced user then you should certainly use -e.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by un1xl0ser on Tuesday March 24, 2015 @08:35AM (#9256162)

  $ diff -u /tmp/a /tmp/b
  --- /tmp/a	2015-03-24 08:33:00.021919797 -0400
  +++ /tmp/b	2015-03-24 08:33:05.629963015 -0400
  @@ -1,5 +1,5 @@
   #!/usr/bin/env bash
   set -e
   i=0
  -let i++
  +let i++ || true
   echo "i is $i"
  $ /tmp/a
  $ /tmp/b
  i is 1
  $
Re:Bug 1202858 – Restarting squid results in deleting all fi... by jiffytick on Tuesday March 24, 2015 @02:17AM (#9255122)
or check the variable before using it, like any other programming language:

[[ "$VAR" ]] && rm -rf "$VAR/*"

I think most of these issues stem from the fact that most developers that write shell scripts don't actually understand what they're doing, treating the script as a necessary annoyance rather than a component of the software.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by TheDong on Tuesday March 24, 2015 @02:54AM (#9255199)
If anyone understands shell scripts, it would be people writing init scripts at Red Hat :)

Anyways, that is not anything like other programming languages. Checking in that way is error prone and not really an improvement (nor equivalent to set -o).

  [[ "$DAEMON_PATH" ]] && rm -rf "$DEAMON_PATH/*"
See what I did there? It's an rm -rf /* bug because "checking variables" is not the answer.

In other programming languages, if an identifier is mis-typed things will blow up. E.g., in ruby if I write:

  daemon_path=1; if daemon_path; puts deamon_path; end
I get "NameError: undefined local variable or method `deamon_path`"

These issues do not always stem from bad developers. Bash's defaults are not safe in many ways and saying "people should just check the variable" isn't helpful here.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by bishc on Tuesday March 24, 2015 @07:34AM (#9255918)
Shameless plug for my language "bish" (compiles to bash) which aims to solve many of these annoyances with shell scripting: https://github.com/tdenniston/bish
Re:Bug 1202858 – Restarting squid results in deleting all fi... by pwg on Tuesday March 24, 2015 @01:43PM (#9258142)
Bash has the ability to also flag use of an undefined variable an error, it is just not on by default.

set -u

Man page quote: "Treat unset variables and parameters other than the special parameters "@" and "*" as an error when performing parameter expansion. If expansion is attempted on an unset variable or parameter, the shell prints an error message, and, if not interactive, exits with a non-zero status."

Re:Bug 1202858 – Restarting squid results in deleting all fi... by woah on Tuesday March 24, 2015 @04:52AM (#9255472)
Elephant in the room- shell is a bizarre language
Re:Bug 1202858 – Restarting squid results in deleting all fi... by Someone1234 on Tuesday March 24, 2015 @09:47AM (#9256479)
Yeah, everyone always loves to shit on BAT (which is fair, it is terrible) and VBS (which is slightly less fair) but inspite of how many problems Bash has (least of all the massive security issue last year), it gets off almost scot free.

These bugs are indicative of Bash's design problems. Why is it used for init scripts? And don't even get me started on how Bash interprets filenames as part of the arguments list when using * (e.g. file named "-rf").

Say what you will about Powershell, but having a typed language that can throw a null exception is useful for bugs like these. The filename isn't relevant, and a null name on a delete won't try to clear out of the OS (just throw).

Re:Bug 1202858 – Restarting squid results in deleting all fi... by rodgerd on Tuesday March 24, 2015 @03:01PM (#9258661)
> it gets off almost scot free.

Not just scot free - during the Great systemd War of 2014 is was a talking point for the antis that using anything other than the pure, reliable simplicity of shell for service management was MADNESS!

Re:Bug 1202858 – Restarting squid results in deleting all fi... by digi_owl on Wednesday March 25, 2015 @03:27AM (#9261654)
I don't think that was the argument, as much as it was that if a shell script fouled up it was easier to get in and do field repairs because it was interpreted rather than compiled.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by sjolsen on Tuesday March 24, 2015 @02:09PM (#9258334)
>And don't even get me started on how Bash interprets filenames as part of the arguments list when using * (e.g. file named "-rf")

That's not Bash. That's just... programs in Unix. Such is life when everything is stringly typed.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by emmelaich on Tuesday March 24, 2015 @08:05AM (#9256056)
I think a better alternative is something like

    rm -r "${VAR:-var_is_not_set_so_please_fix_this_script}"
which substitutes the var_is_... if VAR is not set.

BTW, I hate hate hate -f. It has two meanings: 1. 'force' the removal 2. ignore any error

I've seen an instance of this sort of bug in my sysadmin career that I remember. It was a Solaris patch which wiped a chunk of the system.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by rjcz on Tuesday March 24, 2015 @09:49AM (#9256496)
No, this will remove 'var_is_not_set_so_please_fix_this_script' file if one exists.

If you're suggesting using parameter expansion, at least suggest the correct one (i.e. one that will give a meaningful error message):

    ${parameter:?word}
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3...
Re:Bug 1202858 – Restarting squid results in deleting all fi... by emmelaich on Tuesday March 24, 2015 @06:46PM (#9260062)
Yep, better. Thanks.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by mraison on Tuesday March 24, 2015 @02:58AM (#9255210)
> like any other programming language

some real-world programming languages don't have undefined variables :)

Re:Bug 1202858 – Restarting squid results in deleting all fi... by coldtea on Tuesday March 24, 2015 @04:27AM (#9255417)
Since the variable he shows is used in a string interpolation, it doesn't have to be undefined.

Being the emptys string "" would work just as well.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by guardian5x on Tuesday March 24, 2015 @04:23AM (#9255403)
I wonder why set -eu is not the default setting.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by blueskin_ on Tuesday March 24, 2015 @06:31AM (#9255736)
1. open bash

2. set -e

3. type an invalid command or run one that returns non-zero

4. "crap, where did my shell go?"

Re:Bug 1202858 – Restarting squid results in deleting all fi... by twic on Tuesday March 24, 2015 @09:00AM (#9256268)
It could be the default for non-interactive shells without causing this problem. Or we could have a more nuanced rule, where -e means "stop executing the current sequence of commands as soon as there is an error", where a "sequence of commands" is a single line in an interactive shell (so "false; whoami" would print nothing), or the entire file in a script.

The real answer is that this has not been the default in the time between shells being invented and this comment being posted, and so the squillions of lines of shell script out there in the wild keeping the world turning have not been written with this in mind. Making it the default now would break a lot of things.

With the benefit of hindsight, though, i would say that yes, this should have been the default in scripts. Oh well.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by jamiesonbecker on Tuesday March 24, 2015 @11:24AM (#9257173)
There are lots and lots of these 'nuanced rules'.

http://mywiki.wooledge.org/BashFAQ/105

Re:Bug 1202858 – Restarting squid results in deleting all fi... by anh79 on Tuesday March 24, 2015 @04:54AM (#9255478)
set -u is good. set -e requires to change a lot of code. See for example

https://github.com/icy/bash-coding-style#set--e

Re:Bug 1202858 – Restarting squid results in deleting all fi... by DSMan195276 on Tuesday March 24, 2015 @01:23AM (#9255028)
That's not completely true. At least with the GNU tools, 'rm' won't delete the root directory unless you specifically give it the '--no-preserve-root' flag. Since that flag has no use outside of deleting root, it's unlikely it has the flag on it. With that in mind the script must do some type of manual deleting for some reason.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by viraptor on Tuesday March 24, 2015 @01:29AM (#9255038)
I believe that "--preserve-root" applies only to / itself. That means `rm -rf /*` will expand to `rm -rf /bin /dev /etc /lib ...` and delete all anyway.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by DSMan195276 on Tuesday March 24, 2015 @01:46AM (#9255069)
That's accurate. `rm -rf /* ` will still work to delete everything. But that said, `rm -rf "$STREAMROOT/"` can't ever expand to that, and more-over since the expansions in double-quotes it won't be subject to path expansion by bash. So even "/* /", which would normally expand into "/bin/ /dev/ /etc/ ..." won't. You can see what I mean yourself, just use echo:

    `echo /* `: /bin /dev /etc /lib ...

    `echo /*/`: /bin/ /dev/ /etc/ /lib/ ...

    `echo "/*"`: /*

    `echo "/*/"`: /*/
If you try it with `ls`, you'll find that `ls "/* "` results in `ls: "/* ": no such file or directory`.

Edit: Formatting.

Re:Bug 1202858 – Restarting squid results in deleting all fi... by pskocik on Tuesday March 24, 2015 @02:40AM (#9255167)
The original example was `rm -rf "$STREAMROOT/"*`, though (the asterisk being out of the double quotes.) Now that glob will expand.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by DSMan195276 on Tuesday March 24, 2015 @02:58AM (#9255213)
Ah, my apologies, I think that was HackerNew's markup at work. The '* ' wasn't there when I looked before.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by mirkules on Tuesday March 24, 2015 @02:31AM (#9255148)
This just happened to my coworker today. I'm sitting behind him telling him which commands to type (he's new to Linux...) when suddenly he jumps the gun and pushes enter just as I say "slash". My heart nearly stopped. I didn't even know preserve-root existed (plus I always iterate not to log in as root). It was a snapshotted vm but we still would have lost the day's work.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by arthurcolle on Tuesday March 24, 2015 @04:43PM (#9259347)
Coworker - intern? :P
Re:Bug 1202858 – Restarting squid results in deleting all fi... by jhallenworld on Tuesday March 24, 2015 @01:35AM (#9255050)
It should optimize for this case and run mke2fs instead.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by kevan on Tuesday March 24, 2015 @02:00AM (#9255094)
In the original Steam bug the asterisk is outside the double-quotes and path expansion definitely applies.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by a13xb on Tuesday March 24, 2015 @01:18AM (#9255021)
I feel like it would be a frighteningly common bug. I remember one like this from 2011 [1]. Install/packaging/utility scripts usually do not get as much attention and testing as the application code itself.

[1] https://github.com/MrMEEE/bumblebee-Old-and-abbandoned/issue...

Re:Bug 1202858 – Restarting squid results in deleting all fi... by userbinator on Tuesday March 24, 2015 @04:14AM (#9255373)
I'd say the fact that these bugs only very occasionally happen - relative to the huge number of shell scripts out there that are being executed every day - that it's not really "frighteningly common". You only hear about the ones that fail.
Re:Bug 1202858 – Restarting squid results in deleting all fi... by MichaelGG on Tuesday March 24, 2015 @11:45AM (#9257305)
By the same logic, memory safety issues only happen rarely, right? Most programs/scripts are going to be tested if part of a distribution and such errors removed. But without polling people it'd be hard to know of the many times this kinda thing messed things up. I personally wiped out a production DB due to expanding an unset variable (fortunately immediately after taking a backup.