> -----Original Message-----
> From: Jeff 'japhy' Pinyan [mailto:[EMAIL PROTECTED]
> Sent: Monday, March 14, 2005 7:44 AM
> To: Jame Brooke
> Cc: beginners@perl.org
> Subject: Re: matching password problem
>
> 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;
Well, it did technically solve his problem :) Seriously though, good catch
and nice detailed explanation.
Chris
>
>
>
> > #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>
>
>
>
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>
