Re: [PATCH 3/5] gitweb: Return plain booleans in validation methods
- From
Krzesimir Nowak <krzesimir@endocode.com>
- Date
- Dec 5, 2013, 09:23 UTC
- Message-ID
- <1386235422.2186.7.camel@localhost.localdomain>
- In-Reply-To
- <CANQwDweGgQpnqDqaekBWt-rczjHkJEmFyXW9qDh2En2r=ZXbMw@mail.gmail.com>
On Wed, 2013-12-04 at 17:07 +0100, Jakub Narębski wrote:
Show 9 quoted lines
> 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")).That's what I meant.
Show 11 quoted lines
>
> 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.Ok, I'll rename those.
Show 32 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;
> I prefer to use zeros instead of undefs - one might wonder if that undef is somewhat special that we can't use 0.
-- Krzesimir Nowak Software Developer Endocode AG krzesimir@endocode.com ------ Endocode AG, Johannisstraße 20, 10117 Berlin info@endocode.com | www.endocode.com Vorstandsvorsitzender: Mirko Boehm Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker Aufsichtsratsvorsitzende: Jennifer Beecher Registergericht: Amtsgericht Charlottenburg - HRB 150748 B