connect: in ref advertisement, shallows are last

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. As part of this change, this patch interprets capabilities only on the first line in the ref advertisement, printing a warning message when encountering capabilities on other lines. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Sep 26, 2017 at 16:56 UTC 0cd83283dfd2a22b3f95cbee6eaef196e6bde77d
1 file changed +123 -66
connect.c
+123 -66
@@ -11,6 +11,7 @@
11 #include "string-list.h"
12 #include "sha1-array.h"
13 #include "transport.h"
14 +#include "strbuf.h"
15
16 static char *server_capabilities;
17 static const char *parse_feature_value(const char *, const char *, int *);
@@ -107,6 +108,104 @@ static void annotate_refs_with_symref_info(struct ref *ref)
108 string_list_clear(&symref, 0);
109 }
110
111 +/*
112 + * Read one line of a server's ref advertisement into packet_buffer.
113 + */
114 +static int read_remote_ref(int in, char **src_buf, size_t *src_len,
115 + int *responded)
116 +{
117 + int len = packet_read(in, src_buf, src_len,
118 + packet_buffer, sizeof(packet_buffer),
119 + PACKET_READ_GENTLE_ON_EOF |
120 + PACKET_READ_CHOMP_NEWLINE);
121 + const char *arg;
122 + if (len < 0)
123 + die_initial_contact(*responded);
124 + if (len > 4 && skip_prefix(packet_buffer, "ERR ", &arg))
125 + die("remote error: %s", arg);
126 +
127 + *responded = 1;
128 +
129 + return len;
130 +}
131 +
132 +#define EXPECTING_FIRST_REF 0
133 +#define EXPECTING_REF 1
134 +#define EXPECTING_SHALLOW 2
135 +
136 +static void process_capabilities(int *len)
137 +{
138 + int nul_location = strlen(packet_buffer);
139 + if (nul_location == *len)
140 + return;
141 + server_capabilities = xstrdup(packet_buffer + nul_location + 1);
142 + *len = nul_location;
143 +}
144 +
145 +static int process_dummy_ref(void)
146 +{
147 + struct object_id oid;
148 + const char *name;
149 +
150 + if (parse_oid_hex(packet_buffer, &oid, &name))
151 + return 0;
152 + if (*name != ' ')
153 + return 0;
154 + name++;
155 +
156 + return !oidcmp(&null_oid, &oid) && !strcmp(name, "capabilities^{}");
157 +}
158 +
159 +static void check_no_capabilities(int len)
160 +{
161 + if (strlen(packet_buffer) != len)
162 + warning("Ignoring capabilities after first line '%s'",
163 + packet_buffer + strlen(packet_buffer));
164 +}
165 +
166 +static int process_ref(int len, struct ref ***list, unsigned int flags,
167 + struct oid_array *extra_have)
168 +{
169 + struct object_id old_oid;
170 + const char *name;
171 +
172 + if (parse_oid_hex(packet_buffer, &old_oid, &name))
173 + return 0;
174 + if (*name != ' ')
175 + return 0;
176 + name++;
177 +
178 + if (extra_have && !strcmp(name, ".have")) {
179 + oid_array_append(extra_have, &old_oid);
180 + } else if (!strcmp(name, "capabilities^{}")) {
181 + die("protocol error: unexpected capabilities^{}");
182 + } else if (check_ref(name, flags)) {
183 + struct ref *ref = alloc_ref(name);
184 + oidcpy(&ref->old_oid, &old_oid);
185 + **list = ref;
186 + *list = &ref->next;
187 + }
188 + check_no_capabilities(len);
189 + return 1;
190 +}
191 +
192 +static int process_shallow(int len, struct oid_array *shallow_points)
193 +{
194 + const char *arg;
195 + struct object_id old_oid;
196 +
197 + if (!skip_prefix(packet_buffer, "shallow ", &arg))
198 + return 0;
199 +
200 + if (get_oid_hex(arg, &old_oid))
201 + die("protocol error: expected shallow sha-1, got '%s'", arg);
202 + if (!shallow_points)
203 + die("repository on the other end cannot be shallow");
204 + oid_array_append(shallow_points, &old_oid);
205 + check_no_capabilities(len);
206 + return 1;
207 +}
208 +
209 /*
210 * Read all the refs from the other end
211 */
@@ -123,76 +222,34 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
222 * willing to talk to us. A hang-up before seeing any
223 * response does not necessarily mean an ACL problem, though.
224 */
126 - int saw_response;
127 - int got_dummy_ref_with_capabilities_declaration = 0;
225 + int responded = 0;
226 + int len;
227 + int state = EXPECTING_FIRST_REF;
228
229 *list = NULL;
130 - for (saw_response = 0; ; saw_response = 1) {
131 - struct ref *ref;
132 - struct object_id old_oid;
133 - char *name;
134 - int len, name_len;
135 - char *buffer = packet_buffer;
136 - const char *arg;
137 -
138 - len = packet_read(in, &src_buf, &src_len,
139 - packet_buffer, sizeof(packet_buffer),
140 - PACKET_READ_GENTLE_ON_EOF |
141 - PACKET_READ_CHOMP_NEWLINE);
142 - if (len < 0)
143 - die_initial_contact(saw_response);
144 -
145 - if (!len)
146 - break;
147 -
148 - if (len > 4 && skip_prefix(buffer, "ERR ", &arg))
149 - die("remote error: %s", arg);
150 -
151 - if (len == GIT_SHA1_HEXSZ + strlen("shallow ") &&
152 - skip_prefix(buffer, "shallow ", &arg)) {
153 - if (get_oid_hex(arg, &old_oid))
154 - die("protocol error: expected shallow sha-1, got '%s'", arg);
155 - if (!shallow_points)
156 - die("repository on the other end cannot be shallow");
157 - oid_array_append(shallow_points, &old_oid);
158 - continue;
159 - }
230
161 - if (len < GIT_SHA1_HEXSZ + 2 || get_oid_hex(buffer, &old_oid) ||
162 - buffer[GIT_SHA1_HEXSZ] != ' ')
163 - die("protocol error: expected sha/ref, got '%s'", buffer);
164 - name = buffer + GIT_SHA1_HEXSZ + 1;
165 -
166 - name_len = strlen(name);
167 - if (len != name_len + GIT_SHA1_HEXSZ + 1) {
168 - free(server_capabilities);
169 - server_capabilities = xstrdup(name + name_len + 1);
170 - }
171 -
172 - if (extra_have && !strcmp(name, ".have")) {
173 - oid_array_append(extra_have, &old_oid);
174 - continue;
175 - }
176 -
177 - if (!strcmp(name, "capabilities^{}")) {
178 - if (saw_response)
179 - die("protocol error: unexpected capabilities^{}");
180 - if (got_dummy_ref_with_capabilities_declaration)
181 - die("protocol error: multiple capabilities^{}");
182 - got_dummy_ref_with_capabilities_declaration = 1;
183 - continue;
231 + while ((len = read_remote_ref(in, &src_buf, &src_len, &responded))) {
232 + switch (state) {
233 + case EXPECTING_FIRST_REF:
234 + process_capabilities(&len);
235 + if (process_dummy_ref()) {
236 + state = EXPECTING_SHALLOW;
237 + break;
238 + }
239 + state = EXPECTING_REF;
240 + /* fallthrough */
241 + case EXPECTING_REF:
242 + if (process_ref(len, &list, flags, extra_have))
243 + break;
244 + state = EXPECTING_SHALLOW;
245 + /* fallthrough */
246 + case EXPECTING_SHALLOW:
247 + if (process_shallow(len, shallow_points))
248 + break;
249 + die("protocol error: unexpected '%s'", packet_buffer);
250 + default:
251 + die("unexpected state %d", state);
252 }
185 -
186 - if (!check_ref(name, flags))
187 - continue;
188 -
189 - if (got_dummy_ref_with_capabilities_declaration)
190 - die("protocol error: unexpected ref after capabilities^{}");
191 -
192 - ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
193 - oidcpy(&ref->old_oid, &old_oid);
194 - *list = ref;
195 - list = &ref->next;
253 }
254
255 annotate_refs_with_symref_info(*orig_list);