Re: [PATCH 2/2] index-pack: prefetch missing REF_DELTA bases

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Jonathan Tan <jonathantanmy@google.com>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] index-pack: prefetch missing REF_DELTA bases
Date: Thu, 16 May 2019 10:43:03 +0900	[thread overview]
Message-ID: <xmqqk1er5g48.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190515231617.GA1395@sigill.intra.peff.net> (Jeff King's message of "Wed, 15 May 2019 19:16:18 -0400")

Jeff King <peff@peff.net> writes:

> I agree that the current implementation (and probably any sane
> implementation) would not send us a delta if we have not provided any
> haves. But this does mean that a malicious server could send a client
> into an infinite loop.
>
> Pretty unlikely, but should we put some kind of circuit-breaker into the
> client to ensure this?

That's a pretty good point.  Would it be suffice to have a new
option to tell index-pack that fattens a thin pack and unpack-objects
that expands objects in a small incoming packfile into loose objects
that they are forbidden from on-demand fatching during this invocation,
as it is an error for the packfile they are digesting to depend on a
lazy objects?

> I dunno. Maybe we should just ignore it. It's a fundamental issue with
> partial clones that we're going to have to fetch extra junk here anyway,

Would it be an option not to ask for a thin pack in the first place?

> If we're willing to modify the format, one thing we _could_ do is have
> the server communicate the expectations for each base. I.e., introduce a
> new THIN_DELTA type that behaves exactly as a REF_DELTA, but with the
> extra 1-bit of knowledge that the server knows it is not including the
> base in the pack. I'm not sure how painful that retro-fitting would be.
> It would need at least a new capability and options to pack-objects and
> index-pack. We might be tight on bits in the packfile type field.

The type field is tight, but I wonder how much such a new
representation would help.  Unless the receiving end blindly trusts
what the sender says, there needs to be a logic to detect cyclic
dependencies while following such a delta chain to lazy-fill
promised objects on the receiving end anyway, no?