http://qs321.pair.com?node_id=778617

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

i found this "search engine" while searching for some examples to help me learn CGI....
i think (well i know) its a kind of buggy.....
please help me improve it.......
#!perl -w use strict; use File::Find; use CGI qw(:standard); my $query = param("query"); print header(); print start_html(); print "\n<p>For the query $query, these results were found:</p>\n<ol>\ +n"; undef $/; find( sub { return if($_ =~ /^\./); return unless($_ =~ /\.html/i); stat $File::Find::name; return if -d; return unless -r; open(FILE, "< $File::Find::name") or return; my $string = <FILE>; close (FILE); return unless ($string =~ /\Q$query\E/i); my $page_title = $_; if ($string =~ /<title>(.*?)<\/title>/is) { $page_title = $1; } print "<li><a href=\"$File::Find::name\">$page_title</a></li>\n"; }, '/home/username/public_html'); print "</ol>\n"; print end_html();
------------------------------------------------------
i'am worst at what do best and for this gift i fell blessed...
i found it hard it's hard to find well whatever
NEVERMIND

Replies are listed 'Best First'.
Re: a "search engine" (ahem)
by apl (Monsignor) on Jul 09, 2009 at 19:50 UTC
    please help me improve it.......
    • use warnings;
    • Assign $_ to, say, $path
    • Don't have find( sub return without displaying something. For example:
      • '.' specified!
      • File '$file' is not HTML
      • File '$file' is a directory!
      • File '$file' does not exist!
      • File '$file' could not be opened!
      thanks apl but i cannot clearly understand your third point.... and since this is not my code so i actually cannot understand what $file is used for....

      =====================================================
      i'am worst at what do best and for this gift i fell blessed...
      i found it hard it's hard to find well whatever
      NEVERMIND

        Assune the find sub is called with $_ equal to SomeFile.html, but that file does not exist. The routine as written simply returns, and the user is left to wonder why nothing was displayed. I'm suggesting that a message be displayed explaining why nothing was displayed.

        The reason I suggested the name of the file be passed in a global variable (e.g. $path) is because $_ is used internally by Perl for a number of reasons. To prevent the path from being clobbered, it should be explicitly stored.

Re: a "search engine" (ahem)
by moritz (Cardinal) on Jul 09, 2009 at 20:01 UTC
    Use taint modus, see perlsec.

    use the 3-arg form of open (see also perlopentut.

    Don't interpolate $query into the HTML without HTML-escaping it first; that's a cross-site scripting vulnerability.

      thnx moritz but how do i HTML-escape $query and yeah the 3-arg form is more readable than this one

      =====================================================
      i'am worst at what do best and for this gift i fell blessed...
      i found it hard it's hard to find well whatever
      NEVERMIND

        Use escapeHTML from CGI. Or a template system that has the option to escape interpolated variables.