Forum: Ruby Does it make sense to use rm_rf here?

3a826228ecbbcfaa49c51167df244117?d=identicon&s=25 John Smith (bluesaber27)
on 2017-03-01 09:43
A few months ago I used someone's code on Github and it had this code in
it:

def build(root)
+    FileUtils.rm_rf root
+    self.each do |current|
+      FileUtils.mkdir_p root + '/' + current[:path]
+    end
+  end

Link to full code:
https://github.com/Bahanix/aMazing/commit/94ae581f...


Unfortunately, I didn't properly research what it did and it wiped out
my harddrive. I contacted the author and his reason for having rm_rf is:
"I built it in mind to replace the target directory in order to be able
rerun the script multiple times if the maze was not good."

So i asked whether he tested his own program and he said: "I always used
it specifying the root of the maze (and not the root of the filesystem),
so it has never been a problem to me."

Is he telling the truth or is he just trying to placate me because he's
scared that i tracked down his email address? What made me suspicious
was the fact that in the usage example he used the root directory which
made me think he was trying to wipe out my whole harddrive as it
wouldn't make sense to have a maze there. Also, when i used it, i set
the destination as my desktop.

I know this is a stupid thread, but i just want to know whether he
really didn't mean any harm or not so that i may be able to forgive him
and let this go. And yes, I know I'm an idiot.
0fa73332c8e4a3b06ea439fd3f034322?d=identicon&s=25 Ronald Fischer (rovf)
on 2017-03-02 13:35
It is not uncommen to have a rm -rf, or the equivalent in the respective
language, in one's program. Needless to say that this **is** very
dangerous, but sometimes it is the right thing to do. Of course, with
inappropriate parameters, you can wipe out things you want to keep. To
erase the whole drive is rare (it means that you would run this code
with root permissions, which you certainly won't do unless you have
really good reason for it), but it is bad enough if it erases your home
directory.

There is no general way to safeguard it. If it is an interactive
application, it might makes sense to ask for confirmation before doing
so, but this isn't always practical either.
3a826228ecbbcfaa49c51167df244117?d=identicon&s=25 John Smith (bluesaber27)
on 2017-03-02 13:58
Oh ok, so how was i meant to use the program correctly? I put in my
desktop directory as the place to put the maze and it ended up wiping
out my desktop and then making the maze. Unfortunately for me, i put
pretty much everything on my desktop for convenience sake so I got
screwed bad.
0fa73332c8e4a3b06ea439fd3f034322?d=identicon&s=25 Ronald Fischer (rovf)
on 2017-03-02 14:54
I don't know, what a "Desktop directory" is, but how can it delete
everything, unless you run it as root?

Actually, if I try out a program which is supposed to create a lot of
files/directories, I run it in some temp directory to observe the
behaviour, and I actually cd to this directory.

In particular in this case, the README - that part which says "Result" -
would already suggest that probably nothing is left in the root
directory except what maze had put into it. I would see this danger,
simply because the documentation doesn't say otherwise: If a program is
supposed to create a whole directory structure, and the docs don't say
explicitly that nothing is touched of what is already there, I would at
least consider the possibility that everything else will be removed.

Of course in this case, the program could have checked before, if a
non-empty root directory exists, and stop running, unless a special
--force flag is supplied on the command line. I would do this if I would
write such a program, simply because I could also accidently invoke with
a directory which I don't want to erase. But this is just something on
Github, and only the first commit (after this, the project seemed to
have been abandoned). I wouldn't run such a program without at least
having a quick look at the code what it is doing....
3a826228ecbbcfaa49c51167df244117?d=identicon&s=25 John Smith (bluesaber27)
on 2017-03-02 15:26
By desktop directory I mean I put my desktop as the destination to
create the maze. I don't think i typed in root in the CMD prompt. Also
yeah, I realized i should've researched the code before i ran it, i let
my guard down and now i've been punished.
4828d528e2e46f7c8160c336eb332836?d=identicon&s=25 Robert Heiler (shevegen)
on 2017-03-03 23:20
I do not know what the code does, but I remember some years ago when I
wrote a ruby app, my supervisor asked me if it is dangerous to run the
code. He was quite scared of a problem because he was running as
superuser usually, and he was in charge of the local bioinformatics
cluster of the company - at that point I also got a bit nervous, so I
looked through the code I wrote and I did not see anything where it
could
screw up. But rm_rf actions CAN be scary - always pay good attention to
them! They scare me more than simply file-delete actions.

And I also realized that, when it comes to delete actions, in particular
removing directories, things can be really, really scary. While it was
mentioned to "do not run as superuser", it being a valid comment, let's
face it - people may be lazy or also do mistakes. It happens.

Ronald Fischer already gave one example how proper code could handle
it - e. g. ask interactively whether you are sure to delete a directory
like that.

In my own code, I also make sure that '/' can never become a target.
(I have no code that would ever warrant getting rid of '/' but I
do actually remember that I once, many years ago, also wiped out
my hdd; but this was via bash, when I was hitting tab complete
but was too swift with the space-character and then hitting enter,
I ended up having something like "rm /foo / bar" or something like
that. Since that day I usually do backups regularly. :D)

Anyway, I think that the code at:

https://github.com/Bahanix/aMazing/commit/94ae581f...

is bad in general.

Or perhaps it is just so very different from how I would write this.

I would always use a generic delete/remove method, and inside of
that method, handle any additional checks. Such as querying for
file permissions and doing any other safeguards. I found that
this is usually a lot better than just directly calling
FileUtils.rm_rf() which sounds scary.

Last but not least, I would recommend you to also look at how many
issues a project has; sometimes the more issues that are CLOSED
the better, more people to look at a project AND also have a look
at rubygems.org - if a project has had lots of releases, it may
be that it is of quite good quality.

By the way, since you contacted the dude, this actually gave him
the possibility to also learn. I am sorry for the loss of your
data but I think this also shows that making backups is really
important. I don't even trust myself or my own code fully, even
though I try to make it as good as possible. :D

One day we will be replaced by computers and they will write
much better code. Until then, we will write disaster code!
0fa73332c8e4a3b06ea439fd3f034322?d=identicon&s=25 Ronald Fischer (rovf)
on 2017-03-04 09:11
Robert Heiler wrote in post #1185641:
> In my own code, I also make sure that '/' can never become a target.

Of course this doesn't protect agains, for instance, someone using /home
or a /home/user/I/think/I/am/safe, which happens to be symlinked to
/home ....

There is no foolproof safeguard. We just have to be as careful as
possible. Of course, taking the viewpoint of a devil's advocate, we can
say that wiping out important data is a good test case to measure the
quality of our backup-policy. After all, a disk crash could erase the
data too.

> Anyway, I think that the code at:
>
>
https://github.com/Bahanix/aMazing/commit/94ae581f...
>
> is bad in general.

I would say that the programmer hasn't yet much experience. You can see
this also by the lack of comments. And, it is the first version. Maybe
the programmer lost interest in maintaining the code. I don't think he
would produce such code in a production quality environment, so it was
maybe intended as an exercise to program in Ruby.

> I would always use a generic delete/remove method, and inside of
> that method, handle any additional checks. Such as querying for
> file permissions and doing any other safeguards. I found that
> this is usually a lot better than just directly calling
> FileUtils.rm_rf() which sounds scary.

Yes and no. Seeing it from the viewpoint of a Ruby application, this
certainly makes sense. I think the majority of rmrf in this world are
buried in shell script applications (at least this is the case for my
projects), and, while possible, it is much less common to use a generic
removal function when doing scripting.

Ronald
Please log in before posting. Registration is free and takes only a minute.
Existing account

NEW: Do you have a Google/GoogleMail, Yahoo or Facebook account? No registration required!
Log in with Google account | Log in with Yahoo account | Log in with Facebook account
No account? Register here.