pack-protocol: clarify LF-handling in PKT-LINE()

The spec is very inconsistent about which PKT-LINE() parts of the grammar include a LF. On top of that, the code is not consistent, either (e.g., send-pack does not put newlines into the ref-update commands it sends). Let's make explicit the long-standing expectation that we generally expect pkt-lines to end in a newline, but that receivers should be lenient. This makes the spec consistent, and matches what git already does (though it does not always fulfill the SHOULD). We do make an exception for the push-cert, where the receiving code is currently a bit pickier. This is a reasonable way to be, as the data needs to be byte-for-byte compatible with what was signed. We _could_ make up some rules about signing a canonicalized version including newlines, but that would require a code change, and is out of scope for this patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 3, 2015 at 04:24 UTC 1c9b659d9837fa2bd6ab21edaae94d19c20ac216
2 files changed +30 -21
Documentation/technical/pack-protocol.txt
+26 -20
@@ -14,6 +14,14 @@ data. The protocol functions to have a server tell a client what is
14 currently on the server, then for the two to negotiate the smallest amount
15 of data to send in order to fully update one or the other.
16
17 +pkt-line Format
18 +---------------
19 +
20 +The descriptions below build on the pkt-line format described in
21 +protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
22 +otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
23 +include a LF, but the receiver MUST NOT complain if it is not present.
24 +
25 Transports
26 ----------
27 There are three transports over which the packfile protocol is
@@ -143,9 +151,6 @@ with the object name that each reference currently points to.
151 003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}
152 0000
153
146 -Server SHOULD terminate each non-flush line using LF ("\n") terminator;
147 -client MUST NOT complain if there is no terminator.
148 -
154 The returned response is a pkt-line stream describing each ref and
155 its current value. The stream MUST be sorted by name according to
156 the C locale ordering.
@@ -165,15 +170,15 @@ MUST peel the ref if it's an annotated tag.
170 flush-pkt
171
172 no-refs = PKT-LINE(zero-id SP "capabilities^{}"
168 - NUL capability-list LF)
173 + NUL capability-list)
174
175 list-of-refs = first-ref *other-ref
176 first-ref = PKT-LINE(obj-id SP refname
172 - NUL capability-list LF)
177 + NUL capability-list)
178
179 other-ref = PKT-LINE(other-tip / other-peeled)
175 - other-tip = obj-id SP refname LF
176 - other-peeled = obj-id SP refname "^{}" LF
180 + other-tip = obj-id SP refname
181 + other-peeled = obj-id SP refname "^{}"
182
183 shallow = PKT-LINE("shallow" SP obj-id)
184
@@ -216,8 +221,8 @@ out of what the server said it could do with the first 'want' line.
221
222 depth-request = PKT-LINE("deepen" SP depth)
223
219 - first-want = PKT-LINE("want" SP obj-id SP capability-list LF)
220 - additional-want = PKT-LINE("want" SP obj-id LF)
224 + first-want = PKT-LINE("want" SP obj-id SP capability-list)
225 + additional-want = PKT-LINE("want" SP obj-id)
226
227 depth = 1*DIGIT
228 ----
@@ -284,7 +289,7 @@ so that there is always a block of 32 "in-flight on the wire" at a time.
289 compute-end
290
291 have-list = *have-line
287 - have-line = PKT-LINE("have" SP obj-id LF)
292 + have-line = PKT-LINE("have" SP obj-id)
293 compute-end = flush-pkt / PKT-LINE("done")
294 ----
295
@@ -348,10 +353,10 @@ Then the server will start sending its packfile data.
353
354 ----
355 server-response = *ack_multi ack / nak
351 - ack_multi = PKT-LINE("ACK" SP obj-id ack_status LF)
356 + ack_multi = PKT-LINE("ACK" SP obj-id ack_status)
357 ack_status = "continue" / "common" / "ready"
353 - ack = PKT-LINE("ACK SP obj-id LF)
354 - nak = PKT-LINE("NAK" LF)
358 + ack = PKT-LINE("ACK" SP obj-id)
359 + nak = PKT-LINE("NAK")
360 ----
361
362 A simple clone may look like this (with no 'have' lines):
@@ -467,10 +472,10 @@ references.
472 ----
473 update-request = *shallow ( command-list | push-cert ) [packfile]
474
470 - shallow = PKT-LINE("shallow" SP obj-id LF)
475 + shallow = PKT-LINE("shallow" SP obj-id)
476
472 - command-list = PKT-LINE(command NUL capability-list LF)
473 - *PKT-LINE(command LF)
477 + command-list = PKT-LINE(command NUL capability-list)
478 + *PKT-LINE(command)
479 flush-pkt
480
481 command = create / delete / update
@@ -521,7 +526,8 @@ Push Certificate
526
527 A push certificate begins with a set of header lines. After the
528 header and an empty line, the protocol commands follow, one per
524 -line.
529 +line. Note that the the trailing LF in push-cert PKT-LINEs is _not_
530 +optional; it must be present.
531
532 Currently, the following header fields are defined:
533
@@ -560,12 +566,12 @@ update was successful, or 'ng [refname] [error]' if the update was not.
566 1*(command-status)
567 flush-pkt
568
563 - unpack-status = PKT-LINE("unpack" SP unpack-result LF)
569 + unpack-status = PKT-LINE("unpack" SP unpack-result)
570 unpack-result = "ok" / error-msg
571
572 command-status = command-ok / command-fail
567 - command-ok = PKT-LINE("ok" SP refname LF)
568 - command-fail = PKT-LINE("ng" SP refname SP error-msg LF)
573 + command-ok = PKT-LINE("ok" SP refname)
574 + command-fail = PKT-LINE("ng" SP refname SP error-msg)
575
576 error-msg = 1*(OCTECT) ; where not "ok"
577 ----
Documentation/technical/protocol-common.txt
+4 -1
@@ -62,7 +62,10 @@ A pkt-line MAY contain binary data, so implementors MUST ensure
62 pkt-line parsing/formatting routines are 8-bit clean.
63
64 A non-binary line SHOULD BE terminated by an LF, which if present
65 -MUST be included in the total length.
65 +MUST be included in the total length. Receivers MUST treat pkt-lines
66 +with non-binary data the same whether or not they contain the trailing
67 +LF (stripping the LF if present, and not complaining when it is
68 +missing).
69
70 The maximum length of a pkt-line's data component is 65520 bytes.
71 Implementations MUST NOT send pkt-line whose length exceeds 65524