[Rt-commit] rt branch, 4.0/disallow-blank-field-names, created. rt-4.0.2-193-gb101a39
Jason May
jasonmay at bestpractical.com
Fri Oct 14 16:55:27 EDT 2011
The branch, 4.0/disallow-blank-field-names has been created
at b101a39552c290f5924e67247e1ef1f2c9ae32d5 (commit)
- Log -----------------------------------------------------------------
commit 4adf2a4f271d7e156c50701b1df03567b4b07318
Author: Jason May <jasonmay at bestpractical.com>
Date: Thu Oct 13 16:39:02 2011 -0400
Disallow '0' as well as other integers for the Name column
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index fcc7bed..2833c9f 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -591,10 +591,10 @@ Validate the name of the record we're creating. Mostly, just make sure it's not
sub ValidateName {
my $self = shift;
my $value = shift;
- if ($value && $value=~ /^\d+$/) {
+ if (defined $value && $value=~ /^\d+$/) {
return(0);
} else {
- return (1);
+ return(1);
}
}
commit 0832ade8678192576d12d4718fe0cce47e79ab05
Author: Jason May <jasonmay at bestpractical.com>
Date: Fri Oct 14 10:59:10 2011 -0400
Do not set CF properties unless it was created.
In order to set these properties. The CF has to be a valid object (i.e.
have an id) to check user rights.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 031fa9e..3f3f1c5 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -388,22 +388,24 @@ sub Create {
Repeated => $args{'Repeated'},
);
- if ( exists $args{'LinkValueTo'}) {
- $self->SetLinkValueTo($args{'LinkValueTo'});
- }
+ if ($rv) {
+ if ( exists $args{'LinkValueTo'}) {
+ $self->SetLinkValueTo($args{'LinkValueTo'});
+ }
- if ( exists $args{'IncludeContentForValue'}) {
- $self->SetIncludeContentForValue($args{'IncludeContentForValue'});
- }
+ if ( exists $args{'IncludeContentForValue'}) {
+ $self->SetIncludeContentForValue($args{'IncludeContentForValue'});
+ }
- return ($rv, $msg) unless exists $args{'Queue'};
+ return ($rv, $msg) unless exists $args{'Queue'};
- # Compat code -- create a new ObjectCustomField mapping
- my $OCF = RT::ObjectCustomField->new( $self->CurrentUser );
- $OCF->Create(
- CustomField => $self->Id,
- ObjectId => $args{'Queue'},
- );
+ # Compat code -- create a new ObjectCustomField mapping
+ my $OCF = RT::ObjectCustomField->new( $self->CurrentUser );
+ $OCF->Create(
+ CustomField => $self->Id,
+ ObjectId => $args{'Queue'},
+ );
+ }
return ($rv, $msg);
}
commit f0d04a788987ac26a2a81ad8e1dd66ffe590cb14
Author: Jason May <jasonmay at bestpractical.com>
Date: Wed Sep 14 09:39:03 2011 -0400
Do not allow CFs to be created with the name '' or an integer
It does not make sense to have a CF with a blank name.
Allowing CF to have an integer for a name confuses the Load method, as
it acccepts both 'name' values and IDs.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 3f3f1c5..52d4423 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -589,6 +589,20 @@ sub DeleteValue {
}
+=head2 ValidateQueue Queue
+
+Make sure that the name specified is valid
+
+=cut
+
+sub ValidateName {
+ my $self = shift;
+ my $value = shift;
+
+ return 0 unless length $value;
+
+ return $self->SUPER::ValidateName($value);
+}
=head2 ValidateQueue Queue
diff --git a/t/web/cf_access.t b/t/web/cf_access.t
index 0eaf7b3..73b7765 100644
--- a/t/web/cf_access.t
+++ b/t/web/cf_access.t
@@ -1,7 +1,7 @@
#!/usr/bin/perl -w
use strict;
-use RT::Test tests => 25;
+use RT::Test tests => 32;
my ($baseurl, $m) = RT::Test->started_ok;
@@ -13,6 +13,42 @@ ok $m->login, 'logged in';
diag "Create a CF";
{
$m->follow_link( id => 'tools-config-custom-fields-create');
+
+ # Test form validation
+ $m->submit_form(
+ form_name => "ModifyCustomField",
+ fields => {
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '',
+ Description => 'img',
+ },
+ );
+ $m->text_contains('Invalid value for Name');
+
+ $m->submit_form(
+ form_name => "ModifyCustomField",
+ fields => {
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '0',
+ Description => 'img',
+ },
+ );
+ $m->text_contains('Invalid value for Name');
+
+ $m->submit_form(
+ form_name => "ModifyCustomField",
+ fields => {
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '1',
+ Description => 'img',
+ },
+ );
+ $m->text_contains('Invalid value for Name');
+
+ # The real submission
$m->submit_form(
form_name => "ModifyCustomField",
fields => {
@@ -22,6 +58,36 @@ diag "Create a CF";
Description => 'img',
},
);
+ $m->text_contains('Object created');
+
+ # Validation on update
+ $m->form_name("ModifyCustomField");
+ $m->set_fields(
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '',
+ Description => 'img',
+ );
+ $m->click('Update');
+ $m->text_contains('Illegal value for Name');
+ $m->form_name("ModifyCustomField");
+ $m->set_fields(
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '0',
+ Description => 'img',
+ );
+ $m->click('Update');
+ $m->text_contains('Illegal value for Name');
+ $m->form_name("ModifyCustomField");
+ $m->set_fields(
+ TypeComposite => 'Image-0',
+ LookupType => 'RT::Queue-RT::Ticket',
+ Name => '1',
+ Description => 'img',
+ );
+ $m->click('Update');
+ $m->text_contains('Illegal value for Name');
}
diag "apply the CF to General queue";
commit f8102a3319d6248eed1aafaf606ac34df169f0a5
Author: Jason May <jasonmay at bestpractical.com>
Date: Wed Sep 14 11:15:05 2011 -0400
Do not allow groups to be created with the name '' or an integer
See 35e1428e12c for a similar rationale.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 1ee0591..2c42553 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -506,14 +506,15 @@ Enforces unique user defined group names when updating
=cut
sub ValidateName {
- my ($self, $value) = @_;
+ my $self = shift;
+ my $value = shift;
if ($self->Domain and $self->Domain eq 'UserDefined') {
my ($ok, $msg) = $self->_ValidateUserDefinedName($value);
# It's really too bad we can't pass along the actual error
return 0 if not $ok;
}
- return 1;
+ return $self->SUPER::ValidateName($value);
}
=head2 _ValidateUserDefinedName VALUE
@@ -524,6 +525,9 @@ Returns true if the user defined group name isn't in use, false otherwise.
sub _ValidateUserDefinedName {
my ($self, $value) = @_;
+
+ return (0, 'Name is required') unless length $value;
+
my $dupcheck = RT::Group->new(RT->SystemUser);
$dupcheck->LoadUserDefinedGroup($value);
return (0, $self->loc("Group name '[_1]' is already in use", $value))
diff --git a/share/html/Admin/Groups/Modify.html b/share/html/Admin/Groups/Modify.html
index b5b0aad..9ad48e8 100755
--- a/share/html/Admin/Groups/Modify.html
+++ b/share/html/Admin/Groups/Modify.html
@@ -52,7 +52,7 @@
-<form action="<%RT->Config->Get('WebPath')%>/Admin/Groups/Modify.html" method="post" enctype="multipart/form-data">
+<form action="<%RT->Config->Get('WebPath')%>/Admin/Groups/Modify.html" name="ModifyGroup" method="post" enctype="multipart/form-data">
%unless ($Group->Id) {
<input type="hidden" class="hidden" name="id" value="new" />
diff --git a/t/web/group_create.t b/t/web/group_create.t
new file mode 100644
index 0000000..26da592
--- /dev/null
+++ b/t/web/group_create.t
@@ -0,0 +1,75 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use RT::Test tests => 13;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok $m->login, 'logged in as root';
+my $root = RT::User->new(RT->SystemUser);
+ok( $root->Load('root'), 'load root user' );
+
+my $group_name = 'test group';
+
+my $group_id;
+diag "Create a group";
+{
+ $m->follow_link( id => 'tools-config-groups-create');
+
+ # Test group form validation
+ $m->submit_form(
+ form_name => 'ModifyGroup',
+ fields => {
+ Name => '',
+ },
+ );
+ $m->text_contains('Name is required');
+ $m->submit_form(
+ form_name => 'ModifyGroup',
+ fields => {
+ Name => '0',
+ },
+ );
+ $m->text_contains('Could not create group');
+ $m->submit_form(
+ form_name => 'ModifyGroup',
+ fields => {
+ Name => '1',
+ },
+ );
+ $m->text_contains('Could not create group');
+ $m->submit_form(
+ form_name => 'ModifyGroup',
+ fields => {
+ Name => $group_name,
+ },
+ );
+ $m->content_contains('Group created', 'created group sucessfully' );
+
+ # Test validation on updae
+ $m->form_name('ModifyGroup');
+ $m->set_fields(
+ Name => '',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->text_contains('Illegal value for Name');
+
+ $m->form_name('ModifyGroup');
+ $m->set_fields(
+ Name => '0',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->text_contains('Illegal value for Name');
+
+ $m->form_name('ModifyGroup');
+ $m->set_fields(
+ Name => '1',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->text_contains('Illegal value for Name');
+
+ $group_id = $m->form_name('ModifyGroup')->value('id');
+ ok $group_id, "found id of the group in the form, it's #$group_id";
+}
+
commit 4834d77f3bcd4691640b0ee578f3398fa47ed70a
Author: Jason May <jasonmay at bestpractical.com>
Date: Wed Sep 14 13:14:36 2011 -0400
Bring us back to Create state if validation is false, not true
Without this fix, form assumes a queue was created if validation really
does fail and a queue was never created.
diff --git a/share/html/Admin/Queues/Modify.html b/share/html/Admin/Queues/Modify.html
index 767ea62..fcbfcd6 100755
--- a/share/html/Admin/Queues/Modify.html
+++ b/share/html/Admin/Queues/Modify.html
@@ -171,7 +171,7 @@ if ($Create) {
} else {
if ( defined $id && $id eq 'new' ) {
my ($val, $msg) = $QueueObj->Create( Name => $Name );
- if ($val) {
+ if (!$val) {
$Create = 1; # Create failed, so bring us back to step 1
}
push @results, $msg;
commit e42335dc0db4cf1e9e4276d5c57b9032809faf98
Author: Jason May <jasonmay at bestpractical.com>
Date: Wed Sep 14 15:28:46 2011 -0400
Prevent undef in loc()'s argument to avoid a cryptic warning:
Use of uninitialized value $_[1] in join or string at (eval 1755) line
2.
diff --git a/share/html/Admin/Queues/Modify.html b/share/html/Admin/Queues/Modify.html
index fcbfcd6..74d929d 100755
--- a/share/html/Admin/Queues/Modify.html
+++ b/share/html/Admin/Queues/Modify.html
@@ -179,7 +179,7 @@ if ($Create) {
else {
$QueueObj->Load($id) || $QueueObj->Load($Name) || Abort(loc("Couldn't load queue '[_1]'", $Name));
}
- $title = loc('Configuration for queue [_1]', $QueueObj->Name);
+ $title = loc('Configuration for queue [_1]', $QueueObj->Name||'');
}
if ( $QueueObj->Id ) {
my @attribs= qw(Description CorrespondAddress CommentAddress Name
commit ae23f22a5dfc5347485047beae2de54ea96edd03
Author: Jason May <jasonmay at bestpractical.com>
Date: Fri Oct 14 12:33:04 2011 -0400
Factor validation message returning to _ValidateName
SearchBuilder expects a scalar value from ValidateFoo so we can't return
a list. However, it would still be nice to have specific error messages
in some cases, so we'll keep the old logic in _ValidateName in this
case.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 73733fc..b41a215 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -405,7 +405,7 @@ sub Create {
}
{
- my ($val, $msg) = $self->ValidateName( $args{'Name'} );
+ my ($val, $msg) = $self->_ValidateName( $args{'Name'} );
if (!$val) {
return ($val, $self->loc($msg));
@@ -538,6 +538,15 @@ sub ValidateName {
my $self = shift;
my $name = shift;
+ my ($ok, $msg) = $self->_ValidateName($name);
+
+ return $ok ? 1 : 0;
+}
+
+sub _ValidateName {
+ my $self = shift;
+ my $name = shift;
+
my $tempqueue = RT::Queue->new(RT->SystemUser);
$tempqueue->Load($name);
@@ -553,7 +562,6 @@ sub ValidateName {
else {
return (undef, "That's not a valid name.");
}
-
}
commit b101a39552c290f5924e67247e1ef1f2c9ae32d5
Author: Jason May <jasonmay at bestpractical.com>
Date: Wed Sep 14 15:34:10 2011 -0400
Do not allow queues to be created with the name '' or an integer
See 35e1428e12c for a similar rationale.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index b41a215..1a4f85c 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -547,6 +547,15 @@ sub _ValidateName {
my $self = shift;
my $name = shift;
+ return (undef, "Queue name is required") unless length $name;
+
+ # Validate via the superclass first
+ # Case: short circuit if it's an integer so we don't have
+ # fale negatives when loading a temp queue
+ unless ( my $q = $self->SUPER::ValidateName($name) ) {
+ return ($q, "That's not a valid name.");
+ }
+
my $tempqueue = RT::Queue->new(RT->SystemUser);
$tempqueue->Load($name);
@@ -555,13 +564,7 @@ sub _ValidateName {
return (undef, "Queue already exists.");
}
- #If the queue doesn't exist, return 1
- elsif (my $q = $self->SUPER::ValidateName($name)) {
- return ($q);
- }
- else {
- return (undef, "That's not a valid name.");
- }
+ return (1);
}
diff --git a/share/html/Admin/Queues/Modify.html b/share/html/Admin/Queues/Modify.html
index 74d929d..153fd88 100755
--- a/share/html/Admin/Queues/Modify.html
+++ b/share/html/Admin/Queues/Modify.html
@@ -51,7 +51,7 @@
-<form action="<%RT->Config->Get('WebPath')%>/Admin/Queues/Modify.html" method="post">
+<form action="<%RT->Config->Get('WebPath')%>/Admin/Queues/Modify.html" name="ModifyQueue" method="post">
<input type="hidden" class="hidden" name="SetEnabled" value="1" />
<input type="hidden" class="hidden" name="id" value="<% $Create? 'new': $QueueObj->Id %>" />
diff --git a/t/web/queue_create.t b/t/web/queue_create.t
new file mode 100644
index 0000000..ae7106b
--- /dev/null
+++ b/t/web/queue_create.t
@@ -0,0 +1,75 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use RT::Test tests => 13;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok $m->login, 'logged in as root';
+my $root = RT::User->new(RT->SystemUser);
+ok( $root->Load('root'), 'load root user' );
+
+my $queue_name = 'test queue';
+
+my $queue_id;
+diag "Create a queue";
+{
+ $m->follow_link( id => 'tools-config-queues-create');
+
+ # Test queue form validation
+ $m->submit_form(
+ form_name => 'ModifyQueue',
+ fields => {
+ Name => '',
+ },
+ );
+ $m->text_contains('Queue name is required');
+ $m->submit_form(
+ form_name => 'ModifyQueue',
+ fields => {
+ Name => '0',
+ },
+ );
+ $m->text_contains("That's not a valid name");
+ $m->submit_form(
+ form_name => 'ModifyQueue',
+ fields => {
+ Name => '1',
+ },
+ );
+ $m->text_contains("That's not a valid name");
+ $m->submit_form(
+ form_name => 'ModifyQueue',
+ fields => {
+ Name => $queue_name,
+ },
+ );
+ $m->content_contains('Queue created', 'created queue sucessfully' );
+
+ # Test validation on update
+ $m->form_name('ModifyQueue');
+ $m->set_fields(
+ Name => '',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->content_contains('Illegal value for Name');
+
+ $m->form_name('ModifyQueue');
+ $m->set_fields(
+ Name => '0',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->content_contains('Illegal value for Name');
+
+ $m->form_name('ModifyQueue');
+ $m->set_fields(
+ Name => '1',
+ );
+ $m->click_button(value => 'Save Changes');
+ $m->content_contains('Illegal value for Name');
+
+ $queue_id = $m->form_name('ModifyQueue')->value('id');
+ ok $queue_id, "found id of the queue in the form, it's #$queue_id";
+}
+
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list