You are reading in lines from a file named ship.cfg and you are not applying any particular attention to what is being read in. In perlspeak, the lines are "tainted". They could contain anything, usually benign, but possibly malicious.

Ordinarily, tainted data doesn't matter at all, as long as you content yourself to operations that don't have an effect outside the perl interpreter. But as soon as you reach out and start interacting with the system, whether it be disk, network or otherwise, tainting is of utmost importance.

The problem is that you are asking perl to use that unclean information read from ship.cfg to connect to a socket. And perl has a problem with that. It can't know whether what you are doing is benign, or whether you're trying to tie up a database listener or something crucial to your infrastructure. So until you untaint your data, it's not going to let you. Read up on perl security for information on untainting.

This is what I get when running it with #!/usr/bin/perl -T

Insecure dependency in connect while running with -T switch at /usr/libdata/perl5/Net/Ping.pm line 914.

This is most likely because you did not clean up $ENV{PATH} to be secure. Most likely, if your Unix installation is a standard installation, the following should work at the top of your script:

$ENV{PATH} = '/bin:/usr/bin:/usr/local/bin';
[download]

If that doesn't work, you need to find out in which directory the ping executable lives, and put the name of that directory into $ENV{PATH} instead.

doesn't work

Here are two links that could be of help to you to present your questions more effectively:

• How (Not) To Ask A Question
• a comedy about a wrong question's side effects

HTH

using -T doesn't work

Before proceeding, however, I can't help pointing out that your description of the problem is particularly poor: generally "doesn't work" doesn't give any clue on what may actually be wrong. So the first suspect one can legitimately have is that your problem does not really have to do with -T. In particular I've noticed that you misplaced the header, so that may bring you to a "doesn't work" situation. But due to the precise nature of your question and willing to trust you, I assume that you have tried both with -T and without. Well, in any case this is not a good reason not to use it. Adopt the standard method to untaint any tainted variable you want to, instead, if really needed!

Let's see the code, anyway...

use strict;
use warnings;
[download]

use Net::Ping;
use CGI qw/:standard/;
[download]

my @hosts;
if ( -e "ship.cfg" ) {
        open (CFGFILE, "ship.cfg") || die;
        @hosts=<CFGFILE>;
        close (CFGFILE);
} else {
        # Enter your hosts here...
        @hosts = qw/host1 host2 host3/;
}
[download]

As a side note, you chomp() values below, and it obviously won't affect the latter entries that do not have an ending "\n", so it is indeed consistent. But IMHO it is slightly stylistically inconsistent: if it were me I would prefer to have all the values chomp()ed at this point.

UPDATE: this is really a minor point, but you rather than having two literal "ship.cfg" it may be better to assign to a variable to reduce the risk of typos. You may even want to

for ('ship.cfg') {
    # ...
}
[download]

UPDATE2: it didn't occur to me first because I naively and mistakenly thought that since you hardcoded the config file in the source, anything coming from it would not have been tainted. But that's still data coming in from the outside so indeed it is tainted! As I wrote above, just untaint it!

my @uphosts=();
my @downhosts=();
[download]

my $proto;
my @verbage=("are", "hosts", "are", "hosts");
[download]

You may want to know that you can declare more than one variable at a time, e.g.: my (@uphosts, @downhosts).

UPDATE: this is another minor point, but since you used qw// above I wonder why you didn't use it here too, since it makes for a much more agile syntax.

my $p = Net::Ping->new('udp');

foreach my $host(@hosts) {
        chomp($host);
        if ($p->ping($host)) {
                push (@uphosts, $host);
        } else {
                push (@downhosts, $host);
        }
}
[download]

$p->close();
print start_html,br;
print header,br;
print start_form,br;
print submit('Refresh'),p;
[download]

if (scalar(@downhosts) == 1 ) {
        $verbage[2]="is";
        $verbage[3]="host"
}
if (scalar(@uphosts) == 1 ) {
        $verbage[0]="is";
        $verbage[1]="host";
}
[download]

You can use an array slice.

unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
[download]

(@downhosts),"  $verbage[3] down",br }
print "There $verbage[0] ",scalar(@uphosts)," $verbage[1] alive",br;

print p,"The following $verbage[1] $verbage[0] alive: \n",br;
foreach my $uitem (sort @uphosts) {
        print li("$uitem"),br;
}
[download]

UPDATE: please note that there's non need to quote all variables, see 'perldoc -q quoting'.

unless (scalar(@downhosts) == 0) { print p,"The following $verbage[3] 
$verbage[2] down: ",br }
foreach my $ditem (sort @downhosts) {
        print li("$ditem"),br;
}
[download]

Ditto as above wrt the topicalizer.

All in all I'd rewrite the code as follows:

The full code, revisited

#!/usr/bin/perl

use strict;
use warnings;
use Net::Ping;
use CGI qw/:standard/;

my @hosts= -e 'ship.cfg' ? do {
    open my $cfgfile, '<', 'ship.cfg' 
      or die $!;
    map {chomp; $_ } <$cfgfile>;
} : qw/host1 host2 host3/;  # defaults

my (@up, @down);
my $p=Net::Ping->new('udp');
push @{ $p->ping($_) ? \@up : \@down }, $_ for @hosts;
$p->close();

print header, start_html, 
  start_form, submit 'Refresh';
    
my @verbage=qw/are hosts are hosts/;
@verbage[0,1]=qw/is host/ if @up == 1;
@verbage[2,3]=qw/is host/ if @down == 1;
    
print p, "There $verbage[2] ",
  (scalar @down),
  " $verbage[3] down" unless @down;

print p, "There $verbage[0] ",
  (scalar @up),
  " $verbage[1] alive" unless @up;

if (@up) {
    print p, "The following $verbage[1] $verbage[0] alive:\n", br;
    print li($_) for sort @up;
}

if (@down) {
    print p, "The following $verbage[3] $verbage[2] down:\n", br;
    print li($_) for sort @down;
}

__END__
[download]

Thanks for the help, Im slowly revising the code - I want to understand the code Im replacing before I replace it, so - so far this is where im at with the code.

use strict;
use warnings;
use Net::Ping;
use CGI qw/:standard/;
use POSIX qw(strftime);

# Clean up our UNIX environment
# for more infor read perldoc perlsec
$ENV{'PATH'} = '/bin:/usr/bin';
delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};

my (@hosts, @uphosts, @downhosts);

if ( -e "ship.cfg" ) {
        open (CFGFILE, "ship.cfg") || die $!;
        while (my @TMP=<CFGFILE>) {
                #if ( $_ =~ /^#/ ) { next }
                # Lets untaint the data after making sure it only matc
+hes
                # word characters... For more info read perldoc perlse
+c.
                foreach $_(@TMP) {
                        if ($_ =~ /^([\w.]+)$/) {
                                $_ = $1;
                                push (@hosts, $1);
                        } else {
                                next;
                        }
                }
        }
        close (CFGFILE);
} else {
        # or enter your hosts here...
        # This is the most secure/preferred way to do this
        @hosts = qw/sapdev0 sapprd0 saptst0 pcbackup vader/;
}

# I haven't decided what else to do here - it just semantics...
my @verbage=("are", "hosts", "are", "hosts");

# udp is default
# other values can be icmp (if youre root)
# or TCP, which is expensive on the wire
my $p = Net::Ping->new('udp');

# Iterate through my hosts and get their status
push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts;
$p->close();

print header, start_html;

print "<tt>\n";
print "\n<center><h3>MSHIP - my status of host IP's</h3></center>\n";
my $now=strftime "%m/%d/%Y %H:%M:%S", (localtime);
print "<center>$now</center>\n";
print hr;

# Just cleaning up the verbage is are host hosts etc...
if (scalar(@downhosts) == 1 ) {
        $verbage[2]="is";
        $verbage[3]="host"
if (scalar(@uphosts) == 1 ) {
        $verbage[0]="is";
        $verbage[1]="host";
}

# We don't care about things that don't exist...
unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
+(@downhosts),"  $verbage[3] unreachable\n",br }
print "There $verbage[0] ",scalar(@uphosts)," $verbage[1] alive\n";

print p,"The following $verbage[1] $verbage[0] alive: \n",br;
foreach my $uitem (sort @uphosts) { # sort is for cleanliness...
        print li("$uitem\n"),br;
}
# Again, we don't care about things that don't exist
unless (scalar(@downhosts) == 0) { print p,"The following $verbage[3] 
+$verbage[2] unreachable: \n",br }
foreach my $ditem (sort @downhosts) {
        print li("$ditem\n"),br;
}
print "</tt>\n";

print start_form,
        p,submit('Refresh'),
        end_html;
[download]

Where Im at now is on this piece of code:

# Iterate through my hosts and get their status
push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts;
$p->close();
[download]


what I get from this is if ping then either @uphosts (if successful) or @downhosts gets $_ from @host. Im just not getting why is it in the form of
@{ $p->ping($_) ? \@uphosts : \@downhosts }
and what does referencing @downhosts and @uphosts do that just calling them won't do?
Or where can I read more on this other than perldata, perlref, etc...
Thanks
Ted

Thanks for the help, Im slowly revising the code - I want to understand the code Im replacing before I replace it, so - so far this is where im at with the code.

Ok, Please note that to distinguish my code from yours, mine will be commented out (when in a paragraph by itself).

Note: the code I proposed the last time was rewritten from yours according to my own personal preferences. I tried to keep its general structure as close as possible to yours, though.

my (@hosts, @uphosts, @downhosts);

if ( -e "ship.cfg" ) {
        open (CFGFILE, "ship.cfg") || die $!;
[download]

If you use a lexical filehandle then you do not need to close() it (when going out of scope (and if there are no more references to it)).

Personally I like to avoid all parentheses altogether and use the low priority C<and> for flow control. I like to use the three-args from of open() even if it is not strictly necessary for I think it's a good habit.

        while (my @TMP=<CFGFILE>) {
[download]

Perl's idiom for reading "lines" (whatever these can be according to $/) is

        # while (<CFGFILE>) {
[download]

If you want to do *exactly* the same with an explicit variable rather than $_, then you have to do

        # while (defined(my $line = <ARGV>)) {
        #     ...
        # }
[download]

What you wrote won't do what you think, since notwithstanding the C<while>, all of the file's content will be read into @TMP at a time.

                #if ( $_ =~ /^#/ ) { next }  # commented out in the OP
[download]

And $_ is?

Also, while legal, you're missing the whole point about $_: it's the implicit variable on which so many operators and functions act. So you either want to $variable =~ /something/ or /something/ (if your variable is $_).

Also, you may not like statement modifiers, but if there's a case in which one would come really handy is exactly this one: next if /^#/; (IMHO much more readable/immediate).

                foreach $_(@TMP) {
[download]

Don't! Quite similarly you either do something for my $var (@list) { ... or for (@list) { ... (if you want to use $_).

                        if ($_ =~ /^([\w.]+)$/) {
[download]

Don't!

                                $_ = $1;
[download]

Why?!?

                                push (@hosts, $1);
                        } else {
                                next;
                        }
[download]

No need for the C<next> block, as if the condition is not fulfilled it will pass to the next iteration anyway.

        close (CFGFILE);
[download]

If you use a lexical filehandles then you do not need to close() them.

# Just cleaning up the verbage is are host hosts etc...
if (scalar(@downhosts) == 1 ) {
        $verbage[2]="is";
        $verbage[3]="host"
if (scalar(@uphosts) == 1 ) {
        $verbage[0]="is";
        $verbage[1]="host";
}
[download]

As I wrote the other time, no need for scalar() there, and you can use an array slice. Not to say that this is not correct, strictly speaking, only IMHO it is much cleaner the other way round. For an actual example see the sample code I posted in the other node.

# We don't care about things that don't exist...
unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
[download]

No need for the first scalar() as well as no need for the C<==0>. (No: this is not a mouse!! ;-)

The same cmts apply to much of the following code, I'm not repeating them.

Also, this is yet another one of those things that depend largely on personal preferences, but well written code should be self explanatory and only really relevant comments should be there. Commenting the obvious doesn't add to readability, IMHO: it has just the opposite effect instead!

Where Im at now is on this piece of code: # Iterate through my hosts and get their status push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts; $p->close();

what I get from this is if ping then either @uphosts (if successful) or @downhosts gets $_ from @host. Im just not getting why is it in the form of @{ $p->ping($_) ? \@uphosts : \@downhosts }

Well, it's just a way to do it that I find to be convenient. Of course it's up to your personal preferences. You may consider it to be slightly ineffcient, but we're dealing with only a bunch of them anyway and $p->ping($_) takes most of the time here.

and what does referencing @downhosts and @uphosts do that just calling them won't do? Or where can I read more on this other than perldata, perlref, etc...

Hint: push() wants an array as its first argument, I'm giving it one. I think perlref has all that is needed to be known. Check perlreftut, as well.