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

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". ;-)

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

Replies are listed 'Best First'.
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]

Replies are listed 'Best First'.

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]