Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2015, 17:30 UTC
Message-ID: <xmqqwpyd19dy.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAD0k6qT8=xQb6MRcLkyvZBm0MRdQ0Z-8ojqghovdgeJQ2EBNEA@mail.gmail.com>

Dave Borowitz <dborowitz@google.com> writes:

> I think I understand the confusion now. I think you and I are working
> from different assumptions about the client behavior.

I agree that we now both understand where we come from ;-)

And sorry for not being clear when I did the "push-cert" originally in the documentation. As I already said, "packets between push-cert and push-cert-end are contents of individual lines of the GPG signed push certificate" was the design meant from day one, and a85b377d (push: the beginning of "git push --signed", 2014-09-12) could have made it clearer.

> The problem with the documentation, then, is that the documentation
> does not say anything to indicate that the signed payload is anything
> other than what is on the wire.

Yeah, that was untold assumption, as I considered "what is on the wire" to include pkt-line framing when I wrote a85b377d (push: the beginning of "git push --signed", 2014-09-12).

Show 5 quoted lines

> So maybe this series should include an explicit description of the
> singed payload outside of the context of a push. Then, in the push
> section, we can describe the set of transformations that the client
> MUST perform (splitting on LF; adding pkt-line headers) and MAY
> perform (adding LFs).

Yes, and the latter is not limited to push-cert but anything sent on pkt-line.

That incidentally is the only point I deeply care about. I just want to minimize "the protocol is this way in general, but only for this one you must do it differently".

One example of "only for this one you must do it differently" is another caveat for protocol implementors for the sending side (again not limited to "push cert").

That existing implementations of the receivers treat an empty packet (i.e. "0004") as if it is the same as a flush packet (i.e. "0000"), so even if the sending side chooses to ignore the "SHOULD terminate each non-flush line using LF", it is strongly advised not to do so when it wants to send an empty payload. This needs to be documented.

The receiving end SHOULD NOT treat "0004" the same way as "0000". I think that must be documented and implementations (including our own) should be fixed.

Thanks.