Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

I've been trying to get this program to load. You see, it simply takes in the user's information, i.e. the name of the file. It then parses that and puts it into a template. The only problem is that I can't seem to get the template to work right. Does my code look all right? I think the problem is at the end of the code. 
#!/usr/bin/perl -w
use strict;

my $video_info;

# Loads the CGI Module
use CGI;

# creates a new CGI object
my $page = new CGI;

# This will print a standard HTML header
print $page->header;

# Grab a named CGI parameter
my $value = $page->param("video");
#Open up the file that contains the review of the video

open(SEE, "../$value");

###The Shtick for the name in the Title Bar

#split up the variable $value

($Directory, $File) = split(/\//, $value);

#Take the information from the split and split it up a little bit more

($Name, $Extension) = split(/\./, $File);

#More splitting

@Words = split(/_/, $Name);

#Join the formation from the previous split and make it useful

$Title = join(' ', @Words);

#Save the file to an array

@video_info = <SEE>;


#Now print out lots of HyperText Mark-up Language using
# HTML::Template
my $template = HTML::Template->new(filename => '../Template/my.tmpl');
$template->param(info => @video_info);
print $template->output;
exit;

[download]

Replies are listed 'Best First'.
Re: Syntax and code debugging
by Chmrr (Vicar) on Oct 23, 2001 at 06:14 UTC

    A few things, off the top:

    • I'm skeptical as to whether this will work as written. Yes, you have a use strict; at the top, but you don't seem to be my'ing most of your variables.
    • Check the return value of your open call. You're assuming the open succeeded, while it may be failing because of permissions or file not found problems.
    • I'm not a regular HTML::Template user, but as a general rule, putting a use HTML::Template; at the top will probably make the last few lines happier.
    • @video needs to be chomp'd, otherwise both your keys and values will have endlines on them. I'm assuming that the file in question has alternating key/value lines, otherwise you'll need to munge @video into a usable form.
    • You go through all of the trouble of getting the title, but never pass it to the template. You might want to make the call to $template->param() look like: 
      $template->param(TITLE=>$Title,
                 @video);

      [download]
      On a similar note, you can skip the @Words array entirely by saying: ($Title = $Name) =~ tr/_/ /;
    • By blindly passing the contents of a form parameter to open, you could be opening yourself up to a big security hole. You might want to do some validation on $value before you start blindly opening it.

    Update: My list grew from being "two points" to "three" to "fiv -- aww, heck, a few" as I glanced at the code.

    perl -pe '"I lo*`+$^X$\"$]!$/"=~m%(.*)%s;$_=$1;y^+*`^ ve^#$&V"+@( NO CARRIER'

[reply]
[d/l]
[select]
Re: Syntax and code debugging
by chipmunk (Parson) on Oct 23, 2001 at 06:38 UTC
    A few things off the top of my head, to add to Chmrr's advice:

    First and foremost, you haven't actually told us what is wrong. It's not clear what you mean when you say you can't get the template to work right. Does the script not produce any output? Do you get a server error? Have you checked your server logs or tested the script on the command line?

    When you open the file, you should make sure that the open succeeded. Here's how to find out what went wrong when the file can't be opened: open(SEE, "../$value") or die "Can't open '../$value': $!";

    Lastly, it may be the case that, when the script is run by the web server, the current working directory is not what you expect. This is a quirk of some web servers. You can try using absolute paths in your script, to see if that fixes the problem.

    Hope that helps!

[reply]
[d/l]
Re: Syntax and code debugging
by czarfred (Beadle) on Oct 23, 2001 at 08:51 UTC
    A Note On Security:

    Take EVERYTHING said about security VERY seriously. Even using taint mode, and other cautious measures, crackers have a truck load full of tricks and can make your script do *many* things if you are not *very* careful.

    If I were you, I would look around for a more secure way to do this, and if and only if you do not find a better way, then I would move forward, as making theis script secure in the current way you're doing it would be very hard for a beginner. If you decide to go on with, remember that a cracker could easily change values (param()s) passed to your script even if you're using the POST method. They could pass null bytes which might make it through your taint checking, but go through to the underlying c as everything up to the null byte (look around the web for details). This is just one example of something that you should take into consideration.

    Now that I've successfully blabbered my head off, I have two words to say: be careful.
[reply]
Re: Syntax and code debugging
by Desdinova (Friar) on Oct 23, 2001 at 08:30 UTC
    A few things in addition to what is mentioned
    • the obligitory mention of Taint mode. you should add a -T to the /usr/bin/perl -w line. Then you need to check the validity of the input line from the web page. you are trsting the user not to put extra .. and /'s in that could redirect the open command to operate on say ../../../etc/passwd for example.
    • You are making assumptions that the filename given will be file.ext and it will break when it gets something like file.ext.gz which is valid on some OSs
    • You could save yourself some parseing of the filename by using FILE::BASENAME
    UPDATE:Fixed 'not enough sleep this week' typo picked up by blakem
[reply]
      Taint mode is specified with the -T switch, not -t. (-t doesn't seem to be assigned to anything)

      -Blake

[reply]

Back to Seekers of Perl Wisdom