On 12/01, Jeff King wrote:
> On Thu, Dec 01, 2016 at 11:35:24AM -0800, Brandon Williams wrote:
>
> > > I wouldn't expect anyone to ever set GIT_PROTOCOL_FROM_USER=1, but it
> > > does behave in a funny way here, overriding the "redirect" flag. I think
> > > we'd want something more like:
> > >
> > > if (redirect < 0)
> > > redirect = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> > >
> > > and then pass in "-1" from transport_check_allowed().
> >
> > I don't think I quite follow your solution but I came up with this:
> >
> > case PROTOCOL_ALLOW_USER_ONLY:
> > return redirect ? 0 : git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> >
> > Which should address the same issue.
>
> I think mine was confused a bit by using the word "redirect". It was
> really meant to be "from_user", and could take three values: definitely
> yes, definitely no, and unknown (-1). And in the unknown case, we pull
> the value from the environment.
>
> Yours combines "definitely no" and "unknown" into a single value ("1" in
> your case, but that is because "redirect" and "from_user" have inverted
> logic from each other).
>
> I think that is OK, as there isn't any case where a caller would want to
> say "definitely no". The most they would say is "_I_ am not doing
> anything to make you think this value is not from the user", but we
> would still want to check the environment to see that nobody _else_ had
> put in such a restriction.
>
> -Peff
Oh ok, I see what you were going for now. That may be a better
(clearer) solution then what I just sent out. Mostly because we can
maintain the same logic polarity and use the same vocabulary.
--
Brandon Williams