Bitte um kurzes Review

[Alexander (HH)]

Moin Moin!

#!/usr/bin/perl -w

-T fehlt.

Set configuration constants

use constant FEED => '../RealWorld/xhtml/news.xml';
use constant FEED2XHTML => '../RealWorld/feed_atom.xsl';
use constant XHTMLTEMPLATE => '../RealWorld/template.xsl';

Wer oder was garantiert Dir, dass das Script im "richtigen" Verzeichnis aufgerufen wird? Absolute Pfade würden meiner Paranoia weniger weh tun.

use constant PLAINCACHE => 'feed.tmp';

Das aktuelle Verzeichnis ist für den WWW-User (sprich: das GESAMTE Internet) BESCHREIBBAR? SOFORT ABSTELLEN!

Die Temp-Datei ist ohnehin unnötig, siehe unten. Ansonsten benutze ein Modul, das sicheren Umgang mit Temp-Files garantiert.

Initialize variables

my $cgi = new CGI or die('Error while initializing CGI');
my $xslt_param_plain = ' --novalid --nonet';
my $xslt_param = $xslt_param_plain;
my $id = $cgi->param('id');
my $cat = $cgi->param('c');
my $mode = $cgi->param('m');

Filter parameters

$id = ($id and $id =~ /^\d$/) ? $id : undef;
$cat = ($cat and $cat =~ /^\w+?$/) ? $cat : undef;
$mode = ($mode and $mode =~ /^\w+?$/) ? $mode : undef;

Compose xsltproc parameters

$xslt_param .= " --param entry $id"     if ($id);
$xslt_param .= " --stringparam category  "$cat"" if ($cat);
$xslt_param .= " --stringparam param "m=$mode"" if ($mode);

Umständlich. Und wegen dem system("xsltproc") weiter unten und dem -T von oben nicht wirklich guter Stil.

my @xslt_with_arguments=('/usr/bin/xsltproc');
push @xslt_with_arguments,'--param','entry',$1 if $id=~/^(\d+)$/;
push @xslt_with_arguments,'--stringparam','category',$1 if $cat=~/^(\w+)$/;
push @xslt_with_arguments,'--stringparam','param',$1 if $mode=~/^(\w+)$/;

Auf diese Art sparst Du Dir jede Menge Ärger mit der Shell (siehe unten) und übergibst xsltproc nur geprüfte Parameter.

Transform the feed

if ($mode and $mode eq 'raw') {
print "Content-type: text/xml\n\n";
system('cat '.FEED);

Wer garantiert Dir, dass nicht irgendwo in $ENV{'PATH'} ein cat-Programm existiert, das etwas völlig Unerwartetes macht (böse Sprüche ausgeben, Festplatte plätten, Oma umbringen)?

Minimale Perl-Lösung:

open FILE,'<',FEED or die; print while <FILE>; close FILE;

} else {
print "Content-type: text/html\n\n";
system("/usr/bin/xsltproc $xslt_param ".FEED2XHTML.' '.FEED.'>'.PLAINCACHE);
if ($mode and $mode eq 'plain') {
  system('cat ' . PLAINCACHE);
} else {
  system("/usr/bin/xsltproc $xslt_param_plain ".XHTMLTEMPLATE.' '.PLAINCACHE);
}
}

END OF SCRIPT

Ganz übel!

Parameter solltest Du in einem Array übergeben, statt sie erst zu einem String zusammenzukleben, die die Shell wieder auseinander pflücken muß (und dabei gelegentlich katastrophalen Mist baut).

Temp-File ist unnötig, Du kannst das Programm direkt nach STDOUT schreiben lassen. Fester Name für das Temp-File bedeutet Datenverlust bei gleichzeitigem Zugriff mehrerer Benutzer.

perldoc perlipc: Safe Pipe opens sollte Dir helfen. Im Child machst Du jeweils ein exec @xlst_with_arguments ohne IO Redirection, im Parent ein print while <KID_TO_READ>.

Noch eleganter ist allerdings, da das Script hier ohnehin zu Ende ist, es direkt nach der Ausgabe des Headers mit exec @xlst_with_arguments durch xsltproc mit den passenden Parametern zu ersetzen.

Alexander