On "fixing" Perl core modules "in place"

[tlm]

I'm about to blow a gasket. Hopefully writing this question for my fellow monks will let me blow off some steam.

Today, suddenly several of my scripts were failing, out of the blue. After looking in all the obvious places for what the problem could be, I found out that someone in our team had decided to "fix" /usr/lib/perl5/5.8.6/CGI/Carp.pm. (Yes, the copy used by every other user of our system.) The "fix" actually produced a version of CGI/Carp.pm that didn't even compile...

(I shudder to think how many other such "fixes" have slid through undetected.)

In the aftermath of the whole incident, the person who instigated the fix conceded that the process of applying this "fix" had not been executed properly, but still he insisted that he saw nothing wrong, in principle, with replacing /usr/lib/perl5/5.8.6/CGI/Carp.pm with a home-doctored version.

This is when I had to bite my tongue, count to 10, and focus on not going ballistic.

The thing is, it seems to me so obvious that one shouldn't muck with anything in /usr/lib/perl5 that I actually have a hard time explaining it to someone who doesn't already see it. So maybe it's all just a prejudice on my part...

(To add irony to injury, the motivation for the "fix" was that this person considers that CGI::Carp produces error messages that would be confusing to most users. Now, as merlyn often points out, letting untrusted users see CGI::Carp messages is a security breach, so worrying about confusing users with CGI::Carp messages implies a misuse of CGI::Carp. But, I was already having a hard enough time making the case for the obvious, so I figured I'd leave the (maybe) less obvious for some other day.)

In the past, whenever I haven't liked the way some module from CPAN behaves, I make a copy of the module, put it in my privlib, and change it there. Depending on the situation, I may subclass the original class, or just cannibalize the code I need and stick in a new module. There are many approaches, but the one thing they all have in common is that they leave the shared stuff intact. Isn't this all self-evident?

Any persuasive talking points and any other words of wisdom on the subject would be much appreciated.

(There. I'm feeling much better already.)

[BrowserUk]

The guy did wrong, that's obvious, but I think it indicates a procedural/permissions problem with your development setup also.

Developer's working on multi-user systems and servers should not be able to update system-wide library files. Any and all changes to shared libraries should only be possible from an account specifically charged with systems admin resposibilities. And only after testing has been signed-off.

There should be no need for a developer to have write access to /usr/lib/perl5/*. Even if a developer is specifically charged with modifying a module that normally lives in that pathtree, it is quite easy for him to work on a copy of the module in a private path located ahead of the system path, for his account only.

Once he is satisfied with his changes, the copy should go through at least a minimal regression testing against your existing applications in some sort of test server environment before being signed-off for deployement into shared library trees.

[samtregar]

You are absolutely right. Your co-worker is an idiot. Perhaps you can walk him through the likely fate of such "improvements" when Perl is upgraded or a new machine is built!

I once saw this kind of upgrade performed on Apache::Session. I had found a bug in a contractor's code which could cause deadlocks in Apache::Session. To "fix" the bug the coder removed the locking code from Apache::Session! There's only one word for that - awesome.

-sam

[rhesa]

"brillant" comes to mind... ;)

(See http://thedailywtf.com)

[Old_Gray_Bear]

Well, the first thing I'd do is revoke his (I am making an assumption here that the cowboy was is a boy) Root access on every machine in the company. Then I go to his manager and explain why I took these steps. If the manager has a difficulty with the idea, I move up to the next manager in the food chain. If I don't get support at some level, and they make me put the Jerk back, I smile most politely, acquiese, and immediately start circulating my resume. If I do get support, then the Jerk stays in User-privilege, and we re-build Perl on every machine he had Root access to. And if I am feeling really churchy, we charge the time back to the Jerk's project, as "virus remediation".

[tlm]

One point that I did not make sufficiently clear is that the person who instigated the change was not the one who actually carried it out. There were two people involved. One, Official A, who did not have root access, but who managed to persuade another one, Official B, who did have root access but not too much knowledge of Perl, to do the evil did.

To B's credit, the minute that I voiced my objection to the whole thing, he immediately reversed the change with a "what on earth was I thinking???" apology. Yes, it was a major goof, but very uncharacteristic of B. I can only explain it by A's insistence, and the fact that A is somewhat higher up in the food chain than B.

My big argument is with A, who kept advocating for the change and pressuring B to do it, even after B had come back to his senses.

[gloryhack]

I've done that very thing in the past, and ended up circulating my resume. While it's good advice, it's something one should undertake only after careful consideration of his financial standing. I wasn't fired, but came very close to it because after five minutes of the jerk standing just a foot away yelling at me I suggested (calmly, quietly) that if he continued making me feel threatened I might respond accordingly.

I didn't really want to work for HP anyway.

[jdtoronto]

Yes, you are right! I had a contractor who decided to midfy a module in place - he doesn't do any work for me any longer. If you have to 'fix' a module, put it in your own space and let the programmes that need it does so explicitly. So did he understand what would happen when somebody used CPAN to do an update? Or a new Perl got installed?

We have two modules we have needed to modify for various reasons, on a Tk module and the other is Net::FTP. Both are kept in their own directory within the development space rather than the Perl filespace - both are tested for rigorously in the code that uses them. If somebody gets it wring here than potentially we can have nearly 3000 problems - we have nearly 3000 end users.

jdtoronto

[UnderMine]

Sometimes it's neccessary to enhance the functionality of a core module in order for it to become more usable.

A couple of years ago I had reason to patch Apache::DBI for a project. There were a number of scripts running under modperl that were creating or altering database handles leaving us with a pool of rarely used or polluted handles.

a) First major thing was we HEAVILY documented what we were about to do.

b) Developed the new version under version control.

c) Created a patch file.

d) Patched the server using the patch.

e) Released the patch back to the community.

The patch was not initially accepted but it still has helped out other people and it may eventually find its way in. Discussion about Apache::DBI Patching

Suggestions on how this process could be improved would be welcome.

Thanks

UnderMine

Update Patch was accepted into Apache::DBI. Just did not check before posting. ;)

[derby]

this person considers that CGI::Carp produces error messages that would be confusing to most users

Umm ... did the person even bother to read the docs for CGI::Carp:

You may also pass in a code reference in order to create a custom error message. At run time, your code will be called with the text of the error message that caused the script to die. Example:

    use CGI::Carp qw(fatalsToBrowser set_message);
    BEGIN {
       sub handle_errors {
          my $msg = shift;
          print "<h1>Oh gosh</h1>";
          print "<p>Got an error: $msg</p>";
      }
      set_message(\&handle_errors);
    }

[Argel]

Amazingly I just spent some time skimming through The Practice of System and Network Administration (the book's website, Addison Wesley) but could not find a specific reference addressing this. The closest I noticed was in the Software Depot section where a bulleted list (pgs 491-492) of things to think about touches on the subject: Who can install packages? Who does the quality-control? How are upgrades handled? How are packages deleted?

I think the problem is this is part of the fundamentals of good system administration. Something the stereotypical clueless (about the art of system administration) developer is going to have a very hard time grasping. One obvious approach is to ask him how much he would enjoy explaining to upper management why a fast, undocumented fix he just made broke something highly visible in production. Then ask him why he thinks it is okay to do something similar on the current server.

I agree with the sentiment expressed above about revoking root access (though I would get management buy-in first). There is responsibility that comes with root access that this person clearly does not have.

Or alternatively give him full responsibility for that system (make a backup first). He will quickly learn it’s not as easy or simple as he thought!

You could also have him read some of the BOFH stories and then ask him how he would feel as the luser.

Or, you could take on the mantle of the BOFH and show him the true power of root!! Muahahah!! :-)

[adrianh]

Any persuasive talking points and any other words of wisdom on the subject would be much appreciated.

The sort of questions I would ask would be:

• What happens when we install a new version of Perl?
• What about code that relies on the existing behaviour - you did check that didn't you?
• How do we track that change when we install our software on another machine - you did check it into source control didn't you?
• Can you show me the automated test that highlights the bug in the module (fails with the system module, passes with your fix)?

[Anonymous Monk]

He's an idiot. Then aain, protecting against and fixing the mistakes of idiots with too much power is a large part of what programmers *do*. If it wasn't for cleaning up after overhyped consultants, self-styled experts, and management blunders, dealing with underspecified and mis-specified systems, and other day to day insanity, coding would be child's play.

Anyone can write a system and walk away. The trick is to bulletproof it so that no one else who walks up to it can break it without havin to work at it.

Then again, as the saying goes, "Never underestimate the damage that can be done by a truely determined idiot". That seems to have been the situation in your case. My condolences. :-(

[diotalevi]

You're being unfair or you're not describing what kind of fix it was. Perhaps his bug fix should have been integrated into core perl but hadn't yet. If that's true, fix it in place and send the patch to p5p. I do that all the time to my own system. Lately that's how B::Lint has been progressing - I fix it locally in my own installation and the changes get kicked up to p5p.

[johngg]

I think the OP described a "fix" that didn't even compile. Perhaps "fix" is the wrong word here? The "fix", so called, doesn't sound like something that should be integrated into core Perl to me.

Cheers,

JohnGG

[Fletch]

Perhaps "fix" is the wrong word here?

I took it in a veterinary sense (your pet doesn't "work" too well in some ways after you get them "fixed" either :).

[tlm]

What was done could be described as a "bug fix" only if we expand the definition of "bug" to include something like "an error message that is confusing to the naive user". Given that, as I pointed out, CGI::Carp messages are arguably not meant for the eyes of naive users in the first place, the justification for this particular "fix" was exceedingly slim.

For a single-user system, the approach you describe is probably fine, but, IMO, for multi-user systems the patch should be sent first to the maintainers, and have the fixed version come back as a standard distribution. In the meantime, I patch the version in my privlib and leave everything in /usr/lib/perl5 untouched. After all, "shadowing" of the untouched original version by the patched version in privlib (through a suitable definition of PERL5LIB) works like a charm.

[diotalevi]

For a single-user system, the approach you describe is probably fine, but, IMO, for multi-user systems the patch should be sent first to the maintainers, and have the fixed version come back as a standard distribution.

You're full of it. You can make changes to system libraries before and independantly of the vendor if you document it and make a plan for dealing with this disparity between your system and the rest of the world. You can do what I recommended which was to get that change into the official version or you can maintain your forked version. Either way, it isn't forbidden. It's not a good idea and it's a last resort kind of thing. Doesn't mean it isn't occasionally necessary.

I see that you want to get approval for the idea that it's never ok to change a system library like this guy did. Sure. That's fine. He broke production and didn't bother to test, notify the other users, or get their approval. That deeply, deeply sucks. It doesn't mean it should be impossible for him to write patches. Assuming the guy grows a brain next week and learns how to write valid code and it comes to pass that a core library has a bug, it really might make sense for the thing to be patched.

[bunnyman]

How many other users are affected by your change? How many other programmers? Just you?

If there are other programmers affected, do you notify them in advance so they can be aware of possible issues? Can you be sure you won't cause problems for production services?

It sounds like your own installation is your own personal development playground. That is the correct place to make these types of changes. Production installations should be treated with care, and any changes should be well-tested and well-publicized.

[diotalevi]

How many other users are affected by your change?

Everyone that's going to use the next version of perl whether as a user, developer, or admin. Oh wait, you meant the change that's local that hasn't roundtripped through p5p? I haven't found reason to patch a core module on the production systems but it wouldn't be forbidden if there weren't some better alternative like a newer, fixed version. We're probably in agreement that it'd be really far down on the list of things to try when working around a problem.

[spiritway]

By any chance, was this guy a manager?

[tlm]

No, a developer, but very influential with management, and with way too many years as a software developer under his belt to be coming up with harebrained proposals like this. A very nice guy, BTW, and very good at persuading people. Certainly far better than yours truly, who is prone to go into orbit and self-combust upon learning of such schemes.

[liverpole]

Wow, what a great story!

It brings back the memory of a Q/A engineer I worked with almost 20 years ago.  When I would handoff code to him to test, rather than take a copy of it for himself, he would go into my source directory, and change my Makefile to build it the way he wanted!

Both stories remind me of the quote by Rick Cook:   Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.

[ambrus]

I agree. In theory.

I usually try to avoid modifying system files when it's enough to modify one user's. I remove commands from /etc/profile but don't add commands to it but instead add them to user's profiles.

Once, however, I made the same error. I replaced the system shell (/bin/bash to which /bin/sh was a symlink) with a patched version of bash3. There was absolutely ne reason to do this, as the modification was needed only be one user; wasn't quite important; the patched shell previously worked pretty well installed to my home directory, and later, installed to /usr/local/bin. It was a mistake, I admit, but luckily I saw no harmful effects from it.