Re: [RFC/PATCH 0/3] protocol v2

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 4, 2015, 12:05 UTC
Message-ID: <CACsJy8B=MN412dZYYc=NwpXHVz=Pxv+8wWMT7JNurVgqKrWn8A@mail.gmail.com>
In-Reply-To: <CAJo=hJtgda4ATtPeLWbhgDPiTm9h-gzBkpknxsOdRddueUgTbw@mail.gmail.com>

On Wed, Mar 4, 2015 at 11:27 AM, Shawn Pearce <spearce@spearce.org> wrote:

Show 8 quoted lines

> Let me go on a different tangent a bit from the current protocol.
>
> http://www.grpc.io/ was recently released and is built on the HTTP/2
> standard. It uses protobuf as a proven extensibility mechanism.
> Including a full C based grpc stack just to speak the Git wire
> protocol is quite likely overkill, but I think the embedding of a
> proven extensible format inside of a bi-directional framed streaming
> system like HTTP/2 offers some good guidance.

I'll take this as "learn from grpc, not just reuse grpc"

Show 6 quoted lines

> Network protocol parsing is hard. Especially in languages like C where
> buffer overflows are possible. Or where a client could trivially DoS a
> server by sending a packet of size uint_max and the server naively
> trying to malloc() that buffer. Defining the network protocol in an
> IDL like protobuf 3 and being machine generated from stable well
> maintained code has its advantages.

I'm still studying the spec, so I can't comment if using IDL/protobuf3 is a good idea yet.

But I think at least we can avoid DoS by changing the pkt-line (again) a bit: the length 0xffff means that actual length is 0xfffe and the next pkt-line is part of this pkt-line. Higher level (upload-pack or fetch-pack, for example) must set an upper limit for packet_read() so it won't try to concatenate pkt-lines forever.

-- 
Duy