(Ovid - hand-rolled CGI review) Re: variables not posting?

[Ovid]

stuffy claimed:

Before anyone rips me for not using CGI, I want to let it be known that I am unable to use CGI for this application due to the server I will be using it on.

Okay, I'm an idiot and I know better, but I'll bite. What is so different about your server that you can't use a standard module that's virtually guaranteed to already be present?

Also, you do know that your CGI form processing code is so riddled with bugs as to be virtually useless, right? Let's go through your code line by line:

  1. read (STDIN,my($temp), $ENV{'CONTENT_LENGTH'});
  2. my (@pairs) =split(/&/,$temp);
  3. my($item) = "";
  4. foreach $item(@pairs) {
  5.         my($key,$content) =split (/=/, $item, 2);
  6.         $content=~tr/+/ /;
  7.         $content=~s/%(..)/pack("c",hex($1))/ge;
  8.         my($fields{$key})=$content;
  9. }

1. Line 1: Only does POST, not GET.
2. Line 1: Why don't you check to see if the read was successful?
3. Line 1: You don't verify that the amount of data read is the same as $ENV{'CONTENT_LENGTH'}.

Hmm... three problems and we're still on the first line

4. Line 2: The semicolon is an alternate delimeter. An agent submits data using that and your code breaks. Of course, since you can't guarantee that the data in $temp isn't corrupt...
5. Line 3: Misplaced. $item should be scoped in the for loop:

   for my $item ( @pairs ) {

6. Line 4: See line 3 comment above.
7. Line 5: If an equals sign is submitted in form data, it is encoded as %3D to avoid clashing with the name/value pair delimiter. Therefore, the third argument to split is superfluous (though I admit that I'm just nitpicking now).
8. Line 6: What about the key? Spaces are allowed in the keys, also. If you say, "yeah, but this is only for my forms", than you deliberately limit all future programs you write because you didn't bother to address this now. Don't forget to think about what you might need to use this for later.
9. Line 7: See line 6 comment above.
10. Line 8: Did you know the query string color=red&color=blue is quite valid? You code breaks on that.
11. Line 9: I can't find a problem with this line.

I don't mean to come across as harsh, but this is the reason why people say "don't hand-roll this stuff!" Read what merlyn wrote about how to get use CGI.pm when it's not allowed on your server.

Just looking at your code, one can tell that you have some basic programming issues to learn (sanity checking, scoping, the benefits of strict, etc). Do you really assume that your code snippet is superior to the collective wisdom of thousands of programmers the world over?

I realize that you said you were a newbie. Here's my confession: when I was a newbie, I also preferred to "roll my own." It took a lot of time for me to get over my basic stubborness and see the error of my ways. But let's keep this last paragraph between us, shall we? ;-)

Cheers,
Ovid

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.