From 1bf2fe91359571b4b70019eddc5fd81c847b1c49 Mon Sep 17 00:00:00 2001 From: Matt Zagrabelny Date: Mon, 1 Feb 2016 13:47:22 -0600 Subject: [PATCH] pass a more descriptive error message for PhysAddr validation When a routine calls PhysAddr->validate to validate an L2 address the descriptive message is lost by just returning a boolean success or failure. Instead allow passing in a reference to a scalar to give the calling function an opportunity to give the user more information about the specifics behind Netdot rejecting the MAC address they entered. Small amounts of whitespace formatting were performed in this commit as well as the functional changes. Changes such as removing trailing whitespace in functions that were touched and also replacing tabs with four spaces. --- lib/Netdot/Model/DhcpScope.pm | 11 ++- lib/Netdot/Model/PhysAddr.pm | 126 ++++++++++++++++++++++------------ 2 files changed, 89 insertions(+), 48 deletions(-) diff --git a/lib/Netdot/Model/DhcpScope.pm b/lib/Netdot/Model/DhcpScope.pm index 066ec2935..cc6b896fc 100644 --- a/lib/Netdot/Model/DhcpScope.pm +++ b/lib/Netdot/Model/DhcpScope.pm @@ -462,8 +462,9 @@ sub _objectify_args { if ( $argv->{physaddr} && !ref($argv->{physaddr}) ){ # Could be an ID or an actual address - my $phys; - if ( PhysAddr->validate($argv->{physaddr}) ){ + my $phys; + my $validation_error_message = undef; + if ( PhysAddr->validate($argv->{physaddr}, \$validation_error_message) ){ # It looks like an address $phys = PhysAddr->find_or_create({address=>$argv->{physaddr}}); }elsif ( $argv->{physaddr} !~ /\D/ ){ @@ -473,7 +474,11 @@ sub _objectify_args { if ( $phys ){ $argv->{physaddr} = $phys; }else{ - $self->throw_user("Could not find or create physaddr"); + my $error_message = "Could not find or create physaddr"; + if (defined($validation_error_message)) { + $error_message .= "\n$validation_error_message"; + } + $self->throw_user($error_message); } } diff --git a/lib/Netdot/Model/PhysAddr.pm b/lib/Netdot/Model/PhysAddr.pm index 4677141d6..48bf37d4a 100644 --- a/lib/Netdot/Model/PhysAddr.pm +++ b/lib/Netdot/Model/PhysAddr.pm @@ -226,6 +226,7 @@ sub fast_update { Arguments: Physical address string + Return message buffer (optional) Returns: Physical address string in canonical format, false if invalid Examples: @@ -234,51 +235,81 @@ sub fast_update { =cut sub validate { - my ($self, $addr) = @_; + my ($self, $addr, $return_message_buffer) = @_; + $self->isa_class_method('validate'); - return unless $addr; - $addr = $self->format_address($addr); - if ( $addr !~ /^[0-9A-F]{12}$/ ){ - # Format must be DEADDEADBEEF - $logger->debug(sub{ "PhysAddr::validate: Bad format: $addr" }); - return 0; - - }elsif ( $addr =~ /^([0-9A-F]{1})/ && $addr =~ /$1{12}/ ) { - # Assume the all-equal-bits address is invalid - $logger->debug(sub{ "PhysAddr::validate: Bogus address: $addr" }); - return 0; - - }elsif ( $addr eq '000000000001' ) { - # Skip Passport 8600 CLIP MAC addresses - $logger->debug(sub{ "PhysAddr::validate: CLIP: $addr" }); - return 0; - - }elsif ( $addr =~ /^00005E00/i ) { - # Skip VRRP addresses - $logger->debug(sub{ "PhysAddr::validate: VRRP: $addr" }); - return 0; - - }elsif ( $addr =~ /^00000C07AC/i ) { - # Skip VRRP addresses - $logger->debug(sub{ "PhysAddr::validate: HSRP: $addr" }); - return 0; - - }elsif ( $addr =~ /^([0-9A-F]{2})/ && $1 =~ /.(1|3|5|7|9|B|D|F)/ ) { - # Multicast addresses - $logger->debug(sub{ "PhysAddr::validate: address is Multicast: $addr" }); - return 0; - - }elsif ( scalar(my @list = @{$self->config->get('IGNORE_MAC_PATTERNS')} ) ){ - foreach my $ignored ( @list ){ - if ( $addr =~ /$ignored/i ){ - $logger->debug(sub{"PhysAddr::validate: address matches configured pattern ($ignored): $addr"}); - return 0; - } - } + my $retval = 1; + my $error_message = undef; + + if (! $addr) { + $error_message = "PhysAddr::validate: Address is not valid."; + $retval = 0; + } + else { + $addr = $self->format_address($addr); + + if ( $addr !~ /^[0-9A-F]{12}$/ ) { + # Format must be DEADDEADBEEF + $error_message = "PhysAddr::validate: Bad format: $addr"; + $retval = 0; + } + elsif ( $addr =~ /^([0-9A-F]{1})/ && $addr =~ /$1{12}/ ) { + # Assume the all-equal-bits address is invalid + $error_message = "PhysAddr::validate: Bogus address: $addr"; + $retval = 0; + } + elsif ( $addr eq '000000000001' ) { + # Skip Passport 8600 CLIP MAC addresses + $error_message = "PhysAddr::validate: CLIP: $addr"; + $retval = 0; + } + elsif ( $addr =~ /^00005E00/i ) { + # Skip VRRP addresses + $error_message = "PhysAddr::validate: VRRP: $addr"; + $retval = 0; + } + elsif ( $addr =~ /^00000C07AC/i ) { + # Skip HSRP addresses + $error_message = "PhysAddr::validate: HSRP: $addr"; + $retval = 0; + } + elsif ( + $addr =~ /^([0-9A-F]{2})/ + && + $1 =~ /.(1|3|5|7|9|B|D|F)/ + ) { + # Multicast addresses + $error_message = "PhysAddr::validate: address is Multicast: $addr"; + $retval = 0; + } + elsif ( scalar(my @list = @{$self->config->get('IGNORE_MAC_PATTERNS')} ) ) { + foreach my $ignored ( @list ) { + if ( $addr =~ /$ignored/i ) { + $error_message = "PhysAddr::validate: address matches configured pattern ($ignored): $addr"; + $retval = 0; + last; + } + } + } + } + + if ($retval == 0) { + # If the return message buffer is a reference to a scalar, then set + # its contents to the error message. + if ( + defined($return_message_buffer) + && + ref($return_message_buffer) eq ref(\"") + ) { + $$return_message_buffer = $error_message; + } + $logger->debug(sub{ $error_message }); + return $retval; + } + else { + return $addr; } - - return $addr; } ################################################################# @@ -811,10 +842,15 @@ sub _canonicalize { my $class = ref($self) || $self; my $address = ($self->_attrs('address'))[0]; $self->throw_user("Missing address") - unless $address; + unless $address; $address = $self->format_address($address); - unless ( $class->validate( $address ) ){ - $self->throw_user("Invalid Address: $address"); + my $validation_error_message = undef; + if (! $class->validate($address, \$validation_error_message)) { + my $error_message = "Invalid Address: $address"; + if (defined($validation_error_message)) { + $error_message .= "\n$validation_error_message"; + } + $self->throw_user($error_message); } $self->_attribute_store( address => $address ); }