Re: [PATCH 3/5] gitweb: Return plain booleans in validation methods

From: Jakub Narębski <jnareb@gmail.com>
Date: Dec 4, 2013, 16:07 UTC
Message-ID: <CANQwDweGgQpnqDqaekBWt-rczjHkJEmFyXW9qDh2En2r=ZXbMw@mail.gmail.com>
In-Reply-To: <1386164583-14109-4-git-send-email-krzesimir@endocode.com>

On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak <krzesimir@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"));

I would say that the problem was that validate_sth() subroutines returned value of parameter if it was valid, which could be a problem if said value is false-ish (e.g. validate_refname("0"), or validate_pathname("0")).

Returning undef on invalid data newer was a problem, using 'return $input;' on valid input was, especially that validate_sth() functions were ever used in a conditional:

  if (!validate_sth($param)) {
      die_error(...)
  }

While at it validate_sth() is not a best name for boolean predicate: is_valid_sth() would be better, I think.

Show 22 quoted lines

> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
> ---
>  gitweb/gitweb.perl | 45 +++++++++++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 67415b9..3434602 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1419,63 +1419,68 @@ sub href {
>  ## validation, quoting/unquoting and escaping
>
>  sub validate_action {
> -       my $input = shift || return undef;
> -       return undef unless exists $actions{$input};
> -       return $input;
> +       my $input = shift;
> +
> +       return 0 unless defined $input;
> +       return 0 unless exists $actions{$input};
> +       return 1;
>  }

The only change that needs to be doe is replacing

           return $input;

with

           return 1;

-- 
Jakub Narebski