ckluver has asked for the wisdom of the Perl Monks concerning the following question:

I have been instructed to expand on the website of my company, which uses perl scripts for the password protected area. I have little Perl experience. The following code is a script I wrote to add a user to database(which is actually just a text file). For some reason it won't write to text file. It is basically pulling a company code and password off of an HTML document and writing to a text file. Does anybody have any idea? 
$UserDB = "datahere/UserDB.txt";

print "Content-type: text/html\n\n";

# Parse Form Contents
&parse_form;

# Add to user database
&add_db;

# display results
&results;

sub parse_form {
    if ($ENV{'REQUEST_METHOD'} eq 'GET') {
          # Split the name-value pairs
        @pairs = split(/&/, $ENV{'QUERY_STRING'});
    }
    elsif ($ENV{'REQUEST_METHOD'} eq 'POST') {
        # Get the input
        read(STDIN, $buffer, $ENV{'CONTENT_LENGTH'});
 
        # Split the name-value pairs
        @pairs = split(/&/, $buffer);
    }

    foreach $pair (@pairs) {
          ($name, $value) = split(/=/, $pair);
            $value =~ tr/+/ /;
            $value =~ s/%([a-fA-F0-9][a-fA-F0-9])/pack("C", hex($1))/e
+g;
          $Form{$name} = $value;
        }
}

sub add_db {
      open (FILE,">>$UserDB");
   print FILE "$Form{'companycode'}\t";
   print FILE "$Form{'password'}\t";
    print FILE "\t\n";
      close(FILE);
}

sub results {
   print "<html>";
    print "<p>User Is Added</p>";
   print "</html>";
}

[download]

Replies are listed 'Best First'.
Re: Get Form Data/Write to Text File
by Joost (Canon) on Jun 04, 2007 at 19:42 UTC
    A couple of hints:

    Looking at the code, you're using some very outdated (and problematic) constructs:

    1. You're not using warnings / -w or strict. That will hurt you when your script grows any larger than this. It's also cumbersome to retro-actively make scripts strict-compliant. You should make it a habit to start any script you write with 

    #!/usr/bin/perl -w
use strict;

    [download]

    2. Calling subroutines as &sub_name; passes the current argument list to the called sub. That's almost never what you want. To be safe you should call subroutines with parentheses, i.e. sub_name( possible arguments here )

    3. You should probably not parse CGI parameters by hand. For one thing, your code doesn't handle multi-part encoded forms or the - suggested but not required by the standard - use of ";" instead of "&" as a parameter separator. The canonical module to handle CGI parameters is CGI from the standard distribution.

    4. You don't check if your open() call succeeds. Also, using the 3-argument form of open is clearer and has less "issues". See the link. Something like open (FILE,">>",$UserDB) or die "Can't open $UserDB: $!"; is a better way of handling this.

    5. It looks like your password file is accessible to outsiders via the webserver. You should probably put it somewhere outside the document root.

    Sidenote: You're writing passwords to a file, but you don't seem to be checking if the current user has permission to do that. Note that ANYONE who opens the url "http://yourhost/yourscript.cgi?password=mypassword&companycode=mycompanycode" can currently add a password.

    You're also not using the passwords anywhere else. Is this part of a larger system?

    "What should it profit a man, if he should win a flame war, yet lose his cool?"
[reply]
[d/l]
[select]
Re: Get Form Data/Write to Text File
by FunkyMonk (Bishop) on Jun 04, 2007 at 19:39 UTC
    For some reason it won't write to text file. It is basically pulling a company code and password off of an HTML document and writing to a text file. Does anybody have any idea?

    The open is probably failing. Ask perl to tell you why by changing your open to:

    open (FILE,">>$UserDB")
    or die qq{can't open "$UserDB": $!};

    [download]

    I know you have little experience of Perl, but this really isn't very good code. Where did you get all your bad habits from?

    • Don't write your own form parser, use CGI
    • Make sure all your scripts use strict and warnings until you know the reasons not to use them
    • Are you running in taint mode? If not see perlsec
    • Don't call subroutines like &parse_form;. See perlsub for how you should do it.
[reply]
[d/l]
[select]

      Be nice. ;)

      The OP said I have little Perl experience which also means he has very few Perl habits, bad or otherwise. The strong implication is that this is in the context of inherited (and likely fairly old) code. I'd imagine that the OP was feeling a bit battered by the code before coming here for help, so a gentle "here are some things to watch out for" will go down much better than the implied "here are some dumb ass things you did".

      ++ for helping the OP learn by highlighting issues and pointing to documentation, but be a little aware of how tone affects message: it's the sugar as helps the medicine go down.

      DWIM is Perl's answer to Gödel
[reply]
        yellow sugar
[reply]
Re: Get Form Data/Write to Text File
by bradcathey (Prior) on Jun 05, 2007 at 01:25 UTC

    Hmmm, this looks like the parsing code from the old Perl/CGI Quickstart book that got me in trouble with the monks back in '03.

    Besides the obvious (not using CGI), do consider a database like MySQL to do the heavy lifting for you. It's almost a language unto itself, but it will make storing and retrieving data so much easier. And chances are, it's installed on your web server already.

    And if you do get into MySQL, read the amazing DBI recipes


    —Brad
    "The important work of moving the world forward does not wait to be done by perfect men." George Eliot
[reply]
Re: Get Form Data/Write to Text File
by perlfan (Parson) on Jun 04, 2007 at 19:40 UTC
    Try defining the path to the text file as absolute.

    Also, you'll be saving yourself a heck of a lot of trouble if you use the Perl CGI module.
[reply]
Re: Get Form Data/Write to Text File
by naikonta (Curate) on Jun 05, 2007 at 06:28 UTC
    There are many good advices already, so I just want to add that you may need check the permission of the database file. You can consult your local docs for how to set file permission.

    Open source softwares? Share and enjoy. Make profit from them if you can. Yet, share and enjoy!

[reply]
      Thank you O wise monk. You were correct. I needed to set the file permissions on the database and the Perl Script
[reply]

Back to Seekers of Perl Wisdom