Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect

From: Jeff King <peff@peff.net>
Date: Dec 1, 2016, 19:59 UTC
Message-ID: <20161201195902.td4zfolqpc3uwfgq@sigill.intra.peff.net>
In-Reply-To: <xmqqbmwva0im.fsf@gitster.mtv.corp.google.com>

On Thu, Dec 01, 2016 at 11:54:09AM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> > I'm not sure if we should call this "redirect" here. That's how it's
> > used by the curl code, but I think from the perspective of the transport
> > whitelist, it is really "are you overriding the from_user environment".
> >
> > Calling it "from_user" may be confusing though, as the default value
> > would become "1", even though it means only "as far as I know this is
> > from the user, but maybe the environment says otherwise". So bizarrely,
> > I think calling it "not_from_user" is the clearest value.
> 
> Bikeshedding: perhaps call it "unsafe" (in the sense that it is "not
> known to be safe")?

That is definitely what we are going for, but it is vague about how it is unsafe. :)

I think I may have converted Brandon in the other thread to my way of thinking of it as a tristate[1]. That lets us call it "from_user", and just do:

  case PROTOCOL_ALLOW_FROM_USER:
	if (from_user < 0)
		from_user = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
	return from_user;

which is pretty clear. Nobody would ever pass "1" as from_user to the function, but it does the sensible thing if they do.

-Peff

[1] The original I posted calling it "redirect" was totally bogus
    because the logic between the two names is inverted.