On Mar 14, Jame Brooke said:
Program password.pl
This program is rather poorly written, and appears to have been "inspired" by some very old Perl programs lying about on your server.
#!/usr/local/bin/perl
#windows environment
#Read in Parameter passed
require "cgi-lib.pl";
&ReadParse (*input);
Eww. EWW. Use the standard CGI module, and turn on warnings. I'm also going to suggest turning on strictures, which will require you to write safer code. In this case, it primarily means declaring your variables with my():
#!/usr/bin/perl
use CGI; use strict; use warnings;
my $input = CGI->new; # create a new CGI object from the form data
$name_status =$input{'name'}; #Read User ID from HTML form
$pass_status =$input{'pass'}; #Read User Password from HTML form
my $name = $input->param('name'); my $pass = $input->param('pass');
open(PASSWORD,"password.dat");
$pass_count=0;
while (!eof(PASSWORD)) {
$name[$pass_count] = &get(PASSWORD); $pass[$pass_count] = &get(PASSWORD); $pass_count++;
}
close (PASSWORD);
First, I'm going to clean this code up, and then I'm going to explain why it's the wrong way to do things. Let's look at the get() function:
sub get { local ($handle)= $_[0]; local ($str)=""; local ($ch);
while ($ch ne "\n") { $ch=getc($handle); $str=$str.$ch }
return $str; }
What is this, C? This function is a waste of time to write, and a waste of time to use. Instead of called get(PASSWORD), do <PASSWORD> instead.
my (@names, @passwords);
# when you try opening a file, give an error if you fail!
open PASSWORD, "< password.dat" or die "can't read password.dat: $!";
while (my $n = <PASSWORD>) {
my $p = <PASSWORD>;
chomp ($n, $p); # remove the newlines from the end
push @names, $n;
push @passwords, $p;
}
close PASSWORD;#Check Whether the user name and user password match with password file or not
print "Content-type: text/html\n\n";
print "
<html>
You should use a here-doc, it's nicer-looking:
print << "END_OF_HTML"; Content-type: text/html
<html> ... <table> END_OF_HTML
for($i=0; $i<$pass_count;$i++) {
This is a C-style loop over an array, and it's rather ugly.
if ($name[i] = $name_status) {
if ($pass[i] = $pass_status) {
Here's your big problem. You're missing the $ on $i, and you're using '=' instead of 'eq'. You need to compare strings, not set a variable to a certain value.
if ($names[$i] eq $name) {
if ($passwords[$i] eq $pass) {
# print success message
last; # <-- THIS will exit the for loop
}
}But here's how your code should really look. You don't need to store the usernames and passwords in arrays in your code, because you only need to look at ONE username and ONE password at a time. And if you DID want to store them, you should use a hash, not an array.
Here's how I'd write the code:
#!/usr/bin/perl
use CGI; use strict; use warnings;
my $query = CGI->new;
my $name = $query->param('name');
my $pass = $query->param('pass');# ...
open PASSWORD, "< password.dat" or die "can't read password.dat: $!";
while (my $n = <PASSWORD>) {
my $p = <PASSWORD>;
chomp ($n, $p); if ($n eq $name) {
if ($pass eq $p) {
# the names match and the passwords match
}
else {
# the names match but the passwords don't match
} last; # stop looping over the file
}
}
close PASSWORD;Another problem I see here is that password.dat lives in the same location as your CGI program (uh oh) and stores passwords in PLAIN TEXT (UH OH!).
-- Jeff "japhy" Pinyan % How can we ever be the sold short or RPI Acacia Brother #734 % the cheated, we who for every service http://japhy.perlmonk.org/ % have long ago been overpaid? http://www.perlmonks.org/ % -- Meister Eckhart
-- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>
