All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Dave Borowitz <dborowitz@google.com>
Cc: Shawn Pearce <spearce@spearce.org>, git <git@vger.kernel.org>
Subject: Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Date: Mon, 06 Jul 2015 10:30:33 -0700	[thread overview]
Message-ID: <xmqqwpyd19dy.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAD0k6qT8=xQb6MRcLkyvZBm0MRdQ0Z-8ojqghovdgeJQ2EBNEA@mail.gmail.com> (Dave Borowitz's message of "Mon, 6 Jul 2015 13:11:45 -0400")

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).

> 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.

  parent reply	other threads:[~2015-07-06 17:30 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-01 18:08 [PATCH 0/7] Clarify signed push protocol documentation Dave Borowitz
2015-07-01 18:08 ` [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies Dave Borowitz
2015-07-01 19:39   ` Jonathan Nieder
2015-07-01 19:52     ` Junio C Hamano
2015-07-01 19:56     ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 2/7] pack-protocol.txt: Mark LF in command-list as optional Dave Borowitz
2015-07-01 18:21   ` Stefan Beller
2015-07-01 18:46     ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required Dave Borowitz
2015-07-01 20:00   ` Junio C Hamano
2015-07-01 20:07     ` Dave Borowitz
2015-07-01 20:49       ` Junio C Hamano
2015-07-06 14:46         ` Dave Borowitz
2015-07-06 15:22           ` Dave Borowitz
2015-07-06 15:27             ` Dave Borowitz
2015-07-06 15:29               ` Dave Borowitz
2015-07-06 15:35             ` Dave Borowitz
2015-07-06 16:12             ` Junio C Hamano
2015-07-06 15:46         ` Shawn Pearce
2015-07-06 16:28           ` Junio C Hamano
2015-07-06 16:28           ` Junio C Hamano
2015-07-06 16:38             ` Dave Borowitz
2015-07-06 16:57               ` Junio C Hamano
2015-07-06 17:11                 ` Dave Borowitz
2015-07-06 17:18                   ` Dave Borowitz
2015-07-06 17:34                     ` Junio C Hamano
2015-07-06 17:38                       ` Dave Borowitz
2015-07-06 18:06                         ` Junio C Hamano
2015-07-06 18:08                           ` Dave Borowitz
2015-07-06 18:23                           ` Junio C Hamano
2015-07-06 17:30                   ` Junio C Hamano [this message]
2015-07-06 17:35                     ` Dave Borowitz
2015-07-06 17:59                       ` Junio C Hamano
2015-07-01 20:36     ` Junio C Hamano
2015-07-01 20:39       ` Junio C Hamano
2015-07-02 13:53         ` Jeff King
2015-07-03 17:45           ` Junio C Hamano
2015-07-03 18:07             ` Jeff King
2015-07-03 18:43               ` Shawn Pearce
2015-07-03 18:46                 ` Jeff King
2015-07-01 18:08 ` [PATCH 4/7] pack-protocol.txt: Elaborate on pusher identity Dave Borowitz
2015-07-01 18:58   ` Junio C Hamano
2015-07-01 18:08 ` [PATCH 5/7] pack-protocol.txt: Be more precise about pusher-key relationship Dave Borowitz
2015-07-01 18:08 ` [PATCH 6/7] pack-protocol.txt: Mark pushee field as optional Dave Borowitz
2015-07-01 18:56   ` Junio C Hamano
2015-07-01 19:06     ` Dave Borowitz
2015-07-01 19:07     ` Junio C Hamano
2015-07-01 19:08       ` Junio C Hamano
2015-07-01 19:31       ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 7/7] send-pack.c: Die if the nonce is empty Dave Borowitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwpyd19dy.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=dborowitz@google.com \
    --cc=git@vger.kernel.org \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.