Output problem after query is run

**Lizzy** · 05-20-2005, 08:23 AM

Hi,
I have a problem with my output from my script after the script has executed, my output is in a long string, the data is correct but the format is incorrect.

Could someone show me how to format my output into a more readable format please? in a table or something.

The script takes input from a form and runs a query against a database of cars, the values applicable to the query (the results) are then output.

The values are all registration numbers and colours of cars which match the make and model chosen in the form.

At the moment my output looks like this:

Code:

BlackRedYellowGreenGreenWhiteBlueBlue  
GA53EVZKK50ZBKNV51KYURH50EEQTZ50STLVO50OZYXY50BMNYC51KYY

What I want is something like this :

Code:

GA53EVZ  Black
KK50ZBK   Red
etc

Here is my script:

Code:

#!/www/perl/bin/perl
use CGI;

use DBI;

use diagnostics;

use warnings;

use CGI::Carp qw(fatalsToBrowser);

$CGI::POST_MAX = 128;
$CGI::DISABLE_UPLOADS = 1;

$model=CGI::param("model");
$make=CGI::param("make");
#connect to database giving pass and username
if (!($dbh = DBI->connect("DBI:mysql:dbname:localhost", 'uname', 'passwd')))
{
        &outputErr("couldn't connect to database".DBI->errstr);
}



if (!($query = $dbh->prepare("SELECT registration, colour FROM car
WHERE make = '$make'
 AND model = '$model'  ")))
{
        &outputErr("couldn't prepare statement".$dbh->errstr);
}

#execute the query
if (!($query->execute()))
{
        &outputErr("couldn't execute statement".$query->errstr);
}

while (@car = $query->fetchrow_array())

{

print $registration.=$car[0];

print $colour.=$car[1];


}

$query->finish;

$dbh->disconnect;

print <<PAGE;
Content-type: text/html \n\n
<html>
<head>
<title>Details</title>
</head>

<body bgcolor="#E7E7E7">
  <table border="1">
  <tr>
  <td>
  $colour
  </td>
  </tr>
  <tr>
  <td>
$registration
  </td>
  </tr>
  </table>
  </body>
</html>
PAGE
exit 0;

**jason** · 05-20-2005, 09:00 AM

These two lines:

print $registration.=$car[0];

print $colour.=$car[1];

are taking each registration number and each color and sticking it onto the end of a string with the rest of the numbers/colors. You then output those strings into your table.

What you really want to do is start building your table before your start stepping through your query results. Then for each result row you will add another row to your output table. HEre's a psudocode example of what you want to do:

[code]setup database connection
execute query
<table>
<tr><th>Colour</th><th>Registration</th></tr>
loop through each query result {
<tr><td>colour</td><td>registration</td></tr>
}
close database connection
</table>

Here's your code with the changes added:

Code:

$model=CGI::param("model");
$make=CGI::param("make");
#connect to database giving pass and username
if (!($dbh = DBI->connect("DBI:mysql:dbname:localhost", 'uname', 'passwd')))
{
        &outputErr("couldn't connect to database".DBI->errstr);
}



if (!($query = $dbh->prepare("SELECT registration, colour FROM car
WHERE make = '$make'
 AND model = '$model'  ")))
{
        &outputErr("couldn't prepare statement".$dbh->errstr);
}

#execute the query
if (!($query->execute()))
{
        &outputErr("couldn't execute statement".$query->errstr);
}

print <<PAGE;
Content-type: text/html \n\n
<html>
<head>
<title>Details</title>
</head>

<body bgcolor="#E7E7E7">
  <table border="1">
    <tr><th>Registration</th><th>Colour</th></tr>
PAGE

while (@car = $query->fetchrow_array())

{

print "<tr><td>$car[0]</td><td>$car[1]</td></tr>";


}

$query->finish;

$dbh->disconnect;

print <<PAGE;
  </table>
  </body>
</html>
PAGE
exit 0;

My perl skills are a little rusty these days, but I'm pretty sure that will work.

Hope this helps.

--Jason

**Lizzy** · 05-20-2005, 09:11 AM

Excellent! Thankyou !

**Spathiphyllum** · 05-20-2005, 01:21 PM

A couple of points:

Add 'use strict;' now, before it's too late.

Second, your script seems to accept tainted values from a CGI query. That's a bit risky as a rule. Perhaps taint checking is in order? Or did you just leave that stuff off for brevity since it didn't address your main issue?

Strict and Taint

**Lizzy** · 05-20-2005, 03:59 PM

If I use strict I seem to get a load of errors, that's why I didn't use it, and I don't understand taint.

Lizzy

**Lizzy** · 05-20-2005, 04:08 PM

Code:

#!/www/perl/bin/perl -wT

use strict;

use CGI;

use DBI;

use warnings;

use CGI::Carp qw(fatalsToBrowser);

$CGI::POST_MAX = 128;
$CGI::DISABLE_UPLOADS = 1;

my$model=CGI::param("model");
my$make=CGI::param("make");
my$dbh;
my $query;
my @car;
#connect to database giving pass and username
if (!($dbh = DBI->connect("DBI:mysql:dbname:localhost", 'uname', 'pass'))) {
        &outputErr("couldn't connect to database".DBI->errstr); }



if (!($query = $dbh->prepare("SELECT registration, colour FROM car WHERE make = '$make'
 AND model = '$model'  ")))
{
        &outputErr("couldn't prepare statement".$dbh->errstr); }

#execute the query
if (!($query->execute()))
{
        &outputErr("couldn't execute statement".$query->errstr); }

print <<PAGE;
Content-type: text/html \n\n
<html>
<head>
<title>Details</title>
</head>

<body bgcolor="#E7E7E7">

  <table border="1">
    <tr><th>Registration</th><th>Colour</th></tr>
PAGE

while (@car = $query->fetchrow_array())

{

print "<tr><td>$car[0]</td><td>$car[1]</td></tr>";


}

$query->finish;

$dbh->disconnect;

  print <<PAGE;
  </table>
  </body>
</html>
PAGE
exit 0;

**Spathiphyllum** · 05-20-2005, 09:44 PM

Lizzy,

Strict is used to impose helpful guides while you code. If there are lots of errors, it reveals places where your code has likely break points. The sooner you use strict when coding, the better off you'll be long term since backtracking to clean "unclean" code is quite difficult.

Taint is a different matter altogether. The idea is a security feature that forbids you from using unchecked input that doesn't explicitly originate from within your program.

You've partially implemented the right tools. Use either -w or warnings but not both - 'use warnings;' is better. When using 'my' to restrict the namespaces, don't forget the space between 'my' and '$variable'.

I've included a sample script you can run and play around with. It's very crude but will allow you to enter two command line values and see the results of your regex parsings. Run it on the server or on a local debugger. The main untainting is done through the regex rule and the returned stored variable $1.

At the command line if running in a shell session:

/path/to/testtaint.cgi [string1] [string2]

Code:

#!/usr/bin/perl -T
#
########################################
#
# testtaint.cgi ver 0.01 [22:44 5/20/2005]
# taint checks sandbox
#
# [www.histosoft.com] or [www.myhistosoft.com]
#
#  May 20, 2005
#  Copyright(c)2005 HistoSoft Corporation. All Rights Reserved.
#
########################################

use strict;
use warnings;

# Variables
my $input1 = 'oops1';
my $input2 = 'oops2two';
my $output = 'Nothing to parse';

# Get your Standard Input
$input1 = shift @ARGV if @ARGV;
$input2 = shift @ARGV if @ARGV;
print "User entered: $input1 $input2\n\n";

# Set your REGEX (some examples)
unless ($input1 eq '') {
  $input1 =~ /^(\D{1,6})/; # If first value matches 1-6 non-digits, store the match
  $input1 =  $1;           # Take stored value and assign it to $input1
                           # (untainted now)
}

unless ($input2 eq '') {
  $input2 =~ /^(\w{1,20})/;# If second value matches any 1-20 letter "word", store the match
  $input2 =  $1;           # Take stored value and assign it to $input2
                           # (untainted now)
  $input2 =~ s/\D+//g;     # Remove non-digits
}

  $output = $input1 . '/' . $input2; # assign input to output with some separator
  $output = uc $output;    # for grins, make it uppercase when possible

print "After parsing: ";
print $output;             # Return parsed string

exit;

Play around with it for a while and if you need more help, come on back.

**Spathiphyllum** · 05-21-2005, 01:10 AM

Lizzy,

Here's some code you can use directly in your script. This entire script can be run from command line again but use the following format:

/path/to/makemodel.cgi make=[string1];model=[string2]
* don't actually include the [brackets]

Or to try via browser:
http://www.yourdomain.com/cgi-bin/pa...odel.cgi?make=[string1];model=[string2]

If you don't want to use the entire script but only want to incorporate the untaint code relevant for your script, just use the blue text in your program. This select code will check your CGI input, format it, and return it to your program for the SQL query. If the user CGI input is inproperly entered, it will return an error and log it to screen and to errorlog.

Code:

#!/usr/bin/perl -T
#
########################################
#
# makemodel.cgi ver 0.01 [02:10 5/21/2005]
# taint checking again
# [www.histosoft.com] or [www.myhistosoft.com]
#
#  May 21, 2005
#  Copyright(c)2005 HistoSoft Corporation. All Rights Reserved.
#
########################################

use strict;
use warnings;

use diagnostics;
use CGI;
use CGI::Carp qw(fatalsToBrowser);

my $query;
$query = new CGI;
print $query->header(
    -lang     => 'en-US',
    -encoding => 'iso-8859-1',
    -type     => 'text/html'
);

print $query->start_html( -title => 'Lizzy TEST' );
print <<XHTML;
<h1 style="text-align: center; color: #233e7d";>Practicing Untaint!</h1>
XHTML

my $model = '';
my $make  = '';
my $err1  = 'failed';
my $err2  = 'failed';

$model = CGI::param("model") if CGI::param("model");
$make  = CGI::param("make")  if CGI::param("make");

# registration number?
# Memorize input if it is 7 letter alphanumeric - only!
# Assign untainted value to $model
# Capitalize all letters and return it
# Consider and report input as clean
if ($model =~ /^([A-Za-z0-9]{7})$/) { #$model =~ /^(\w{7})$/; #similar version of this line
	$model =  uc $1;
	$err1  = 'passed';
}

# report previous output here
# comment after debugging
print "<p style=\"text-align: center;\">model: $model - $err1</p>\n";

# color?
# Memorize input if it is between any 3 to 6 letters - only!
# Assign untainted value to $make
# Capitalize the first letter and return it
# Consider and report input as clean
if ($make =~ /^(\D{3,6})$/) {
	$make =  ucfirst $1;
	$err2 = 'passed';
}

# report previous output here
# comment after debugging
print "<p style=\"text-align: center;\">make: $make - $err2</p>\n";

unless (($err1 eq 'passed') and ($err2 eq 'passed')) {
  warn  "Invalid input for model or make\.\n";
# report error to browser as error handler
# comment after debugging or use your regular error routine
  print "<br /><p style=\"color: red; text-align: center;\">Invalid input for model or make\.</p>\n";
}

print $query->end_html();

exit;

Also, see http://search.cpan.org/dist/perl/pod/perlre.pod