in reply to Adding dynamic parameters to a URL

I'm not sure if this is what you were talking about. but it's my guess. This is not a complete script, you need to add the end messages.

It will basicaly assign the email address with an ID and then stores that info in a flat file database. Then when the user clicks the link from the email, the script will check the info from the link with the info in the db and check for a match.
#!/usr/bin/perl # # evs.pl # # use the CGI mod use CGI; use Email::Valid; # new CGI my $CGI = CGI->new(); # define variables my $cmd = $CGI->param('cmd'); my $account = $CGI->param('account'); my $email = $CGI->param('email'); if ($cmd eq "activate") { &activate; # activate the account } else { &verify; # verify the email address and send email } # exit exit; # Verify sub verify { # make sure an email was entered if ( ! $email ) { &error(" No Email Address was entered! "); } # make sure it is a vaid email address if ( ! Email::Valid->address($email) ) { &error(" An invalid Email Address was entered! "); } # Get a new account ID open(FILE, "id.txt") || error("No ID file"); # Make sure yo +u change the path my $ID = <FILE>; close(FILE); # Increase the ID by one $ID++; # Rewrite the ID file open(FILE, ">id.txt") || error("Can't write ID File"); print FILE $ID; close(FILE); # Add this email to the # email db open(DB, ">>email_db.txt") || error("Could not write to DB"); print DB "$ID|$email|0"; # The 0 is a status flag showing close(DB); # that the email address is not activated # Send them an email with # the link and new ID. # I don't know what email server # you're useing but I assume you do. # The link you need will be # www.page.com/evs/evs.pl?cmd=activate&account=$ID&email=$email # then finish with a message # to the user. # ... exit; } # Activate sub activate { # Make sure they included # an account ID and Email Address if ( ! $account || ! $email ) { &error(" An Account ID or Email Address was not included "); } # Open the db open(DB, "email_db.txt") || error("Could not open Email DB"); my @emailDB = <DB>; close(DB); # keep the db my @keepDB = (); # found flag my $found = 0; foreach (@emailDB) { chomp($_); my ($db_ID, $db_email, $db_status) = split(/\|/, $_); # Match the email if ($db_email eq $email) { $found = 1; # We have found the Email # Check to match the ID if ($db_ID eq $account) { $found = 2; # Email found and ID matches } } else { push (@keepDB, $_); # Return to db if it's not the +email } } # Check for some errors if ($found == 0) { &error(" Email Address not in our Database! "); } elsif ($found == 1) { &error(" The Account ID was incorrect! "); } # ReWrite the db to show the this Email # Address has been been activated open(FILE, ">email_db.txt") || error("Could not write email db"); print FILE "$ID|$email|1\n"; foreach (@keepDB) { print FILE "$_\n"; } close(FILE); # Now you can finish the program # as you wish. exit; } # Errors sub error { # get the error message my($error_msg) = @_; # print the page header print $CGI-header(); # print the error print $error_msg; exit; }
If anyone has any suggestions about the script please let me know. I'm somewhat new to perl and could use some constructive criticism about my programming.

Update: Error checking with open, my instead of local, == instead of eq

Replies are listed 'Best First'.
Re: Re: Adding dynamic parameters to a URL
by Fletch (Bishop) on Feb 03, 2003 at 17:39 UTC

    Well, you asked:

    • ALWAYS check the return value from open(); you're inconsistent in your error checking
    • You use local when you mean my in the error routine.
    • You slurp the file rather than reading line by line (granted that if you get to point that that would be an issue you'd really want a real database anyhow . . .)
    • You don't do any sort of file locking to guarantee that you don't clobber the database if multiple people hit simultaneously (or close enough to simultaneously)
    • eq is for strings, == is for numeric comparisons.

    I'll stop there.

      thank you for the help. just one question. the only reason i didn't use file locking was that it was my understanding that not all platforms use file locking in the same way, and as the original question did not say what platform they were using so i left it out.

        See perldoc perlopentut, specifically the section entitled File Locking. Perl will at least try and emulate something resembling flock() everywhere.