Re: bug in HTTP protocol spec
> On Feb 22, 2018, at 12:02 PM, Junio C Hamano <gits...@pobox.com> wrote: > > I saw somewhere "Apple-Mail" and a phrase "repaste". So perhaps > copy on the client is involved in the whitespace damage (of > course the original could be broken, but I somehow doubt it). https://doriantaylor.com/file/well-ill-be-damned.png > For what it's worth, I am slightly negative on this addition. It > makes it inconsistent if we only mention it about _this_ flush and > not any other flush. It even gets in the way of learning the > protocol by new people reading it, by giving an impression that > somehow LF is outside and in between packet line. > > Thanks. This patch exists because I was asked to write it. I don’t know squat about this protocol other than the fact that I followed the spec and it didn’t work. I traced a known-good protocol endpoint and found it contained content that didn’t agree with the spec. I then obliged the request to submit a patch with *what I knew to be true* about the sample that actually worked. I then followed the recommendations *advertised on GitHub* for submitting the patch. You’re welcome. -- Dorian Taylor Make things. Make sense. https://doriantaylor.com signature.asc Description: Message signed with OpenPGP using GPGMail
Re: bug in HTTP protocol spec
> On Feb 22, 2018, at 2:08 AM, Jeff King <p...@peff.net> wrote: > > > This indentation is funny. But I suspect it is because your whole patch > seems to have been whitespace-damaged (see the section on gmail in > "git help git-format-patch"). That is, bit-for-bit, what came out of that submitGit thing that is advertised when you try to open a pull request on the git repository on Github. I was a bit surprised myself. -- Dorian Taylor Make things. Make sense. https://doriantaylor.com signature.asc Description: Message signed with OpenPGP using GPGMail
Re: bug in HTTP protocol spec
> On Feb 21, 2018, at 9:37 PM, Jonathan Nieder <jrnie...@gmail.com> wrote:
>
> Thanks for writing it.
>
> Do you mind if we forge your sign-off? (See Documentation/SubmittingPatches
> item '(5) Certify your work' for details about what this means.)
Sure, or I can just re-paste:
Signed-off-by: Dorian Taylor <dorian.taylor.li...@gmail.com>
---
Documentation/technical/http-protocol.txt | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/http-protocol.txt
b/Documentation/technical/http-protocol.txt
index a0e45f2889e6e..19d73f7efb338 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -214,14 +214,17 @@ smart server reply:
S: Cache-Control: no-cache
S:
S: 001e# service=git-upload-pack\n
+ S:
S: 004895dcfa3633004da0049d3d0fa03f80589cbcaf31 refs/heads/maint\0multi_ack\n
S: 0042d049f6c27a2244e12041955e262a404c7faba355 refs/heads/master\n
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
+ S:
The client may send Extra Parameters (see
Documentation/technical/pack-protocol.txt) as a colon-separated string
-in the Git-Protocol HTTP header.
+in the Git-Protocol HTTP header. Note as well that there is *no* newline
+after the ``.
Dumb Server Response
@@ -264,8 +267,8 @@ Servers MUST set $servicename to be the request parameter
value.
Servers SHOULD include an LF at the end of this line.
Clients MUST ignore an LF at the end of the line.
-Servers MUST terminate the response with the magic `` end
-pkt-line marker.
+Servers MUST follow the first pkt-line, as well as terminate the
+response, with the magic `` end pkt-line marker.
The returned response is a pkt-line stream describing each ref and
its known value. The stream SHOULD be sorted by name according to
@@ -278,6 +281,7 @@ Extra Parameter.
smart_reply = PKT-LINE("# service=$servicename" LF)
*1("version 1")
+""
ref_list
""
ref_list= empty_list / non_empty_list
---
>
>> Note I am not sure what the story is behind that `version 1`
>> element, whether it's supposed to go before or after the null packet
>> or if there should be another null packet or what. Perhaps somebody
>> more fluent with the smart protocol can advise.
>
> I believe the 'version 1' goes after the flush-packet.
I took a traipse through the code and couldn’t determine it one way or another,
but my money is on that looking something like `000aversion 1\n` on the wire.
--
Dorian Taylor
Make things. Make sense.
https://doriantaylor.com
signature.asc
Description: Message signed with OpenPGP using GPGMail
Re: bug in HTTP protocol spec
> On Feb 21, 2018, at 2:15 PM, Jeff King <p...@peff.net> wrote:
>
> Thanks, I agree the document is buggy. Do you want to submit a patch?
Will this do?
Note I am not sure what the story is behind that `version 1` element, whether
it's supposed to go before or after the null packet or if there should be
another null packet or what. Perhaps somebody more fluent with the smart
protocol can advise.
---
Documentation/technical/http-protocol.txt | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/http-protocol.txt
b/Documentation/technical/http-protocol.txt
index a0e45f2889e6e..19d73f7efb338 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -214,14 +214,17 @@ smart server reply:
S: Cache-Control: no-cache
S:
S: 001e# service=git-upload-pack\n
+ S:
S: 004895dcfa3633004da0049d3d0fa03f80589cbcaf31 refs/heads/maint\0multi_ack\n
S: 0042d049f6c27a2244e12041955e262a404c7faba355 refs/heads/master\n
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
+ S:
The client may send Extra Parameters (see
Documentation/technical/pack-protocol.txt) as a colon-separated string
-in the Git-Protocol HTTP header.
+in the Git-Protocol HTTP header. Note as well that there is *no* newline
+after the ``.
Dumb Server Response
@@ -264,8 +267,8 @@ Servers MUST set $servicename to be the request parameter
value.
Servers SHOULD include an LF at the end of this line.
Clients MUST ignore an LF at the end of the line.
-Servers MUST terminate the response with the magic `` end
-pkt-line marker.
+Servers MUST follow the first pkt-line, as well as terminate the
+response, with the magic `` end pkt-line marker.
The returned response is a pkt-line stream describing each ref and
its known value. The stream SHOULD be sorted by name according to
@@ -278,6 +281,7 @@ Extra Parameter.
smart_reply = PKT-LINE("# service=$servicename" LF)
*1("version 1")
+""
ref_list
""
ref_list = empty_list / non_empty_list
--
Dorian Taylor
Make things. Make sense.
https://doriantaylor.com
bug in HTTP protocol spec
Hello list, I had been banging my head all morning trying to figure out why I couldn’t get a little HTTP implementation to clone/push via the smart protocol (just wrapping git-receive-pack/git-upload-pack). I kept getting the following (likely familiar to some) error: ``` fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. ``` I didn’t get an insight until I ran with GIT_TRACE_PACKET=true on a known-good remote (i.e. GitHub), that the null packet-line `` has to follow the service line. This is not reflected in the example here: https://github.com/git/git/blob/6464679d9620d91b639e2681b9cc6473f3856d09/Documentation/technical/http-protocol.txt#L216 It is also not reflected in the BNF: https://github.com/git/git/blob/6464679d9620d91b639e2681b9cc6473f3856d09/Documentation/technical/http-protocol.txt#L279 (Note these links are from the most recent commit of this file as of this writing.) Just thought somebody would like to know. Regards, -- Dorian Taylor Make things. Make sense. https://doriantaylor.com
