[Rt-commit] rt branch, 4.0/ip-address-canonicalization, created. rt-4.0.20-33-g4cd0b9c
Alex Vandiver
alexmv at bestpractical.com
Thu Jun 5 16:25:05 EDT 2014
The branch, 4.0/ip-address-canonicalization has been created
at 4cd0b9c7c258b985159d96f6aa75d871f0300777 (commit)
- Log -----------------------------------------------------------------
commit da5169809e13d63faf1bca3cac6806dcb258593b
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:09:58 2014 -0400
->Content is as formatted for the user, not the DB value
When comparing the canonicalized values, one must bear in mind that
->Content is not just the Content column in the database; instead, it
falls back to the LongContent, in addition to doing explicit formatting
for IP range custom fields.
Compare the 'Content' key to what is actually in the 'Content' column,
using $self->_Value(). This does not fix IP custom field updates, as
the input Content has still not been split into Content and LargeContent
as the database stores it.
diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index 2ab8b08..b1e9964 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -150,7 +150,7 @@ sub HasEntry {
return $item if lc $item->Content eq lc $args->{Content};
}
else {
- if ( $item->Content eq $args->{Content} ) {
+ if ( $item->_Value('Content') eq $args->{Content} ) {
if ( defined $item->LargeContent ) {
return $item
if defined $args->{LargeContent}
commit eb939ac13712d04544c6aa892563e95aab0ee3a4
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:11:49 2014 -0400
Add a generic ->_CanonicalizeValue
Refactor the two places that use can() into a common method.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index fb92714..371a35b 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1530,11 +1530,7 @@ sub AddValueForObject {
}
}
- if (my $canonicalizer = $self->can('_CanonicalizeValue'.$self->Type)) {
- $canonicalizer->($self, \%args);
- }
-
-
+ $self->_CanonicalizeValue(\%args);
my $newval = RT::ObjectCustomFieldValue->new( $self->CurrentUser );
my ($val, $msg) = $newval->Create(
@@ -1557,6 +1553,16 @@ sub AddValueForObject {
}
+sub _CanonicalizeValue {
+ my $self = shift;
+ my $args = shift;
+
+ return unless $self->Type;
+
+ my $method = '_CanonicalizeValue'.$self->Type;
+ return unless $self->can($method);
+ $self->$method($args);
+}
sub _CanonicalizeValueDateTime {
my $self = shift;
diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index b1e9964..4403d03 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -136,12 +136,7 @@ sub HasEntry {
my $args = $canon_value{ $cf->Type };
if ( !$args ) {
$args = { Content => $value, LargeContent => $large_content };
- if ( my $canonicalizer =
- $cf->can( '_CanonicalizeValue' . $cf->Type ) )
- {
- $canonicalizer->( $cf, $args );
- }
-
+ $cf->_CanonicalizeValue( $args );
$canon_value{ $cf->Type } = $args;
}
commit 9ec3b7df17d5caa4d997161e98713f373d6c6719
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Thu Jun 5 16:18:16 2014 -0400
Move canonicalization into RT::ObjectCustomFieldValue->Create
Previously, value canonicalization happened for dates and datetimes in
RT::CustomField->AddValueForObject, and for IP addresses and ranges in
RT::ObjectCustomFieldValue->Create. Standardize on the latter, as it is
lower down and thus harder to skip accidentally.
This requires creating the CustomField object as the current user, not
as the system user, as Date and DateTime canonicalization cares about
the time zone of the current user. This, in turn, requires switching to
_Value('Type') instead of Type, as the constructed $cf object is not
guaranteed to have the right context objects for it to be readable by
the user.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 371a35b..e42f643 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1530,8 +1530,6 @@ sub AddValueForObject {
}
}
- $self->_CanonicalizeValue(\%args);
-
my $newval = RT::ObjectCustomFieldValue->new( $self->CurrentUser );
my ($val, $msg) = $newval->Create(
ObjectType => ref($obj),
@@ -1557,9 +1555,10 @@ sub _CanonicalizeValue {
my $self = shift;
my $args = shift;
- return unless $self->Type;
+ my $type = $self->_Value('Type');
+ return unless $type;
- my $method = '_CanonicalizeValue'.$self->Type;
+ my $method = '_CanonicalizeValue'. $type;
return unless $self->can($method);
$self->$method($args);
}
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index de4bc74..1c79ce9 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -84,11 +84,12 @@ sub Create {
@_,
);
+ my $cf = RT::CustomField->new( $self->CurrentUser );
+ $cf->Load( $args{CustomField} );
- my $cf_as_sys = RT::CustomField->new(RT->SystemUser);
- $cf_as_sys->Load($args{'CustomField'});
+ $cf->_CanonicalizeValue(\%args);
- if($cf_as_sys->Type eq 'IPAddress') {
+ if($cf->_Value('Type') eq 'IPAddress') {
if ( $args{'Content'} ) {
$args{'Content'} = $self->ParseIP( $args{'Content'} );
}
@@ -101,7 +102,7 @@ sub Create {
}
}
- if($cf_as_sys->Type eq 'IPAddressRange') {
+ if($cf->_Value('Type') eq 'IPAddressRange') {
if ($args{'Content'}) {
($args{'Content'}, $args{'LargeContent'}) = $self->ParseIPRange( $args{'Content'} );
}
commit 45b632455d002a77e4592a09535d8ffa9fd3dcfd
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:13:35 2014 -0400
Add common IP address and range canonicalizers
The IP address and range canonicalization in ObjectCustomFieldValue's
Create is moved into the more general canonicalization functions. This
also allows HasEntry can split IP ranges correctly to check their
existance, and ensures that the standard form (padded to three digits
per octet) is compared against the database.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index e42f643..0441484 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1586,6 +1586,26 @@ sub _CanonicalizeValueDate {
$args->{'Content'} = $DateObj->Date( Timezone => 'user' );
}
+sub _CanonicalizeValueIPAddress {
+ my $self = shift;
+ my $args = shift;
+
+ $args->{Content} = RT::ObjectCustomFieldValue->ParseIP( $args->{Content} );
+}
+
+sub _CanonicalizeValueIPAddressRange {
+ my $self = shift;
+ my $args = shift;
+
+ my $content = $args->{Content};
+ $content .= "-".$args->{LargeContent} if $args->{LargeContent};
+
+ ($args->{Content}, $args->{LargeContent})
+ = RT::ObjectCustomFieldValue->ParseIPRange( $content );
+
+ $args->{ContentType} = 'text/plain';
+}
+
=head2 MatchPattern STRING
Tests the incoming string against the Pattern of this custom field object
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 1c79ce9..efb34d1 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -89,33 +89,6 @@ sub Create {
$cf->_CanonicalizeValue(\%args);
- if($cf->_Value('Type') eq 'IPAddress') {
- if ( $args{'Content'} ) {
- $args{'Content'} = $self->ParseIP( $args{'Content'} );
- }
-
- unless ( defined $args{'Content'} ) {
- return
- wantarray
- ? ( 0, $self->loc("Content is an invalid IP address") )
- : 0;
- }
- }
-
- if($cf->_Value('Type') eq 'IPAddressRange') {
- if ($args{'Content'}) {
- ($args{'Content'}, $args{'LargeContent'}) = $self->ParseIPRange( $args{'Content'} );
- }
- $args{'ContentType'} = 'text/plain';
-
- unless ( defined $args{'Content'} ) {
- return
- wantarray
- ? ( 0, $self->loc("Content is an invalid IP address range") )
- : 0;
- }
- }
-
if ( defined $args{'Content'} && length( Encode::encode_utf8($args{'Content'}) ) > 255 ) {
if ( defined $args{'LargeContent'} && length $args{'LargeContent'} ) {
$RT::Logger->error("Content is longer than 255 bytes and LargeContent specified");
commit 638214540c268f2d3a9996b5a2fc21d62d63853a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:21:50 2014 -0400
Let canonicalization return a failure message
This allows CF creation to abort on invalid values, and HasEntry to give
up and move on if passed a value that can never be found in the
database.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 0441484..eeb7295 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1556,10 +1556,10 @@ sub _CanonicalizeValue {
my $args = shift;
my $type = $self->_Value('Type');
- return unless $type;
+ return 1 unless $type;
my $method = '_CanonicalizeValue'. $type;
- return unless $self->can($method);
+ return 1 unless $self->can($method);
$self->$method($args);
}
@@ -1570,6 +1570,7 @@ sub _CanonicalizeValueDateTime {
$DateObj->Set( Format => 'unknown',
Value => $args->{'Content'} );
$args->{'Content'} = $DateObj->ISO;
+ return 1;
}
# For date, we need to store Content as ISO date
@@ -1584,6 +1585,7 @@ sub _CanonicalizeValueDate {
Value => $args->{'Content'},
);
$args->{'Content'} = $DateObj->Date( Timezone => 'user' );
+ return 1;
}
sub _CanonicalizeValueIPAddress {
@@ -1591,6 +1593,9 @@ sub _CanonicalizeValueIPAddress {
my $args = shift;
$args->{Content} = RT::ObjectCustomFieldValue->ParseIP( $args->{Content} );
+ return (0, $self->loc("Content is not a valid IP address"))
+ unless $args->{Content};
+ return 1;
}
sub _CanonicalizeValueIPAddressRange {
@@ -1604,6 +1609,9 @@ sub _CanonicalizeValueIPAddressRange {
= RT::ObjectCustomFieldValue->ParseIPRange( $content );
$args->{ContentType} = 'text/plain';
+ return (0, $self->loc("Content is not a valid IP address range"))
+ unless $args->{Content};
+ return 1;
}
=head2 MatchPattern STRING
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index efb34d1..274f889 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -87,7 +87,8 @@ sub Create {
my $cf = RT::CustomField->new( $self->CurrentUser );
$cf->Load( $args{CustomField} );
- $cf->_CanonicalizeValue(\%args);
+ my ($val, $msg) = $cf->_CanonicalizeValue(\%args);
+ return ($val, $msg) unless $val;
if ( defined $args{'Content'} && length( Encode::encode_utf8($args{'Content'}) ) > 255 ) {
if ( defined $args{'LargeContent'} && length $args{'LargeContent'} ) {
diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index 4403d03..a2ec317 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -136,7 +136,8 @@ sub HasEntry {
my $args = $canon_value{ $cf->Type };
if ( !$args ) {
$args = { Content => $value, LargeContent => $large_content };
- $cf->_CanonicalizeValue( $args );
+ my ($ok, $msg) = $cf->_CanonicalizeValue( $args );
+ next unless $ok;
$canon_value{ $cf->Type } = $args;
}
commit e688a797cbf842de3c1f6c481313e00e7af788a0
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:22:56 2014 -0400
Provide a better error message for loading OCFVs on an invalid CF
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 274f889..2b61a9a 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -139,6 +139,8 @@ sub LoadByCols {
if ( $args{CustomField} ) {
$cf = RT::CustomField->new( $self->CurrentUser );
$cf->Load( $args{CustomField} );
+ return (0, $self->loc("Cannot load custom field [_1]",$args{CustomField})) unless $cf->id;
+
if ( $cf->Type && $cf->Type eq 'IPAddressRange' ) {
my ( $sIP, $eIP ) = $cf->ParseIPRange( $args{'Content'} );
commit b32a81812a9ef80238ceeaf248476d8f46d290f7
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:23:29 2014 -0400
Canonicalize all OCFVs on load, not just IP address ranges
This ensures that IPs are padded to three digits per octet, and that
dates are formatted in standard ISO time.
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 2b61a9a..7162f03 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -141,16 +141,8 @@ sub LoadByCols {
$cf->Load( $args{CustomField} );
return (0, $self->loc("Cannot load custom field [_1]",$args{CustomField})) unless $cf->id;
- if ( $cf->Type && $cf->Type eq 'IPAddressRange' ) {
-
- my ( $sIP, $eIP ) = $cf->ParseIPRange( $args{'Content'} );
- if ( $sIP && $eIP ) {
- $self->SUPER::LoadByCols( %args,
- Content => $sIP,
- LargeContent => $eIP
- );
- }
- }
+ my ($ok, $msg) = $cf->_CanonicalizeValue(\%args);
+ return ($ok, $msg) unless $ok;
}
return $self->SUPER::LoadByCols(%args);
}
commit e93f58570a691314ee9e230d8f1ed1ba63b56375
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Thu Jun 5 13:57:08 2014 -0400
Remove too-many-value OCFVs by id, not Content
This merely serves to bulletproof the removal step; there is no need for
the more expensive Load with Content.
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 45ec7cf..038865d 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1726,8 +1726,8 @@ sub _AddCustomFieldValue {
$i++;
if ( $i < $cf_values ) {
my ( $val, $msg ) = $cf->DeleteValueForObject(
- Object => $self,
- Content => $value->Content
+ Object => $self,
+ Id => $value->id,
);
unless ($val) {
return ( 0, $msg );
commit 61c41182285a4de9ed7aeaaee89d45413f34a4f7
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:23:58 2014 -0400
Reuse existing validation in OCVF canonicalization
Rather than reiterate the logic in the canonicalization functions, use
the error condition they return to determine the validation error.
This also opens the door for invalid date and datetime fields to abort
ticket updates, by providing a validation error if they do not parse.
For backwards compatibility, this does not enforce such.
diff --git a/share/html/Elements/ValidateCustomFields b/share/html/Elements/ValidateCustomFields
index 713885f..8b49828 100644
--- a/share/html/Elements/ValidateCustomFields
+++ b/share/html/Elements/ValidateCustomFields
@@ -83,27 +83,12 @@ while ( my $CF = $CustomFields->Next ) {
for my $value( @values ) {
if ($value) {
- if ( $CF->Type eq 'IPAddress' ) {
- use Regexp::Common qw(RE_net_IPv4);
- my $ip = RT::ObjectCustomFieldValue->ParseIP( $value );
- unless ( $ip ) {
- my $msg =
- loc( "Input can not be parsed as an IP address" );
- $m->notes( ( 'InvalidField-' . $CF->Id ) => $msg );
- push @res, $msg;
- $valid = 0;
- }
- }
- elsif ( $CF->Type eq 'IPAddressRange' ) {
- my ( $start_ip, $end_ip ) =
- RT::ObjectCustomFieldValue->ParseIPRange($value);
- unless ( $start_ip && $end_ip ) {
- my $msg =
- loc( "Input can not be parsed as an IP address range" );
- $m->notes( ( 'InvalidField-' . $CF->Id ) => $msg );
- push @res, $msg;
- $valid = 0;
- }
+ my $ref = { Content => $value };
+ my ($ok, $msg) = $CF->_CanonicalizeValue( $ref );
+ unless ($ok) {
+ $m->notes( ( 'InvalidField-' . $CF->Id ) => $msg );
+ push @res, $msg;
+ $valid = 0;
}
}
diff --git a/t/customfields/ip.t b/t/customfields/ip.t
index 3ab7fbd..37afcb7 100644
--- a/t/customfields/ip.t
+++ b/t/customfields/ip.t
@@ -147,7 +147,7 @@ diag "check that we parse correct IPs only" if $ENV{'TEST_VERBOSE'};
}
);
- $agent->content_contains( 'can not be parsed as an IP address',
+ $agent->content_contains( 'is not a valid IP address',
'ticket fails to create' );
}
diff --git a/t/customfields/iprange.t b/t/customfields/iprange.t
index af9a52f..4bccd9a 100644
--- a/t/customfields/iprange.t
+++ b/t/customfields/iprange.t
@@ -197,7 +197,7 @@ diag "check that we parse correct IPs only" if $ENV{'TEST_VERBOSE'};
}
);
- $agent->content_like( qr/can not be parsed as an IP address range/, 'ticket fails to create' );
+ $agent->content_like( qr/is not a valid IP address range/, 'ticket fails to create' );
}
}
diff --git a/t/customfields/iprangev6.t b/t/customfields/iprangev6.t
index 3b8a4d6..84fec16 100644
--- a/t/customfields/iprangev6.t
+++ b/t/customfields/iprangev6.t
@@ -193,7 +193,7 @@ diag "check that we parse correct IPs only" if $ENV{'TEST_VERBOSE'};
}
);
- $agent->content_like( qr/can not be parsed as an IP address range/,
+ $agent->content_like( qr/is not a valid IP address range/,
'ticket fails to create' );
}
diff --git a/t/customfields/ipv6.t b/t/customfields/ipv6.t
index f97420e..3b02ef9 100644
--- a/t/customfields/ipv6.t
+++ b/t/customfields/ipv6.t
@@ -150,7 +150,7 @@ diag "check that we parse correct IPs only" if $ENV{'TEST_VERBOSE'};
}
);
- $agent->content_contains( 'can not be parsed as an IP address',
+ $agent->content_contains( 'is not a valid IP address',
'ticket fails to create' );
}
}
commit 4cd0b9c7c258b985159d96f6aa75d871f0300777
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 4 16:24:27 2014 -0400
Remove a needless block; ParseIPRange does the exact same as its first step
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 6d536bb..3c4ce29 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -1544,15 +1544,6 @@ sub _CustomFieldLimit {
}
if ( $cf && $cf->Type eq 'IPAddressRange' ) {
-
- if ( $value =~ /^\s*$RE{net}{CIDR}{IPv4}{-keep}\s*$/o ) {
-
- # convert incomplete 192.168/24 to 192.168.0.0/24 format
- $value =
- join( '.', map $_ || 0, ( split /\./, $1 )[ 0 .. 3 ] ) . "/$2"
- || $value;
- }
-
my ( $start_ip, $end_ip ) =
RT::ObjectCustomFieldValue->ParseIPRange($value);
if ( $start_ip && $end_ip ) {
-----------------------------------------------------------------------
More information about the rt-commit
mailing list