[PATCH v7 2/4] gitweb: Return 1 on validation success instead of passed input

2013-12-11 Thread Krzesimir Nowak
Users of validate_* passing 0 might get failures on correct name
because of coercion of 0 to false in code like:
die_error(500, invalid ref) unless (check_ref_format (0));

Also, the validate_foo subs are renamed to is_valid_foo.

Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
 gitweb/gitweb.perl | 61 --
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 46bd6ac..b5a8a36 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -994,7 +994,7 @@ our ($action, $project, $file_name, $file_parent, $hash, 
$hash_parent, $hash_bas
 sub evaluate_and_validate_params {
our $action = $input_params{'action'};
if (defined $action) {
-   if (!validate_action($action)) {
+   if (!is_valid_action($action)) {
die_error(400, Invalid action parameter);
}
}
@@ -1002,7 +1002,7 @@ sub evaluate_and_validate_params {
# parameters which are pathnames
our $project = $input_params{'project'};
if (defined $project) {
-   if (!validate_project($project)) {
+   if (!is_valid_project($project)) {
undef $project;
die_error(404, No such project);
}
@@ -1010,21 +1010,21 @@ sub evaluate_and_validate_params {
 
our $project_filter = $input_params{'project_filter'};
if (defined $project_filter) {
-   if (!validate_pathname($project_filter)) {
+   if (!is_valid_pathname($project_filter)) {
die_error(404, Invalid project_filter parameter);
}
}
 
our $file_name = $input_params{'file_name'};
if (defined $file_name) {
-   if (!validate_pathname($file_name)) {
+   if (!is_valid_pathname($file_name)) {
die_error(400, Invalid file parameter);
}
}
 
our $file_parent = $input_params{'file_parent'};
if (defined $file_parent) {
-   if (!validate_pathname($file_parent)) {
+   if (!is_valid_pathname($file_parent)) {
die_error(400, Invalid file parent parameter);
}
}
@@ -1032,21 +1032,21 @@ sub evaluate_and_validate_params {
# parameters which are refnames
our $hash = $input_params{'hash'};
if (defined $hash) {
-   if (!validate_refname($hash)) {
+   if (!is_valid_refname($hash)) {
die_error(400, Invalid hash parameter);
}
}
 
our $hash_parent = $input_params{'hash_parent'};
if (defined $hash_parent) {
-   if (!validate_refname($hash_parent)) {
+   if (!is_valid_refname($hash_parent)) {
die_error(400, Invalid hash parent parameter);
}
}
 
our $hash_base = $input_params{'hash_base'};
if (defined $hash_base) {
-   if (!validate_refname($hash_base)) {
+   if (!is_valid_refname($hash_base)) {
die_error(400, Invalid hash base parameter);
}
}
@@ -1066,7 +1066,7 @@ sub evaluate_and_validate_params {
 
our $hash_parent_base = $input_params{'hash_parent_base'};
if (defined $hash_parent_base) {
-   if (!validate_refname($hash_parent_base)) {
+   if (!is_valid_refname($hash_parent_base)) {
die_error(400, Invalid hash parent base parameter);
}
}
@@ -1418,27 +1418,30 @@ sub href {
 ## ==
 ## validation, quoting/unquoting and escaping
 
-sub validate_action {
-   my $input = shift || return undef;
+sub is_valid_action {
+   my $input = shift;
return undef unless exists $actions{$input};
-   return $input;
+   return 1;
 }
 
-sub validate_project {
-   my $input = shift || return undef;
-   if (!validate_pathname($input) ||
+sub is_valid_project {
+   my $input = shift;
+
+   return unless defined $input;
+   if (!is_valid_pathname($input) ||
!(-d $projectroot/$input) ||
!check_export_ok($projectroot/$input) ||
($strict_export  !project_in_list($input))) {
return undef;
} else {
-   return $input;
+   return 1;
}
 }
 
-sub validate_pathname {
-   my $input = shift || return undef;
+sub is_valid_pathname {
+   my $input = shift;
 
+   return undef unless defined $input;
# no '.' or '..' as elements of path, i.e. no '.' nor '..'
# at the beginning, at the end, and between slashes.
# also this catches doubled slashes
@@ -1449,33 +1452,33 @@ sub validate_pathname {
if ($input =~ m!\0!) {
 

Re: [PATCH v7 2/4] gitweb: Return 1 on validation success instead of passed input

2013-12-11 Thread Jakub Narębski
On Wed, Dec 11, 2013 at 12:54 PM, Krzesimir Nowak
krzesi...@endocode.com wrote:

 Users of validate_* passing 0 might get failures on correct name
 because of coercion of 0 to false in code like:
 die_error(500, invalid ref) unless (check_ref_format (0));

Very minor issue: there is no check_ref_format() after v7; perhaps
validate_pathname / is_valid_pathname

 Also, the validate_foo subs are renamed to is_valid_foo.

is_valid_foo is better than validate_foo IMVHO, though still not perfect
(it does some validation, but complete validation is done by git command).


 Signed-off-by: Krzesimir Nowak krzesi...@endocode.com

-- 
Jakub Narebski
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html