in reply to Re^5: RFC: Net::SNTP::Client v1
in thread RFC: Net::SNTP::Client v1

Hello Monk::Thomas,

Finally I understood your comments, I was not able to understand that you first need to check if the port number is actually is an Integer:

if (defined $value and $value =~ /\A (\d+) \z/xms)

[download]

Well a few things that I noticed is that you need to modify the second condition from:

if ($MIN_TCP_PORT <= $port and $port <= $MAX_TCP_PORT)

to:

if ($MIN_TCP_PORT <= $port and $port >= $MAX_TCP_PORT)

The port should not be greater than the MAX_TCP_PORT not minor. ;)

Question, why are you assigning the $value to a new value $port = $1. Is there a specific reason?

Last modification that I applied on my updated version, is the port. You are right just by modifying the hash would not be clear to someone else on the future.

So I added a simple or condition when I am creating the socket. Sample of modification:

PeerPort => $moduleInput{-port} || DEFAULT_NTP_PORT, # Default NTP por
+t 123

[download]

Again thank you for your time and effort reviewing my code.

Seeking for Perl wisdom...on the process of learning...not there...yet!

Replies are listed 'Best First'.
Re^7: RFC: Net::SNTP::Client v1
by Monk::Thomas (Friar) on Jul 03, 2015 at 08:09 UTC

    Glad to be of help. =)

    Well a few things that I noticed is that you need to modify the second condition...

    No you don't. Lets have a look at the code again:

    # in case of success: return a 'true' value   (1 or a useful value)
# in case of error:   return a 'false' value  (0 or undef)
sub verify_port {
  my $value = shift;

  # user provided input should be viewed as toxic - do not
  # trust until proven to contain a sane value
  if (defined $value and $value =~ /\A (\d+) \z/xms) {
    my $port = $1;

    if ($MIN_TCP_PORT <= $port and $port <= $MAX_TCP_PORT) {
      return 1;  # or TRUE or $port
    }
  }

  return 0;  # or FALSE or undef
}

    [download]

    As stated in the function's comment, the function is supposed to return a 'true' value if everything is alright and a 'false' value in case it isn't. (You did it the other way round - return FALSE if the port is fine and TRUE if it isn't.) So what I am doing is to make sure the provided port is defined, an integer and _inside_ the allowed port range. Only then '1' (or TRUE or $port) is returned.

    In all other cases (e.g. value is undefined or outside the allowed port range) then the function will return '0' (or FALSE or undef)

    Question, why are you assigning the $value to a new value $port = $1. Is there a specific reason?

    It is just an additional safeguard to make sure I perform the second check against a validated value. Additionally if you choose to return $port instead of 1 or TRUE then you can use the function to untaint your external / user-provided input.

    http://perldoc.perl.org/perlsec.html#Taint-mode
    http://www.webreference.com/programming/perl/taint/index.html
[reply]
[d/l]

In Section Meditations