Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
- From
Dave Borowitz <dborowitz@google.com>
- Date
- Jul 1, 2015, 20:07 UTC
- Message-ID
- <CAD0k6qSN9=afCe3RumJPfP9JERy1w+tAYdjq01MsQnsOjdKu3A@mail.gmail.com>
- In-Reply-To
- <xmqqfv578x87.fsf@gitster.dls.corp.google.com>
On Wed, Jul 1, 2015 at 1:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> Dave Borowitz <dborowitz@google.com> writes: > >> Signed-off-by: Dave Borowitz <dborowitz@google.com> >> --- >> Documentation/technical/pack-protocol.txt | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/technical/pack-protocol.txt >> b/Documentation/technical/pack-protocol.txt >> index 1386840..2d8b1a1 100644 >> --- a/Documentation/technical/pack-protocol.txt >> +++ b/Documentation/technical/pack-protocol.txt >> @@ -534,6 +534,9 @@ A push certificate begins with a set of header >> lines. After the >> header and an empty line, the protocol commands follow, one per >> line. >> >> +Note that (unlike other portions of the protocol), all LFs in the >> +`push-cert` specification above MUST be present. >> + >> Currently, the following header fields are defined: >> >> `pusher` ident:: > > I am moderately negative about this; wouldn't it make the end result > cleaner to fix the implementation?
I'm not sure I understand your suggestion. Are you saying, you would prefer to make LFs optional in the push cert, for consistency with LFs being optional elsewhere?
C git servers in the wild already require LFs when extracting the nonce value from the certificate (see find_header). If we make the LFs optional, then a conforming client may not send LFs, which will cause nonce verification to fail when pushing to an unfixed server. That is why I think we are stuck with this.
(Also, this is probably not insurmountable, but the cert processing code in receive-pack.c would have to be substantially rewritten if we loosened this requirement. Currently it concatenates the cert contents without pkt-line framing into a buffer, and searches around for "\n" and "\n\n".
If LF is optional, then with that approach you might end up with a section of that buffer like: nonce 1234-abcd0000000000000000000000000000000000000000 deadbeefdeadbeefdeadbeefdeadbeefdeadbeef refs/heads/master where it is impossible to distinguish between the end of the nonce and the start of the first command.)