remote-curl: use post_rpc() for protocol v2 also
When transmitting and receiving POSTs for protocol v0 and v1, remote-curl uses post_rpc() (and associated functions), but when doing the same for protocol v2, it uses a separate set of functions (proxy_rpc() and others). Besides duplication of code, this has caused at least one bug: the auth retry mechanism that was implemented in v0/v1 was not implemented in v2. To fix this issue and avoid it in the future, make remote-curl also use post_rpc() when handling protocol v2. Because line lengths are written to the HTTP request in protocol v2 (unlike in protocol v0/v1), this necessitates changes in post_rpc() and some of the functions it uses; perform these changes too. A test has been included to ensure that the code for both the unchunked and chunked variants of the HTTP request is exercised. Note: stateless_connect() has been updated to use the lower-level packet reading functions instead of struct packet_reader. The low-level control is necessary here because we cannot change the destination buffer of struct packet_reader while it is being used; struct packet_buffer has a peeking mechanism which relies on the destination buffer being present in between a peek and a read. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan committed
Feb 21, 2019 at 12:24 UTC
a97d00799a19d2516ca8fae4da9bf782ca6f2e37
4 files changed
+183
-186
pkt-line.c
+1
-1
@@ -117,7 +117,7 @@ void packet_buf_delim(struct strbuf *buf)
117
strbuf_add(buf, "0001", 4);
118
}
119
120
-static void set_packet_header(char *buf, const int size)
120
+void set_packet_header(char *buf, const int size)
121
{
122
static char hexchar[] = "0123456789abcdef";
123
pkt-line.h
+1
@@ -25,6 +25,7 @@ void packet_delim(int fd);
25
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
26
void packet_buf_flush(struct strbuf *buf);
27
void packet_buf_delim(struct strbuf *buf);
28
+void set_packet_header(char *buf, int size);
29
void packet_write(int fd_out, const char *buf, size_t size);
30
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
31
void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len);
remote-curl.c
+149
-184
@@ -518,6 +518,21 @@ struct rpc_state {
518
int any_written;
519
unsigned gzip_request : 1;
520
unsigned initial_buffer : 1;
521
+
522
+ /*
523
+ * Whenever a pkt-line is read into buf, append the 4 characters
524
+ * denoting its length before appending the payload.
525
+ */
526
+ unsigned write_line_lengths : 1;
527
+
528
+ /*
529
+ * Used by rpc_out; initialize to 0. This is true if a flush has been
530
+ * read, but the corresponding line length (if write_line_lengths is
531
+ * true) and EOF have not been sent to libcurl. Since each flush marks
532
+ * the end of a request, each flush must be completely sent before any
533
+ * further reading occurs.
534
+ */
535
+ unsigned flush_read_but_not_sent : 1;
536
};
537
538
/*
@@ -525,17 +540,54 @@ struct rpc_state {
540
* rpc->buf and rpc->len if there is enough space. Returns 1 if there was
541
* enough space, 0 otherwise.
542
*
528
- * Writes the number of bytes appended into appended.
543
+ * If rpc->write_line_lengths is true, appends the line length as a 4-byte
544
+ * hexadecimal string before appending the result described above.
545
+ *
546
+ * Writes the total number of bytes appended into appended.
547
*/
530
-static int rpc_read_from_out(struct rpc_state *rpc, size_t *appended) {
531
- size_t left = rpc->alloc - rpc->len;
532
- char *buf = rpc->buf + rpc->len;
548
+static int rpc_read_from_out(struct rpc_state *rpc, int options,
549
+ size_t *appended,
550
+ enum packet_read_status *status) {
551
+ size_t left;
552
+ char *buf;
553
+ int pktlen_raw;
554
+
555
+ if (rpc->write_line_lengths) {
556
+ left = rpc->alloc - rpc->len - 4;
557
+ buf = rpc->buf + rpc->len + 4;
558
+ } else {
559
+ left = rpc->alloc - rpc->len;
560
+ buf = rpc->buf + rpc->len;
561
+ }
562
563
if (left < LARGE_PACKET_MAX)
564
return 0;
565
537
- *appended = packet_read(rpc->out, NULL, NULL, buf, left, 0);
538
- rpc->len += *appended;
566
+ *status = packet_read_with_status(rpc->out, NULL, NULL, buf,
567
+ left, &pktlen_raw, options);
568
+ if (*status != PACKET_READ_EOF) {
569
+ *appended = pktlen_raw + (rpc->write_line_lengths ? 4 : 0);
570
+ rpc->len += *appended;
571
+ }
572
+
573
+ if (rpc->write_line_lengths) {
574
+ switch (*status) {
575
+ case PACKET_READ_EOF:
576
+ if (!(options & PACKET_READ_GENTLE_ON_EOF))
577
+ die("shouldn't have EOF when not gentle on EOF");
578
+ break;
579
+ case PACKET_READ_NORMAL:
580
+ set_packet_header(buf - 4, *appended);
581
+ break;
582
+ case PACKET_READ_DELIM:
583
+ memcpy(buf - 4, "0001", 4);
584
+ break;
585
+ case PACKET_READ_FLUSH:
586
+ memcpy(buf - 4, "0000", 4);
587
+ break;
588
+ }
589
+ }
590
+
591
return 1;
592
}
593
@@ -545,15 +597,40 @@ static size_t rpc_out(void *ptr, size_t eltsize,
597
size_t max = eltsize * nmemb;
598
struct rpc_state *rpc = buffer_;
599
size_t avail = rpc->len - rpc->pos;
600
+ enum packet_read_status status;
601
602
if (!avail) {
603
rpc->initial_buffer = 0;
604
rpc->len = 0;
552
- if (!rpc_read_from_out(rpc, &avail))
553
- BUG("The entire rpc->buf should be larger than LARGE_PACKET_DATA_MAX");
554
- if (!avail)
555
- return 0;
605
rpc->pos = 0;
606
+ if (!rpc->flush_read_but_not_sent) {
607
+ if (!rpc_read_from_out(rpc, 0, &avail, &status))
608
+ BUG("The entire rpc->buf should be larger than LARGE_PACKET_MAX");
609
+ if (status == PACKET_READ_FLUSH)
610
+ rpc->flush_read_but_not_sent = 1;
611
+ }
612
+ /*
613
+ * If flush_read_but_not_sent is true, we have already read one
614
+ * full request but have not fully sent it + EOF, which is why
615
+ * we need to refrain from reading.
616
+ */
617
+ }
618
+ if (rpc->flush_read_but_not_sent) {
619
+ if (!avail) {
620
+ /*
621
+ * The line length either does not need to be sent at
622
+ * all or has already been completely sent. Now we can
623
+ * return 0, indicating EOF, meaning that the flush has
624
+ * been fully sent.
625
+ */
626
+ rpc->flush_read_but_not_sent = 0;
627
+ return 0;
628
+ }
629
+ /*
630
+ * If avail is non-zerp, the line length for the flush still
631
+ * hasn't been fully sent. Proceed with sending the line
632
+ * length.
633
+ */
634
}
635
636
if (max < avail)
@@ -681,7 +758,11 @@ static curl_off_t xcurl_off_t(size_t len)
758
return (curl_off_t)size;
759
}
760
684
-static int post_rpc(struct rpc_state *rpc)
761
+/*
762
+ * If flush_received is true, do not attempt to read any more; just use what's
763
+ * in rpc->buf.
764
+ */
765
+static int post_rpc(struct rpc_state *rpc, int flush_received)
766
{
767
struct active_request_slot *slot;
768
struct curl_slist *headers = http_copy_default_headers();
@@ -696,16 +777,19 @@ static int post_rpc(struct rpc_state *rpc)
777
* allocated buffer space we can use HTTP/1.0 and avoid the
778
* chunked encoding mess.
779
*/
699
- while (1) {
700
- size_t n;
701
-
702
- if (!rpc_read_from_out(rpc, &n)) {
703
- large_request = 1;
704
- use_gzip = 0;
705
- break;
780
+ if (!flush_received) {
781
+ while (1) {
782
+ size_t n;
783
+ enum packet_read_status status;
784
+
785
+ if (!rpc_read_from_out(rpc, 0, &n, &status)) {
786
+ large_request = 1;
787
+ use_gzip = 0;
788
+ break;
789
+ }
790
+ if (status == PACKET_READ_FLUSH)
791
+ break;
792
}
707
- if (!n)
708
- break;
793
}
794
795
if (large_request) {
@@ -885,7 +969,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads,
969
break;
970
rpc->pos = 0;
971
rpc->len = n;
888
- err |= post_rpc(rpc);
972
+ err |= post_rpc(rpc, 0);
973
}
974
975
close(client.in);
@@ -1179,165 +1263,11 @@ static void parse_push(struct strbuf *buf)
1263
free(specs);
1264
}
1265
1182
-/*
1183
- * Used to represent the state of a connection to an HTTP server when
1184
- * communicating using git's wire-protocol version 2.
1185
- */
1186
-struct proxy_state {
1187
- char *service_name;
1188
- char *service_url;
1189
- struct curl_slist *headers;
1190
- struct strbuf request_buffer;
1191
- int in;
1192
- int out;
1193
- struct packet_reader reader;
1194
- size_t pos;
1195
- int seen_flush;
1196
-};
1197
-
1198
-static void proxy_state_init(struct proxy_state *p, const char *service_name,
1199
- enum protocol_version version)
1200
-{
1201
- struct strbuf buf = STRBUF_INIT;
1202
-
1203
- memset(p, 0, sizeof(*p));
1204
- p->service_name = xstrdup(service_name);
1205
-
1206
- p->in = 0;
1207
- p->out = 1;
1208
- strbuf_init(&p->request_buffer, 0);
1209
-
1210
- strbuf_addf(&buf, "%s%s", url.buf, p->service_name);
1211
- p->service_url = strbuf_detach(&buf, NULL);
1212
-
1213
- p->headers = http_copy_default_headers();
1214
-
1215
- strbuf_addf(&buf, "Content-Type: application/x-%s-request", p->service_name);
1216
- p->headers = curl_slist_append(p->headers, buf.buf);
1217
- strbuf_reset(&buf);
1218
-
1219
- strbuf_addf(&buf, "Accept: application/x-%s-result", p->service_name);
1220
- p->headers = curl_slist_append(p->headers, buf.buf);
1221
- strbuf_reset(&buf);
1222
-
1223
- p->headers = curl_slist_append(p->headers, "Transfer-Encoding: chunked");
1224
-
1225
- /* Add the Git-Protocol header */
1226
- if (get_protocol_http_header(version, &buf))
1227
- p->headers = curl_slist_append(p->headers, buf.buf);
1228
-
1229
- packet_reader_init(&p->reader, p->in, NULL, 0,
1230
- PACKET_READ_GENTLE_ON_EOF |
1231
- PACKET_READ_DIE_ON_ERR_PACKET);
1232
-
1233
- strbuf_release(&buf);
1234
-}
1235
-
1236
-static void proxy_state_clear(struct proxy_state *p)
1237
-{
1238
- free(p->service_name);
1239
- free(p->service_url);
1240
- curl_slist_free_all(p->headers);
1241
- strbuf_release(&p->request_buffer);
1242
-}
1243
-
1244
-/*
1245
- * CURLOPT_READFUNCTION callback function.
1246
- * Attempts to copy over a single packet-line at a time into the
1247
- * curl provided buffer.
1248
- */
1249
-static size_t proxy_in(char *buffer, size_t eltsize,
1250
- size_t nmemb, void *userdata)
1251
-{
1252
- size_t max;
1253
- struct proxy_state *p = userdata;
1254
- size_t avail = p->request_buffer.len - p->pos;
1255
-
1256
-
1257
- if (eltsize != 1)
1258
- BUG("curl read callback called with size = %"PRIuMAX" != 1",
1259
- (uintmax_t)eltsize);
1260
- max = nmemb;
1261
-
1262
- if (!avail) {
1263
- if (p->seen_flush) {
1264
- p->seen_flush = 0;
1265
- return 0;
1266
- }
1267
-
1268
- strbuf_reset(&p->request_buffer);
1269
- switch (packet_reader_read(&p->reader)) {
1270
- case PACKET_READ_EOF:
1271
- die("unexpected EOF when reading from parent process");
1272
- case PACKET_READ_NORMAL:
1273
- packet_buf_write_len(&p->request_buffer, p->reader.line,
1274
- p->reader.pktlen);
1275
- break;
1276
- case PACKET_READ_DELIM:
1277
- packet_buf_delim(&p->request_buffer);
1278
- break;
1279
- case PACKET_READ_FLUSH:
1280
- packet_buf_flush(&p->request_buffer);
1281
- p->seen_flush = 1;
1282
- break;
1283
- }
1284
- p->pos = 0;
1285
- avail = p->request_buffer.len;
1286
- }
1287
-
1288
- if (max < avail)
1289
- avail = max;
1290
- memcpy(buffer, p->request_buffer.buf + p->pos, avail);
1291
- p->pos += avail;
1292
- return avail;
1293
-}
1294
-
1295
-static size_t proxy_out(char *buffer, size_t eltsize,
1296
- size_t nmemb, void *userdata)
1297
-{
1298
- size_t size;
1299
- struct proxy_state *p = userdata;
1300
-
1301
- if (eltsize != 1)
1302
- BUG("curl read callback called with size = %"PRIuMAX" != 1",
1303
- (uintmax_t)eltsize);
1304
- size = nmemb;
1305
-
1306
- write_or_die(p->out, buffer, size);
1307
- return size;
1308
-}
1309
-
1310
-/* Issues a request to the HTTP server configured in `p` */
1311
-static int proxy_request(struct proxy_state *p)
1312
-{
1313
- struct active_request_slot *slot;
1314
-
1315
- slot = get_active_slot();
1316
-
1317
- curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
1318
- curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
1319
- curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
1320
- curl_easy_setopt(slot->curl, CURLOPT_URL, p->service_url);
1321
- curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, p->headers);
1322
-
1323
- /* Setup function to read request from client */
1324
- curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, proxy_in);
1325
- curl_easy_setopt(slot->curl, CURLOPT_READDATA, p);
1326
-
1327
- /* Setup function to write server response to client */
1328
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, proxy_out);
1329
- curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, p);
1330
-
1331
- if (run_slot(slot, NULL) != HTTP_OK)
1332
- return -1;
1333
-
1334
- return 0;
1335
-}
1336
-
1266
static int stateless_connect(const char *service_name)
1267
{
1268
struct discovery *discover;
1340
- struct proxy_state p;
1269
+ struct rpc_state rpc;
1270
+ struct strbuf buf = STRBUF_INIT;
1271
1272
/*
1273
* Run the info/refs request and see if the server supports protocol
@@ -1357,23 +1287,58 @@ static int stateless_connect(const char *service_name)
1287
fflush(stdout);
1288
}
1289
1360
- proxy_state_init(&p, service_name, discover->version);
1290
+ rpc.service_name = service_name;
1291
+ rpc.service_url = xstrfmt("%s%s", url.buf, rpc.service_name);
1292
+ rpc.hdr_content_type = xstrfmt("Content-Type: application/x-%s-request", rpc.service_name);
1293
+ rpc.hdr_accept = xstrfmt("Accept: application/x-%s-result", rpc.service_name);
1294
+ if (get_protocol_http_header(discover->version, &buf)) {
1295
+ rpc.protocol_header = strbuf_detach(&buf, NULL);
1296
+ } else {
1297
+ rpc.protocol_header = NULL;
1298
+ strbuf_release(&buf);
1299
+ }
1300
+ rpc.buf = xmalloc(http_post_buffer);
1301
+ rpc.alloc = http_post_buffer;
1302
+ rpc.len = 0;
1303
+ rpc.pos = 0;
1304
+ rpc.in = 1;
1305
+ rpc.out = 0;
1306
+ rpc.any_written = 0;
1307
+ rpc.gzip_request = 1;
1308
+ rpc.initial_buffer = 0;
1309
+ rpc.write_line_lengths = 1;
1310
+ rpc.flush_read_but_not_sent = 0;
1311
1312
/*
1313
* Dump the capability listing that we got from the server earlier
1314
* during the info/refs request.
1315
*/
1366
- write_or_die(p.out, discover->buf, discover->len);
1316
+ write_or_die(rpc.in, discover->buf, discover->len);
1317
+
1318
+ /* Until we see EOF keep sending POSTs */
1319
+ while (1) {
1320
+ size_t avail;
1321
+ enum packet_read_status status;
1322
1368
- /* Peek the next packet line. Until we see EOF keep sending POSTs */
1369
- while (packet_reader_peek(&p.reader) != PACKET_READ_EOF) {
1370
- if (proxy_request(&p)) {
1323
+ if (!rpc_read_from_out(&rpc, PACKET_READ_GENTLE_ON_EOF, &avail,
1324
+ &status))
1325
+ BUG("The entire rpc->buf should be larger than LARGE_PACKET_MAX");
1326
+ if (status == PACKET_READ_EOF)
1327
+ break;
1328
+ if (post_rpc(&rpc, status == PACKET_READ_FLUSH))
1329
/* We would have an err here */
1330
break;
1373
- }
1331
+ /* Reset the buffer for next request */
1332
+ rpc.len = 0;
1333
}
1334
1376
- proxy_state_clear(&p);
1335
+ free(rpc.service_url);
1336
+ free(rpc.hdr_content_type);
1337
+ free(rpc.hdr_accept);
1338
+ free(rpc.protocol_header);
1339
+ free(rpc.buf);
1340
+ strbuf_release(&buf);
1341
+
1342
return 0;
1343
}
1344
t/t5702-protocol-v2.sh
+32
-1
@@ -542,7 +542,38 @@ test_expect_success 'clone with http:// using protocol v2' '
542
# Client requested to use protocol v2
543
grep "Git-Protocol: version=2" log &&
544
# Server responded using protocol v2
545
- grep "git< version 2" log
545
+ grep "git< version 2" log &&
546
+ # Verify that the chunked encoding sending codepath is NOT exercised
547
+ ! grep "Send header: Transfer-Encoding: chunked" log
548
+'
549
+
550
+test_expect_success 'clone big repository with http:// using protocol v2' '
551
+ test_when_finished "rm -f log" &&
552
+
553
+ git init "$HTTPD_DOCUMENT_ROOT_PATH/big" &&
554
+ # Ensure that the list of wants is greater than http.postbuffer below
555
+ for i in $(test_seq 1 1500)
556
+ do
557
+ # do not use here-doc, because it requires a process
558
+ # per loop iteration
559
+ echo "commit refs/heads/too-many-refs-$i" &&
560
+ echo "committer git <git@example.com> $i +0000" &&
561
+ echo "data 0" &&
562
+ echo "M 644 inline bla.txt" &&
563
+ echo "data 4" &&
564
+ echo "bla"
565
+ done | git -C "$HTTPD_DOCUMENT_ROOT_PATH/big" fast-import &&
566
+
567
+ GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git \
568
+ -c protocol.version=2 -c http.postbuffer=65536 \
569
+ clone "$HTTPD_URL/smart/big" big_child &&
570
+
571
+ # Client requested to use protocol v2
572
+ grep "Git-Protocol: version=2" log &&
573
+ # Server responded using protocol v2
574
+ grep "git< version 2" log &&
575
+ # Verify that the chunked encoding sending codepath is exercised
576
+ grep "Send header: Transfer-Encoding: chunked" log
577
'
578
579
test_expect_success 'fetch with http:// using protocol v2' '