Re: Pulling a blank SQL row, but there are no blank rows.

there are no rows that have blank info

How have you verified this?

Here is redraft of the code with some of what morgon was discussing and what I'd call better practices (declare vars as used, RaiseError, lowercase var names, etc). Note that qq is the same as ". So qq~fatalsToBrowser~ is just a confusing way to write "fatalsToBrowser" (or 'fatalsToBrowser' since there is no interpolation). A more common operator for export lists is qw which will apply quotes to a "list."

use warnings;
use strict;
use DBI;
use CGI::Carp qw( fatalsToBrowser );
use CGI qw( header );
use HTML::Entities qw( encode_entities ); # Updated and below.

my $attr = { RaiseError => 1 };

my $statement = <<"";
      SELECT top 1 Quote, CharacterName, GameTitle
        FROM dbo.GameQuotes
    ORDER BY +newid()

my $dbh = DBI->connect('dbi:ODBC:SQLServer', '', '', $attr)
    or die "$DBI::errstr";

my $sth = $dbh->prepare($statement);

$sth->execute;

my ( $quote, $character, $game ) = 
    map { encode_entities($_) }
    $sth->fetchrow_array;

print header();

print <<"";
<blockquote>
   <p>$quote</p>
   <cite>&ndash;$character <br />
      $game</cite>
<blockquote>

$dbh->disconnect;
[download]

Comment on Re: Pulling a blank SQL row, but there are no blank rows. Select or Download Code

Replies are listed 'Best First'.
Re^2: Pulling a blank SQL row, but there are no blank rows. by afoken (Chancellor) on Jun 14, 2009 at 07:06 UTC
My notes: I would move the connect attributes into `DBI->connect()` instead of using a constant variable. I would not use an empty marker in a here-doc: It is too easy to accidentally remove the empty line, and it is not obvious WHY you need that empty line. Use __END_OF_SQL__ if you don't find a marker with a better name. I would move the SQL statement into `$dbh->prepare` instead of using a constant variable. Using a variable only makes sense if you want to modify the initial statement before passing it to the database. You write data fetched from the database into an HTML document without properly escaping the data. The CGI module has the escapeHTML method for exactly this purpose. A CGI program is exposed to the network, maybe even the internet. But I don't see you enabling the taint mode. Start the script with `#!/usr/bin/perl -T` to reduce the risk of using input without proper validation. Enable taint checks in DBI for the same purpose: Add `Taint => 1` to the `connect` attributes. `fatalsToBrowser` exposes information about the script that you don't want to give out to an attacker. It is fine for debugging, but evil for prodduction servers. Remove it or enable it only on debugging servers: `use CGI::Carp; BEGIN { CGI::Carp->import(fatalsToBrowser) if $DEBUG; }` Alexander -- Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)	[reply] [d/l] [select]
Re^3: Pulling a blank SQL row, but there are no blank rows. by Your Mother (Archbishop) on Jun 14, 2009 at 17:10 UTC
Good advice. But not for me. :) This was a style/clarity update based on the original code, largely to just get RaiseError in there, not the way I would write it from scratch. HTML escaping is almost always a good idea (unless your data is stored that way) and I meant to put that in; I would use HTML::Entities instead so when code is taken out of CGI.pm, as it eventually would be in a perfect world, that's one less line to edit. I'll amend it. Empty (\n) HEREDOC markers became a little vogue in... CDBI? I think? And I prefer it. I've always found them less jarring than starting something by calling it the _END.	[reply]