Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

[Shawn Pearce <spearce@spearce.org>]

On Sun, Jul 10, 2016 at 11:05 AM, Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jul 10, 2016 at 10:06 AM, Shawn Pearce <spearce@spearce.org> wrote:
>> On Fri, Jul 8, 2016 at 5:31 PM, Stefan Beller <sbeller@google.com> wrote:
>>> +
>>> +       /* NEEDSWORK: expose the limitations to be configurable. */
>>> +       int max_options = 32;
>>> +
>>> +       /*
>>> +        * NEEDSWORK: expose the limitations to be configurable;
>>> +        * Once the limit can be lifted, include a way for payloads
>>> +        * larger than one pkt, e.g allow a payload of up to
>>> +        * LARGE_PACKET_MAX - 1 only, and reserve the last byte
>>> +        * to indicate whether the next pkt continues with this
>>> +        * push option.
>>> +        */
>>> +       int max_size = 1024;
>>
>> Instead of this, what about a new config variable
>> receive.maxCommandBytes[1] that places a limit on the number of bytes
>> of pkt-line data the client can supply in both the command list ("old
>> new ref"), push signature framing, and option list?

FYI https://git.eclipse.org/r/77108 is my proposal in JGit.

> The design here with two bounds was used to not care about the oversized
> push options for now. (I mean we can still just reject larger push
> options even when
> having a receive.maxCommandBytes setting.)

I don't really see a problem with a single option being nearly 64 KiB
in size. The pack file is going to (probably) need a lot more memory
than that to be unpacked and have SHA-1s generated. The
maxCommandBytes setting at 3 MiB neatly allows a few options to be up
to the pkt-line 64 KiB size, which is 1/10th of what anyone would ever
need. :)

> In an earlier discussion Jeff said roughly "either make it work well,
> or don't make it work at all, i.e. why are git push options better
> than a `git push .. && curl <server>/REST-API` thing?"

One reason this is less optimal is you need to setup authentication
twice. git push might be working over SSH, or over a custom
git-remote-NNN transport. But curl might not speak the same
authentication. Even if git and curl are both using .netrc or an HTTP
cookie file you need to pass the right options to make sure both use
the same auth.

Another is we can do validation of the incoming Git data in context of
what options you specified to us and refuse it while discarding the
pack if we don't like what was asked. With `git push && curl` we have
to save the data without necessarily knowing exactly what we should do
with it, just in case the user comes back with a corresponding REST
API call on another connection.