Currently, get_remote_heads() parses the ref advertisement in one loop,
allowing refs and shallow lines to intersperse, despite this not being
allowed by the specification. Refactor get_remote_heads() to use two
loops instead, enforcing that refs come first, and then shallows.
This also makes it easier to teach get_remote_heads() to interpret other
lines in the ref advertisement, which will be done in a subsequent
patch.
Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
---
It seems that some people are concerned about looseness in interpreting
the ref advertisement, so here is a patch to tighten it instead. This is
a replacement for Brandon's PATCH 1.5.
I think this is what Jonathan Nieder meant by his instruction flow idea.
I've checked that Brandon's other patches apply cleanly on this patch,
except for "connect: teach client to recognize v1 server response" which
has to be modified to the following:
@@ -149,6 +150,26 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
*list = NULL;
len = read_remote_ref(in, &src_buf, &src_len, &responded);
+
+ switch (determine_protocol_version_client(packet_buffer)) {
+ case protocol_v1:
+ /*
+ * First pkt-line contained the version string.
+ * Continue on to process the ref advertisement.
+ */
+ len = read_remote_ref(in, &src_buf, &src_len, &responded);
+ break;
+ case protocol_v0:
+ /*
+ * Server is speaking protocol v0 and sent a
+ * ref so we need to process it.
+ */
+ break;
+ default:
+ die("server is speaking an unknown protocol");
+ break;
+ }
+
connect.c | 112 ++++++++++++++++++++++++++++++++++----------------------------
1 file changed, 61 insertions(+), 51 deletions(-)
diff --git a/connect.c b/connect.c
index 49b28b83b..9bf97adf6 100644
--- a/connect.c
+++ b/connect.c
@@ -107,6 +107,26 @@ static void annotate_refs_with_symref_info(struct ref *ref)
string_list_clear(&symref, 0);
}
+/*
+ * Read one line of a server's ref advertisement into packet_buffer.
+ */
+int read_remote_ref(int in, char **src_buf, size_t *src_len, int *responded)
+{
+ int len = packet_read(in, src_buf, src_len,
+ packet_buffer, sizeof(packet_buffer),
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_CHOMP_NEWLINE);
+ const char *arg;
+ if (len < 0)
+ die_initial_contact(*responded);
+ if (len > 4 && skip_prefix(packet_buffer, "ERR ", &arg))
+ die("remote error: %s", arg);
+
+ *responded = 1;
+
+ return len;
+}
+
/*
* Read all the refs from the other end
*/
@@ -123,46 +143,23 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
* willing to talk to us. A hang-up before seeing any
* response does not necessarily mean an ACL problem, though.
*/
- int saw_response;
- int got_dummy_ref_with_capabilities_declaration = 0;
+ int responded = 0;
+ int len;
*list = NULL;
- for (saw_response = 0; ; saw_response = 1) {
- struct ref *ref;
+
+ len = read_remote_ref(in, &src_buf, &src_len, &responded);
+ do {
struct object_id old_oid;
char *name;
- int len, name_len;
- char *buffer = packet_buffer;
- const char *arg;
+ int name_len;
- len = packet_read(in, &src_buf, &src_len,
- packet_buffer, sizeof(packet_buffer),
- PACKET_READ_GENTLE_ON_EOF |
- PACKET_READ_CHOMP_NEWLINE);
- if (len < 0)
- die_initial_contact(saw_response);
-
- if (!len)
+ if (len < GIT_SHA1_HEXSZ + 2 ||
+ get_oid_hex(packet_buffer, &old_oid) ||
+ packet_buffer[GIT_SHA1_HEXSZ] != ' ')
break;
- if (len > 4 && skip_prefix(buffer, "ERR ", &arg))
- die("remote error: %s", arg);
-
- if (len == GIT_SHA1_HEXSZ + strlen("shallow ") &&
- skip_prefix(buffer, "shallow ", &arg)) {
- if (get_oid_hex(arg, &old_oid))
- die("protocol error: expected shallow sha-1, got '%s'", arg);
- if (!shallow_points)
- die("repository on the other end cannot be shallow");
- oid_array_append(shallow_points, &old_oid);
- continue;
- }
-
- if (len < GIT_SHA1_HEXSZ + 2 || get_oid_hex(buffer, &old_oid) ||
- buffer[GIT_SHA1_HEXSZ] != ' ')
- die("protocol error: expected sha/ref, got '%s'", buffer);
- name = buffer + GIT_SHA1_HEXSZ + 1;
-
+ name = packet_buffer + GIT_SHA1_HEXSZ + 1;
name_len = strlen(name);
if (len != name_len + GIT_SHA1_HEXSZ + 1) {
free(server_capabilities);
@@ -171,29 +168,42 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
if (extra_have && !strcmp(name, ".have")) {
oid_array_append(extra_have, &old_oid);
- continue;
- }
-
- if (!strcmp(name, "capabilities^{}")) {
- if (saw_response)
+ } else if (!strcmp(name, "capabilities^{}")) {
+ if (*list)
+ /* cannot coexist with other refs */
die("protocol error: unexpected capabilities^{}");
- if (got_dummy_ref_with_capabilities_declaration)
- die("protocol error: multiple capabilities^{}");
- got_dummy_ref_with_capabilities_declaration = 1;
- continue;
+ /*
+ * There should be no more refs; read the next line and
+ * go to next block.
+ */
+ len = read_remote_ref(in, &src_buf, &src_len,
+ &responded);
+ break;
+ } else if (check_ref(name, flags)) {
+ struct ref *ref = alloc_ref(name);
+ oidcpy(&ref->old_oid, &old_oid);
+ *list = ref;
+ list = &ref->next;
}
+ } while ((len = read_remote_ref(in, &src_buf, &src_len, &responded)));
- if (!check_ref(name, flags))
- continue;
+ do {
+ const char *arg;
+ struct object_id old_oid;
- if (got_dummy_ref_with_capabilities_declaration)
- die("protocol error: unexpected ref after capabilities^{}");
+ if (skip_prefix(packet_buffer, "shallow ", &arg)) {
+ if (get_oid_hex(arg, &old_oid))
+ die("protocol error: expected shallow sha-1, got '%s'", arg);
+ if (!shallow_points)
+ die("repository on the other end cannot be shallow");
+ oid_array_append(shallow_points, &old_oid);
+ } else {
+ break;
+ }
+ } while ((len = read_remote_ref(in, &src_buf, &src_len, &responded)));
- ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
- oidcpy(&ref->old_oid, &old_oid);
- *list = ref;
- list = &ref->next;
- }
+ if (len)
+ die("protocol error: unexpected '%s'", packet_buffer);
annotate_refs_with_symref_info(*orig_list);
--
2.14.1.728.g20a5b67d5.dirty
