Problem that I can't find

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

Taking Ovid's advice in Before you post ..., I tried to get to the root of the problem before posting the question.

The script that I am writing will be used to run a website of a bookstore and at the moment, I am working on the catalog search part.

The script goes to the search part of the srcipt and runs all the lines of it until it seems to stop at some point among the lines:

open(DATA, "books.txt") or die "error opening file $!"; 
 
while (<DATA>) {
    @bookinfo = split(/\|/, $_);
    chomp(@bookinfo);
    if ($q{'lang'} eq '' || $bookinfo[1] eq $q{'lang'}) {
        $matchingwords=0;
        foreach $word(@search) {
            if ($_ =~ $word) { 
                $matchBooks[$numMatches] = @bookinfo unless ($matching
+words>0);
                $numMatches++;
                $matchingwords++;
            }
        }
    }    
}

if (@matchBooks>0) {

my $sortby;
if ($q{'sort'} eq 'author') {$sortby = 3;} else {$sortby = 2;}
@matchBooks = sort {${$a}[$sortby] cmp ${$b}[$sortby]} @matchBooks;

my $firstDis;
my $lastDis;
my $firstDisp;

if ($q{'first'}) {
    $firstDis = $q{'first'}; 
    $firstDisp = $q{'first'};} 
else {
    $firstDis = 1; 
    $firstDisp = 1;
}

if ($firstDis + 9 < $numMatches) {$lastDis = $firstDis + 9;} 
else {$lastDis = $firstDis + ($numMatches - $firstDis);}

$firstDisp = $firstDisp - 1;

print '<b>Matching Books</b><br>';
print 'There are '.@matchBooks.' matches.<br>';
print "${firstDis}-${lastDis} are listed below.<br><br>";
print "<nl>";

while ($firstDisp < $lastDis) { ... }
[download]

Why is it doing this?

Thanks in advance.

--Zenon Zabinski

Comment on Problem that I can't find Download Code

Replies are listed 'Best First'.
Re: Problem that I can't find by Ovid (Cardinal) on Jul 01, 2000 at 10:05 UTC
Well, I verified that your code compiles. However, I'd have to see some sample data before I could figure this out. A couple of things that you are doing don't seem terribly intuitive to me, so if you post some sample data (that you've tested and know will make the code fail), I'm sure some Monks would tackle it. I'd suggest e-mailing it to me, but I'm moving tomorrow and won't have time to look at it. Here are some thoughts on the code. Use the taint switch -T!!! It's trivial for a user to alter the form and resubmit with data that can wreak havoc on your site. When you untaint data, make sure that your allowed information is restricted as TIGHTLY AS POSSIBLE!!! For example, when you are testing `$q{'command'}`, try the following: `$q{'command'} =~ /^(search\|\s)$/; $command = $1; # $command is now untainted.` [download] That's pretty bulletproof. `$command` can only equal 'search' or spaces. No malicious code can sneak in. I included the possibility of spaces because you were also testing for an empty string, which appeared to be acceptable. Any Monks know how to make that match 'search' or undef?* Hmm, you could just match for search and use this: `defined $command && $command eq 'search' ? &search : &main; # gosh, I +love the trinary operator :)` [download] Possible problem: is there any chance that `$q{'first'}` can have a non-numeric value? It's used to assign to `$firstDis`. This is later found in: `while ($firstDisp < $lastDis) {...}` [download] Since '<' is a numeric comparison, this will cause a problem. I think it's a longshot, but it's all I can tell without seeing your data. Have you tried running the script from the command line with a debugger? If you do that, step through the code and print out the value of variables in trouble spots. You'll likely find the problem. Also, use CGI.pm. It's much more robust than the code you have above. All of the code you have to get the form values can be substituted with the following: `use CGI; my $q = new CGI;` [download] Then, to access a particular form value, such as `$command`, use the following: `$q->param('command') =~ /^(search\|\s*)$/; # untaint 'command' my $command = $1; # $command is now untainted.` [download] It doesn't matter if `method` is `get` or `post`. You can read about it here. Good luck!	[reply] [d/l] [select]
Re: Problem that I can't find by ar0n (Priest) on Jul 01, 2000 at 15:46 UTC
In addition to Ovid's great (!) comments, see if you can use an HTML template system (there recently was a question on this, search). It helps to seperate the html from the code. I myself have used HTML::Template, and found it very easy to use. Put the header and footer files together into one file, and see how to use <TMPL_VAR> and <TMPL_LOOP> (from HTML::Template perldoc page), depending on your needs. Also, I see in your header: `<form action='index.pl' method='get'>` Someday, try replacing index.pl with $ENV{SCRIPT_NAME}, or, if using HTML::Template, <TMPL_VAR NAME="this_script"> Then in your script replace this_script with $ENV{SCRIPT_NAME}: `$template->param(this_script => $ENV{SCRIPT_NAME});` Your script name is bound to change sometime. -- ar0n	[reply] [d/l] [select]
Re: Problem that I can't find by chromatic (Archbishop) on Jul 01, 2000 at 20:40 UTC
Is <nl> valid HTML? I've not seen it used before. Depending on your browser, it may not display anything in between those tags. (It does for me on Netscape 4.crashcrash2 on Linux.) If you switch to CGI.pm as Ovid suggests, you can test your script from the command line -- highly recommended. I'd also change your loop to this (personal preference): `for ($firstDisp .. $lastDis) { print "<li>" . $matchBooks[$_][2] . ".<br>" . $matchBooks[$_][3] . "." . $matchBooks[$_][8] . "<br>" . $matchBooks[$_][4] . "." . $matchBooks[$_][6] . "</li>"; }` [download] It's ugly, but I test that way just to be sure that my complex data structure elements print out correctly. (I go back later and fix it.)	[reply] [d/l]
(zdog) Re: (2) Problem that I can't find by zdog (Priest) on Jul 01, 2000 at 22:21 UTC
You are right, `<nl>` is not valid html, I meant to put `<ol>`, but when I went back a changed it, it did not make a difference, it still didn't work. Thanks though. -- zdog (Zenon Zabinski) Go Bells!! 'ż'	[reply] [d/l] [select]
Re: Problem that I can't find by tenatious (Beadle) on Jul 02, 2000 at 07:25 UTC
My guess is that there is some metacharacter that the browsers you use don't like (quotes, lessthan, greaterthan, etc). Particularly when you are working with titles of books. If any of the data you are trying to print contains quotes, you are going to run into problems. Check the HTML output that is being generated to make sure that everything is being written out. It would help to see the HTML output (not what it looks like on the screen but a copy of the HTML that you'd see from an "view source" command).	[reply]
(zdog) Re: Problem that I can't find by zdog (Priest) on Jul 02, 2000 at 19:59 UTC
Update: After a day and a half of manual debugging, I figured out what the problem is. The script can't print the sub-elements of @matchBook because I am assigning the array @bookinfo as an element of @matchBook incorrectly. It should be: `$matchBook[$numMatches] = [@bookinfo] unless...` [download] Thanks for the help though. -- Zenon Zabinski Go Bells!! 'ż'	[reply] [d/l]