Re: how does "clone --filter=sparse:path" work?

From: Christian Couder <christian.couder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Jeff Hostetler <git@jeffhostetler.com>, git <git@vger.kernel.org>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Matthew DeVore <matvore@google.com>
Subject: Re: how does "clone --filter=sparse:path" work?
Date: Fri, 24 May 2019 10:05:45 +0200	[thread overview]
Message-ID: <CAP8UFD0XbOUj70pt4X=HDvGBoLaG9qBv9SWGnM6N8FG3t-57rg@mail.gmail.com> (raw)
In-Reply-To: <20181122173931.GA22123@sigill.intra.peff.net>

(Sorry for the late reply to this.)

On Sat, Nov 24, 2018 at 8:07 AM Jeff King <peff@peff.net> wrote:
>
> On Thu, Nov 08, 2018 at 01:57:52PM -0500, Jeff Hostetler wrote:
>
> > > Should we simply be disallowing sparse:path filters over upload-pack?

I agree that it should either be disallowed or heavily restricted.

> > The option to allow an absolute path over the wire probably needs more
> > thought as you suggest.
> >
> > Having it in the traverse code was useful for local testing in the
> > client.
> >
> > But mainly I was thinking of a use case on the client of the form:
> >
> >     git rev-list
> >         --objects
> >         --filter=spec:path=.git/sparse-checkout

Do you mean "sparse:path" instead of "spec:path"?

> >         --missing=print
> >         <commit>
> >
> > and get a list of the blobs that you don't have and would need before
> > you could checkout <commit> using the current sparse-checkout definition.
> > You could then have a pre-checkout hook that would bulk
> > fetch them before starting the actual checkout.  Since that would be
> > more efficient than demand-loading blobs individually during the
> > checkout.  There's more work to do in this area, but that was the idea.
> >
> > But back to your point, yes, I think we should restrict this over the
> > wire.
>
> Thanks for your thorough response, and sorry for the slow reply. I had
> meant to reply with a patch adding in the restriction, but I haven't
> quite gotten to it. :)

The way I see it could be restricted is by adding a config option on
the server, maybe called "uploadpack.sparsePathFilter", to tell which
filenames can be accessed using "--filter=sparse:path=".

For example with uploadpack.sparsePathFilter set to
"/home/user/git/sparse/*" and "--filter=sparse:path=foo" then
"/home/user/git/sparse/foo" on the server would be used if it exists.
(Of course care should be taken that things like
"--filter=sparse:path=bar/../../foo" are rejected.)

If uploadpack.sparsePathFilter is unset or set to "false", then
"--filter=sparse:path=<stuff>" would always error out.

Is this what you had in mind?