> If someone could point me in the right direction...
> I have a form on a system I am maintaining that works fine most of the
> time, but sporadically spits out an error. Users hit the back button,
> only to find that the data they had filled in is no longer in the form
> (but sometimes it stays).
The form not still being filled in sounds like a browser issue rather
than a scripting issue, does it exhibit itself when using the same
browser for multiple failed queries? On top of the fact that this is
browser (version) implementation specific it will also depend on the
chosen cache settings in the browser used, at least for some browsers.
> The system is using latest RH9 apache2, perl, msql2.
> Other forms on the system work fine.
>
Wow, msql, been a while...
> Some details:
> Here is the most common error on screen (in the browser) that I can
> reproduce:
> parse error at line 1 near ", ,"::S1000
In general I believe the line number in the above error represents the
line number of the query *not of the perl script*.
> However, according to error_log for httpd, the error is on line 130 of
> the cgi:
> $sth->execute;
Right which is the line that executes 'line 1' of the query.
> The $sth is executing an insert query to insert all of the values from
> the form into the msql database. I *think* the parse error is referring
> to this query. So there's something wrong at the time of the query.
> The query lists its values in this manner:
> ,\'$form{pub_title}\', # so a hash ref
This is confusing to me, why are the single quotes escaped? Is this in a
larger double quoted string?
> but some are referenced directly as variables
> ,$num_pages,
> which were given the value of the $form{num_pages} earlier, and then
> checked with eval().
Right because they are numbers which don't need to be quoted, though
this whole scheme is not the best way to handle things, see below.
> if ($num_pages){
> $num_pages = eval($num_pages);
> }
As to why the num_pages is being checked like this I have no idea, there
is *no* reason to do this, not to mention it is insecure, slow, yada
yada yada... yikes in fact. What happens when someone comes along and
decides to put arbitrary Perl code in the form, for instance an 'unlink'
or something. If you want to check 'num_pages' for a number (integer
presumably and not 0), then something like:
if ($num_pages =~ /^[1-9]+$/) {
# good a number
}
Would be much better. Unless you know why you need an 'eval' you don't.
When you understand why you would need an eval, you wouldn't use it for
doing the above kind of things.
> But if I comment out these eval lines it does not change the behavior of
> the page. Seems like old debugging cruft from a previous coder.
It is a good thing to do form validation, just don't do it the way they
did it.
> I don't see this doubled comma from the parse error anywhere in the
> code, and the query syntax seems kosher, so I guess maybe it's getting
> into one of the variables somehow? I'm definitely not putting it into
> the form.
> To summarize my questions:
> --Where can I read the code for a good example of such a cgi page? (I am
> already studying what the oreilly books have to offer)
That is probably a good start. Ask specific questions on this forum.
Presumably you should also read the documentation for the CGI.pm module
and the DBI module (you are using it right?).
> --Could this be caused by a variable being null?, hence , , instead of ,
> value ,?
Yep. But again I don't understand where the ? is coming from, is that
an explicit question mark or one of the form entry?
> --Any debugging advice for forms? i.e. best way to check/correct form
> data as it submits, so that the db on the receiving end will be happy?
Well not specifically about your form data, as that is a rather generic
question, your form data needs to be checked or not depending on how
"dumb" your user is likely to be and how weird your data will be.
I would highly suggest using binding variables for SQL (insert)
statements. If you haven't already done so check out the binding section
of DBI's docs. This should allow you to separate the building of your
statements from the actual values to be inserted. This will also
prevent the need for all the escaping of quotes, whether or not to quote
numbers, etc. which is so very nice.
I generally use something along the lines of:
my $fields = '';
my $values = '';
my @bind = ();
# in here do your form validation, field setting and value string setting
if ($num_pages =~ /^\d+$/) {
$fields .= ', ' if ($fields ne '');
$fields .= 'num_pages';
$values .= ', ' if ($values ne '');
$values .= '?';
push @bind, $num_pages;
}
# setup the rest of our statement
my $st = "INSERT INTO table_name ($fields) VALUES ($values)";
my $sth = $dbh->prepare($st);
unless ($sth) {
# error here
}
unless ($sth->execute(@bind)) {
# error here
}
You get the general idea. Obviously checking multiple values in a loop
may help depending on the type of values, etc. This will also help you,
though it less frequently used on the web, prepare the statement once
but do multiple insertions of records in the cases where that is desired
which provides a significant speed increase for large amounts of data.
If you aren't using the modules listed you should be (at least for DBI,
CGI is up for some debate though I would suggest it). If you don't have
use strict and use warnings at the top of your script you should. If the
original author didn't do these things, unless the script is over a
couple hundred lines (even still maybe) you are better off scrapping it
and starting fresh. You *may* increase your development time on this
particular project, but you will avoid repeating bad habits that plague
what you have.
http://danconia.org
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
