Beefy Boxes and Bandwidth Generously Provided by pair Networks
Syntactic Confectionery Delight
 
PerlMonks  

Some advice on another's scripts

by Mr. Muskrat (Abbot)
on Jun 25, 2002 at 17:17 UTC ( #177143=perlmeditation: print w/ replies, xml ) Need Help??

My fellow monks:

I spoke with some of you this morning in the CB concerning the following matter. I was looking through a linux website and came across an article on a perl script. So I followed the link and looked at the code. What I found was poorly written at best lacking things. It uses global variables, no does not 'use strict', no use of does not use -w, etc...

I then looked at a CGI script at the same site written by the same person and it was even worse. There was no taint checking even though it is receiving input, no -w, no 'use strict', global variables, and he sent fatals to the browser. He uses CGI.pm but hand rolls most of his HTML. (Which is okay but if your gonna use it, use it to it's fullest!) I contacted the man via email telling him how he should change things for the better (trying to stress the security related items). I sent him a link to PerlMonks as well as Ovid's wonderful CGI Course. He replied saying that, yes he should follow most of my suggestions but it would have to wait for the next version when he migrates it to mod_perl. He did not, however, mention the poor security of his scripts. (see updates below)

I am not an expert perl programmer. I am not a security professional. I haven't even had the time to fully go through all of the code yet. I felt that I should make the author aware of the mistakes and that I should show him a cleaner way of producing HTML in CGI scripts.

Did I do the right thing? I think so.

Did I take it too far by telling him to use CGI.pm to it's fullest? Maybe... it is primarily a personal preference.

Now that he has told me that he does not want to fix his code until he is ready,
That is not what I meant...
Now that he has told me that the update will have to wait until he releases the mod_perl version, what should I do?
Have I done enough by letting him know?
Or should I post his URL as a warning to others?
(He posted a message to the web page saying that it does not do taint checking.)

Updated*6:
I sent a follow up message about the security issues and received a reply: "I will indeed look at that more closely. If you find any holes, do let me know."
To which I replied, "Would you like some experts to look at it? If so, then I'll post your URL at PerlMonks. Many of the perl gurus frequent the site."
Here is the URL he replied with: Programming Methods
So the cat is out of the bag now... FutureSQL is the CGI script in question. I just noticed that there are demos available. SSL demo or Normal demo

peterbrown, you will get your ++ tomorrow :) It was a bit misworded but I have been trying to clarify it. I have also been making updates as the day progressed based on our conversations. Had I waited until after work to post, this would have been much clearer from the start.
I am glad to see you made it here!<br (strikethroughs and bold text above was editted during the updates)

Comment on Some advice on another's scripts
•Re: Some advice on another's scripts
by merlyn (Sage) on Jun 25, 2002 at 17:23 UTC
    Or should I post his URL as a warning to others?
    If the code is published on the web, it is subject to peer review.

    I'd say publish the URL, but ethically, you should let him respond to your criticism, and update your comment if the referenced material changes favorably.

    -- Randal L. Schwartz, Perl hacker

Re: Some advice on another's scripts
by vladb (Vicar) on Jun 25, 2002 at 17:25 UTC
    I think you did a conscious thing by pointing the guy to the wrongs he has done. For the most part, I wouldn't bother doing the same thing if I were in your shoes. So, I would command your courage and selflessness.

    In addition to the points of improvement you've suggested, I'd also go as far as saying to use a templating system instead of simply writing HTML inside your script. There's a lot of wonderful tools out there. samtregar's HTML::Template is one of them.

    I would like to also respectfully disagree with your "use CGI.pm to it's fullest" statement, though. Just how do you go and measure that? In most of my scripts, I use CGI to retrieve parameters sent to the script, process cookies, file uploads and so forth. However, I don't particularly favor CGI's html producing features. Certainly, this probably stems from the fact that I'm big on templates. ;-).

    _____________________
    # Under Construction
      I'd really love to see a "Perl Style and Security Coding Guide" (hopefully with a better name) that covered all of these things. For myself, I have written a lot of garbage scripts that exist out there on the net right now without -w, use strict, taint, etc. I didn't know better when I was starting to lean Perl because there were no resources (especially no single resource) that covered these. The books I learned from never even touched on tainting, or if they did no "Why this is important" info was given. Kickstart
        I'd have to agree with Kickstarts's comment 100%. I too have coded several Perl scripts that get the job done, but not necessarily in the best way. After having hung out on PM.org, I've picked up things like -w and use strict, but I feel there is much more to learn before my code is to be considered "well written". I learned Perl from several different books and am still continuing along the path of Perl wisdom. It would be great to have a tutorial on proper Perl style, security issues, modules etc. I know I'd reference it at least once a day! Monks?
        Ovid's excellent CGI course is much of what you ask for; I try to plug it at every opportunity.

        Makeshifts last the longest.

Re: Some advice on another's scripts
by insensate (Hermit) on Jun 25, 2002 at 17:28 UTC
    You've done enough. Unsolicited critisim of any code is a very delicate subject. What the author decides to do with your justifiable concerns is a function of his or her integrity as a publisher. There is a whole lot of bad code floating around on the net and it is fortunate when someone such as yourself is observant and concerned enough to take the iniative to draw attention to the present dangers. You should sleep well tonight knowing that you've done all you can and should. If this author fails to publish an addendum I will be greatly discouraged by his or her lack of care for the audience.
    -Jason
Re: Some advice on another's scripts
by Marza (Vicar) on Jun 25, 2002 at 17:54 UTC

    You done good. The fact you publish something opens you to critisim. Most writers already know that. A good one will evaluate what you say and either change or offer an argument back.

    I once contacted an author for training book on IIS about several mistakes. Some he felt where justified and others he thought were simply my opinion. So there is no harm in pointing something out.

    If the author gets pissed then he won't be writing long.

    Heck I bet even Merlyn gets contacted on stuff he published.

      What better way to learn than hearing some little(or big) pointers from your peers!

      Seriously, don't worry about it! Have you ever been in a simmilar situation, where you got a bit of a code review? Sure, it is a humbling experience, but at the same time, you feel great to know that someone out there wants to help? Isn't that the beauty of the open source community??(other than sharing with others of course)

      IMHO, you did this guy a favor, and like Marza said, if he gets pissed then he won't be writing long.

      --~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--
      perl -e '$a="3567"; $b=hex($a); printf("%2X\n",$a);'
      --~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--~~--

Re: Some advice on another's scripts
by peterbrown (Novice) on Jun 25, 2002 at 19:19 UTC
    As the author in question (grimace), I feel obligated to respond.

    I do want to correct what I perceive to be a misunderstanding of my intent, when Mr. Muskrat states:

    "Now that he has told me that he does not want to fix his code until he is ready, what should I do? Have I done enough by letting him know? Or should I post his URL as a warning to others? "

    I don't think I meant "I'm not going to fix it until I'm good and ready and to heck with the consequences." I'm very concerned about the viability of my scripts. If there's a serious and urgent flaw, I want to fix them immediately.

    I do, however, have to schedule things, since I have a day job.

    I responded to your email immediately, Mr. Muskrat, because I AM concerned. In fact, based on your email, I immediately posted a page at:

    http://worldcommunity.com/opensource/programming.methods.html

    My goal is to become a better and better programmer. As a working programmer, the last thing I want is to be considered a 'hack'. I profoundly appreciate everyone's advice, and will do my best to improve.

    Oh... and in response to a post above about html templates -- FutureSQL is built around an extensive template system, so that the code is separated from the html.

    Thanks :-)

    Peter F. Brown

Re: Some advice on another's scripts
by ignatz (Vicar) on Jun 25, 2002 at 21:44 UTC
    I would agree that those who posts code online are subject to peer review. This standard also applies to writers of critiques. I'm wondering how I would feel if I read a review of a book and the reviewer admitted to just skimming it and not being an expert on the subject at hand.

    Parroting "use strict" and "-w" without backing it up is not a valuable critique. How would CGI.pm fare under that standard? TMTOWTDI. If you are going to attack someone else's work, take the time to show the specific reasons behind your comments.

    Did you do the right thing by bringing your concerns to the attention of the author? By all means. Did you do the right thing by turning your concerns into an cursory unsolicited public critique of someone else's code? I'm not so sure.

    ()-()
     \"/
      `                                                     
    
      ignatz,
      I couldn't show specifics without exposing the code which may have exposed who the author was. I was trying to make it an anonymous post. I did not give the url or so much as the name of the script until I told to post the url to peterbrown's write up on his programming practices. This post started as a question. That question was answered. I updated the info as it was made available to me.
Re: Some advice on another's scripts
by Anonymous Monk on Jun 26, 2002 at 00:10 UTC
    Be careful with handing out advice that you aren't damned sure of.

    For instance telling him to use CGI.pm's html generating methods is crap. Search for "separation of content and presentation" some time. (Templating as well.) While handrolled HTML is bad, at least it is easier to move between that and editing complex HTML using standard web design tools than it is to have to mentally translate everything to and from Perl calls just to see what changed.

    Just because CGI is supposed to be a good module, and it offers those functions, doesn't mean that it is a good idea to use those functions for anything other than toy examples!

Re: Some advice on another's scripts
by Mr. Muskrat (Abbot) on Jun 26, 2002 at 13:18 UTC
    This started out as a question and ended up a circus of sorts. Not at all how I wanted it to end.
    peterbrown, I apologize for bringing this up here. Had I waited until I made it home yesterday, this probably would not have been posted.
    And I really will read through your code thoroughly starting this evening like I said I would.
      Dear Mr. Muskrat,

      I appreciate and accept your kind apology, and also the remarks of Brother Frankus and others.

      I'm truly appreciative of advice and eager to learn more. As perhaps many programmers are, I'm self-taught by trying to inhale book after book and examine other's code. I would be really nuts to think there's nothing more to learn.

      In fact, as I mentioned in my initial email to you, the points you raised were excellent -- I plan to integrate most of them in a substantial rewrite of FutureSQL that will make FutureSQL compatible with mod_perl.

      As you mentioned, security really is a gigantic concern. I would indeed appreciate feedback about any security holes in my scripts. I also have been looking for feedback about the session id security method that I created in FutureSQL, using a hand-roled XOR method. See Security Text for details.

      My biggest worry about this posting has been that I make my living (supposedly :-) as a programmer. I try to really do my best to serve my clients with code that works and doesn't break, and is hopefully robust. So far, my clients have given me pretty good marks, as one could see from their quotes at: Programming Information.

      In other words, my reputation (if I have one :-) is important to me, and having esteemed programmers such as the Perl Monks conclude that I was a careless and/or unconcerned programmer would be a dreadful thing, in my opinion.

      Thank you once again for your technical recommendations!

      Best regards,

      Peter

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://177143]
Approved by broquaint
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (15)
As of 2014-09-16 17:15 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (36 votes), past polls