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

This node falls below the community's threshold of quality. You may see it by logging in.

Replies are listed 'Best First'.
Re: To register or not (2)!!!
by tachyon (Chancellor) on Dec 31, 2002 at 23:23 UTC

    There are dozens of errors in your code both syntactic and logical. The code below will compile and run and probably do what you want. Please study it.

    A few of the errors corrected were:

    1. bad indentation making it impossible to see extra { or } braces
    2. using q( ...$some_var... ) ie trying to interpolate into a literal string. "name: $name" or qq!name: $name! will interpolate whereas 'name: $name' and q!name: $name! will not.
    3. See how you get values passed to a sub (using @_ or shift) - you were not ever going to get a value in $email in the check email sub.
    4. Your check email logic was totally screwed up
    5. password() very broken. There a 21 consonants so you need rand 21 BTW
    6. Your printing to file neglected newlines and used literal tabs which makes it hard to see. also had an inexplicable concatenation.
    7. Your HTML comes from frontpage by the look of it. I removed the web-bot timestamp and put in a perl one ;-)
    8. failure to check return values on file opens with the error
    9. opening a file in one place then using it in a sub then forgetting to close it. just bad practice
    use strict; use diagnostics; use warnings; use CGI; my $q = new CGI; my $user_name = $q->param('user_name') || ''; my $gender = $q->param('gender') || ''; my $real_name = $q->param('real_name') || ''; my $city = $q->param('city') || ''; my $country = $q->param('country') || ''; my $email = $q->param('email') || ''; my $confirmemail= $q->param('confirmemail') || ''; my $mm = $q->param('birthmm') || ''; my $dd = $q->param('birthdd') || ''; my $yy = $q->param('birthyy') || ''; my ($title, $body, $i, $s, @v, @c); my $file = 'c:/test.file'; #--------------------------------------------------------------------- #START OF MAIN PROGRAM #--------------------------------------------------------------------- my $password = password(5); my $dateofbirth = "$dd/$mm/$yy"; my @info = ($user_name, $password, $email, $gender, $real_name, $dateo +fbirth, $city, $country); if ($email ne $confirmemail){ ($title, $body) = &email_invalid(); } elsif (&check_user_email($user_name,$email) eq "good"){ ($title, $body) = &good(@info); } else { ($title, $body) = &check_user_email($user_name,$email); } print qq(Content-type: text/html\n <html> <head> <title>$title</title> </head> $body </html> ); #--------------------------------------------------------------------- #END OF MAIN PROGRAM:BEGINING OF VERIFICATION #--------------------------------------------------------------------- sub email_invalid { my $title = "Email's do not match!!"; my $body = q(<body bgcolor="black" text="red"> <h3>Your e-mail doesn't match.</h3><br><br> <h2>Please click on your browsers back button and try again.</h2>< +/a><hr> </body>); return ($title, $body); } #--------------------------------------------------------------------- sub check_user_email{ my ( $user, $email ) = @_; open USRNF, $file or die "Couldn't find user file $file, Perl says + $!\n"; # generate a lookup hash from the file while(<USRNF>) { chomp; my @userinfo = split /\t/, $_; next unless $userinfo[0] and $userinfo[2]; $hash{$userinfo[0]} = $userinfo[2]; } close USRNF; if ( exists $hash{$user} ){ # username exists, two posibilities if ( $hash{$user} eq $email ) { my $title = "Account already activated.!"; my $body = qq| <body bgcolor="black" text="red"> <h3>Our records show that you already have a valid account +.</h3><br><br> <h2>If you wish to create another one you must first delet +e the old one.<br></h2> <h3>Please email me<a href="mailto:eoinmurphy\@dublin.com" +> here</a> to edit any account info. <hr> </body> |; return $title, $body; } else { my $title = "User name already in use!"; my $body = qq| <body bgcolor="black" text="red"> <h3>The username $user is already in use.</h3><br><br> <h2>Please click on your browser's back button and try a d +ifferent one.</h2></a><hr> </body> |; return $title, $body; } } # if we have not returned by here then the username is unique so.. +. return 'good'; } #--------------------------------------------------------------------- sub good { my @info = @_; my $title = "Registration successful!"; my $revised = scalar localtime; my $body = <<HTML; <body> <h1>Registration Confirmation</h1> <hr> <p>Dear, $real_name</p> <p>Thank you for registering with us. You have supplied us with the following information:</p> <blockquote> <p> <strong>Real name:</strong>\t$real_name <br> <strong>Username:</strong>\t$user_name <br> <strong>Password:</strong>\t$password <br> <strong>E-mail:</strong>\t$email <br> <strong>Date of Birth:</strong>\t$dateofbirth <br> <strong>City:</strong>\t$city <br> <strong>Country:</strong>\t$country <br> </p> </blockquote> <p>If any of this information is incorrect, please email me <a href="mailto:eoinmurphy\@dublin.com">here</a> to change it. I reserve the right to delete any user if he/she use this account for illegal purposes.</p> <blockquote> <p>Sincerely,</p> <p><em>Webmaster, Eoin.</em></p> </blockquote> <hr> <h5>You may return to the feedback form by using the <em>Back</em> button in your browser.</h5> <h5>Revised: $revised</h5> </body> HTML open USRNF,">>$file" or die "Couldn't find user file $file, Perl s +ays $!\n"; print USRNF join "\t", @info; print USRNF "\n"; close USRNF; return $title, $body; } sub password{ my $s = shift; srand($s ^ time); @c = split //, "bcdfghjklmnpqrstvwxyz"; @v = split //, "aeiou"; my $password = ''; $password .= $c[int(rand(21))] .$v[int(rand(5))] for 1..4; return $password; }

    cheers

    tachyon

    s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

Re: To register or not (2)!!!
by pfaut (Priest) on Dec 31, 2002 at 20:50 UTC

    eoin, this appears to be a new revision of the same script that has been discussed here, here, here, and again here. If you kept the discussion in one thread, it might be easier for people to figure out what's been discussed about it before.

    --- print map { my ($m)=1<<hex($_)&11?' ':''; $m.=substr('AHJPacehklnorstu',hex($_),1) } split //,'2fde0abe76c36c914586c';
      this appears to be a new revision of the same script that has been discussed here, here, here, and again here.

      ...oh. I feel dumb now. Is there any mechanism that can be used to consolidate the response threads? Is that even logical?

      ...All the world looks like -well- all the world, when your hammer is Perl.
      ---v

        Sorry all, i've been kind of busy trying to work all the errors out of the program. And there where alot of them, believe me.
        This program takes info from a html form(http://eoinmurphy.netfirms.com/reg.htm), checks the info for errors in the info and then compares it to a txt file. If everything works out ok it'll record the same info to the file. Users can then use this info to log in. When i check the script in dos, it scrolls the html code for the sub good down the screen. The html code is too long, so all i see is the end of the code. and the I can't see the top of the error where it says whats wrong. Can anyone help???