Re: [PATCH v4 3/3] connect: advertized capability is not a ref
From: Junio C Hamano <hidden>
Date: 2016-09-09 20:10:11
Jonathan Tan [off-list ref] writes:
Git advertises the same capabilities^{} ref in its ref advertisement for push
but since it never did so for fetch, the client didn't need to handle this
case. Handle it.
In this aspect, JGit is compliant with the specification in pack-protocol.txt.The last sentence somehow looks out of place.
quoted hunk
+ int got_dummy_ref_with_capabilities_declaration = 0; *list = NULL; for (saw_response = 0; ; saw_response = 1) {@@ -172,8 +173,24 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, continue; } + if (!strcmp(name, "capabilities^{}")) { + if (saw_response) + warning("protocol error: unexpected capabilities^{}, " + "continuing anyway");
OK. saw_response tells us that we saw ".have", a valid ref, or "shallow",
but "capabilities^{}" should happen before any of them, so that is a
protocol violation. Makes perfect sense.
+ if (got_dummy_ref_with_capabilities_declaration)
+ warning("protocol error: multiple capabilities^{}, "
+ "continuing anyway");
+ got_dummy_ref_with_capabilities_declaration = 1;
+ continue;
+ }
+
if (!check_ref(name, flags))
continue;
+
+ if (got_dummy_ref_with_capabilities_declaration)
+ warning("protocol error: unexpected ref after capabilities^{}, "
+ "using this ref and continuing anyway");
Likewise. "capabilities^{}" is used when we cannot piggyback the
capability list after a real ref, so it is unusual to see a real ref
after seeing one. Makes perfect sense.
Do we want to abort the connection in these cases, I wonder, though?
Thanks.