[Rt-commit] rt branch, 4.2/ip-address-canonicalization, created. rt-4.2.5rc1-14-gcd94a08
Alex Vandiver
alexmv at bestpractical.com
Thu Jun 5 16:26:57 EDT 2014
The branch, 4.2/ip-address-canonicalization has been created
at cd94a08cbb807b16ccf985501085db3cf658a380 (commit)
- Log -----------------------------------------------------------------
commit 5e2dc55ac098de24d8a4f3f7aa34208cecb8a38b
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 11c797f..e02c4ad 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -149,7 +149,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 66cf1d4262d7f33381c4d73e74f755db2c48dc51
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 5e2b90b..17271b3 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1675,11 +1675,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(
@@ -1702,6 +1698,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 e02c4ad..bec3e81 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -135,12 +135,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 d3928d7b7a052b31fb60ec58e2ce0250d73bc2a4
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 17271b3..53a83ac 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1675,8 +1675,6 @@ sub AddValueForObject {
}
}
- $self->_CanonicalizeValue(\%args);
-
my $newval = RT::ObjectCustomFieldValue->new( $self->CurrentUser );
my ($val, $msg) = $newval->Create(
ObjectType => ref($obj),
@@ -1702,9 +1700,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 07f452d..b788835 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -82,11 +82,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'} );
}
@@ -99,7 +100,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 c1727f60145e3f5ceaa737779205d9a8c9358b46
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 53a83ac..5180e4c 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1731,6 +1731,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 b788835..4ee1ff1 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -87,33 +87,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 b7a8c03f6ba1485489d3df10347e9c3cc1dff1a2
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 5180e4c..efa7c81 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1701,10 +1701,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);
}
@@ -1715,6 +1715,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
@@ -1729,6 +1730,7 @@ sub _CanonicalizeValueDate {
Value => $args->{'Content'},
);
$args->{'Content'} = $DateObj->Date( Timezone => 'user' );
+ return 1;
}
sub _CanonicalizeValueIPAddress {
@@ -1736,6 +1738,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 {
@@ -1749,6 +1754,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 4ee1ff1..19b17a1 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -85,7 +85,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 bec3e81..ec3155d 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -135,7 +135,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 404787322dffbf7faa13f43a3ad9ff0691ea6b55
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 19b17a1..83c8739 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -140,6 +140,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 8ed92aed53e673f317e844453abae6aca490fcee
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 83c8739..110928c 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -142,16 +142,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 cd370c4ac0a50c47a81d5c9b06895c5d406a48ba
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 3da6608..c794d49 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1981,8 +1981,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 d0cef42ff8a0ed64026bb01b14913adde5da439a
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 fc0049b..8320ed8 100644
--- a/share/html/Elements/ValidateCustomFields
+++ b/share/html/Elements/ValidateCustomFields
@@ -95,27 +95,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, $CF->Name .': '. $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, $CF->Name .': '. $msg;
- $valid = 0;
- }
+ my $ref = { Content => $value };
+ my ($ok, $msg) = $CF->_CanonicalizeValue( $ref );
+ unless ($ok) {
+ $m->notes( ( 'InvalidField-' . $CF->Id ) => $msg );
+ push @res, $CF->Name .': '. $msg;
+ $valid = 0;
}
}
diff --git a/t/customfields/ip.t b/t/customfields/ip.t
index 2537b1e..35a245c 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 c99d1f1..2a323a3 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 3d73a23..445df33 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 9c82d0d..24f7c2a 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 cd94a08cbb807b16ccf985501085db3cf658a380
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/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index b8b2c2f..ce0e62d 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -529,13 +529,6 @@ sub _LimitCustomField {
$RT::Logger->warn("$value is not a valid IPAddress");
}
} elsif ( $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