diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f48a031dc3..93baeeb029 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -405,7 +405,9 @@ Supported if the helper has the "connect" capability. trying to fall back). After line feed terminating the positive (empty) response, the output of the service starts. Messages (both request and response) must consist of zero or more - PKT-LINEs, terminating in a flush packet. The client must not + PKT-LINEs, terminating in a flush packet. Response messages will + then have a response end packet after the flush packet to + indicate the end of a response. The client must not expect the server to store any state in between request-response pairs. After the connection ends, the remote helper exits. + diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index 7e3766cafb..3996d70891 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -33,6 +33,8 @@ In protocol v2 these special packets will have the following semantics: * '0000' Flush Packet (flush-pkt) - indicates the end of a message * '0001' Delimiter Packet (delim-pkt) - separates sections of a message + * '0002' Message Packet (response-end-pkt) - indicates the end of a response + for stateless connections Initial Client Request ---------------------- diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 4771100072..94b0c89b82 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -224,7 +224,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) version = discover_version(&reader); switch (version) { case protocol_v2: - get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL); + get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL, args.stateless_rpc); break; case protocol_v1: case protocol_v0: diff --git a/connect.c b/connect.c index 23013c6344..0df45a1108 100644 --- a/connect.c +++ b/connect.c @@ -127,6 +127,7 @@ enum protocol_version discover_version(struct packet_reader *reader) die_initial_contact(0); case PACKET_READ_FLUSH: case PACKET_READ_DELIM: + case PACKET_READ_RESPONSE_END: version = protocol_v0; break; case PACKET_READ_NORMAL: @@ -310,6 +311,7 @@ struct ref **get_remote_heads(struct packet_reader *reader, state = EXPECTING_DONE; break; case PACKET_READ_DELIM: + case PACKET_READ_RESPONSE_END: die(_("invalid packet")); } @@ -404,10 +406,21 @@ out: return ret; } +void check_stateless_delimiter(int stateless_rpc, + struct packet_reader *reader, + const char *error) +{ + if (!stateless_rpc) + return; /* not in stateless mode, no delimiter expected */ + if (packet_reader_read(reader) != PACKET_READ_RESPONSE_END) + die("%s", error); +} + struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, struct ref **list, int for_push, const struct argv_array *ref_prefixes, - const struct string_list *server_options) + const struct string_list *server_options, + int stateless_rpc) { int i; *list = NULL; @@ -444,6 +457,9 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, if (reader->status != PACKET_READ_FLUSH) die(_("expected flush after ref listing")); + check_stateless_delimiter(stateless_rpc, reader, + _("expected response end packet after ref listing")); + return list; } diff --git a/connect.h b/connect.h index 5f2382e018..235bc66254 100644 --- a/connect.h +++ b/connect.h @@ -22,4 +22,8 @@ int server_supports_v2(const char *c, int die_on_error); int server_supports_feature(const char *c, const char *feature, int die_on_error); +void check_stateless_delimiter(int stateless_rpc, + struct packet_reader *reader, + const char *error); + #endif diff --git a/fetch-pack.c b/fetch-pack.c index 7eaa19d7c1..d8bbf45ee2 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1451,6 +1451,13 @@ enum fetch_state { FETCH_DONE, }; +static void do_check_stateless_delimiter(const struct fetch_pack_args *args, + struct packet_reader *reader) +{ + check_stateless_delimiter(args->stateless_rpc, reader, + _("git fetch-pack: expected response end packet")); +} + static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, int fd[2], const struct ref *orig_ref, @@ -1535,6 +1542,10 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, /* Process ACKs/NAKs */ switch (process_acks(negotiator, &reader, &common)) { case READY: + /* + * Don't check for response delimiter; get_pack() will + * read the rest of this response. + */ state = FETCH_GET_PACK; break; case COMMON_FOUND: @@ -1542,6 +1553,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, seen_ack = 1; /* fallthrough */ case NO_COMMON_FOUND: + do_check_stateless_delimiter(args, &reader); state = FETCH_SEND_REQUEST; break; } @@ -1561,6 +1573,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, process_section_header(&reader, "packfile", 0); if (get_pack(args, fd, pack_lockfile, sought, nr_sought)) die(_("git fetch-pack: fetch failed.")); + do_check_stateless_delimiter(args, &reader); state = FETCH_DONE; break; diff --git a/pkt-line.c b/pkt-line.c index a0e87b1e81..8f9bc68ee2 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -99,6 +99,13 @@ void packet_delim(int fd) die_errno(_("unable to write delim packet")); } +void packet_response_end(int fd) +{ + packet_trace("0002", 4, 1); + if (write_in_full(fd, "0002", 4) < 0) + die_errno(_("unable to write stateless separator packet")); +} + int packet_flush_gently(int fd) { packet_trace("0000", 4, 1); @@ -306,10 +313,10 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size, return ret; } -static int packet_length(const char *linelen) +int packet_length(const char lenbuf_hex[4]) { - int val = hex2chr(linelen); - return (val < 0) ? val : (val << 8) | hex2chr(linelen + 2); + int val = hex2chr(lenbuf_hex); + return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2); } enum packet_read_status packet_read_with_status(int fd, char **src_buffer, @@ -337,6 +344,10 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer, packet_trace("0001", 4, 0); *pktlen = 0; return PACKET_READ_DELIM; + } else if (len == 2) { + packet_trace("0002", 4, 0); + *pktlen = 0; + return PACKET_READ_RESPONSE_END; } else if (len < 4) { die(_("protocol error: bad line length %d"), len); } diff --git a/pkt-line.h b/pkt-line.h index fef3a0d792..5b373fe4cd 100644 --- a/pkt-line.h +++ b/pkt-line.h @@ -22,6 +22,7 @@ */ void packet_flush(int fd); void packet_delim(int fd); +void packet_response_end(int fd); void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3))); void packet_buf_flush(struct strbuf *buf); void packet_buf_delim(struct strbuf *buf); @@ -74,6 +75,15 @@ int write_packetized_from_buf(const char *src_in, size_t len, int fd_out); int packet_read(int fd, char **src_buffer, size_t *src_len, char *buffer, unsigned size, int options); +/* + * Convert a four hex digit packet line length header into its numeric + * representation. + * + * If lenbuf_hex contains non-hex characters, return -1. Otherwise, return the + * numeric value of the length header. + */ +int packet_length(const char lenbuf_hex[4]); + /* * Read a packetized line into a buffer like the 'packet_read()' function but * returns an 'enum packet_read_status' which indicates the status of the read. @@ -85,6 +95,7 @@ enum packet_read_status { PACKET_READ_NORMAL, PACKET_READ_FLUSH, PACKET_READ_DELIM, + PACKET_READ_RESPONSE_END, }; enum packet_read_status packet_read_with_status(int fd, char **src_buffer, size_t *src_len, char *buffer, diff --git a/remote-curl.c b/remote-curl.c index 1c9aa3d0ab..75532a8bae 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -601,6 +601,8 @@ static int rpc_read_from_out(struct rpc_state *rpc, int options, case PACKET_READ_FLUSH: memcpy(buf - 4, "0000", 4); break; + case PACKET_READ_RESPONSE_END: + die(_("remote server sent stateless separator")); } } @@ -643,7 +645,7 @@ static size_t rpc_out(void *ptr, size_t eltsize, return 0; } /* - * If avail is non-zerp, the line length for the flush still + * If avail is non-zero, the line length for the flush still * hasn't been fully sent. Proceed with sending the line * length. */ @@ -679,9 +681,55 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp) } #endif +struct check_pktline_state { + char len_buf[4]; + int len_filled; + int remaining; +}; + +static void check_pktline(struct check_pktline_state *state, const char *ptr, size_t size) +{ + while (size) { + if (!state->remaining) { + int digits_remaining = 4 - state->len_filled; + if (digits_remaining > size) + digits_remaining = size; + memcpy(&state->len_buf[state->len_filled], ptr, digits_remaining); + state->len_filled += digits_remaining; + ptr += digits_remaining; + size -= digits_remaining; + + if (state->len_filled == 4) { + state->remaining = packet_length(state->len_buf); + if (state->remaining < 0) { + die(_("remote-curl: bad line length character: %.4s"), state->len_buf); + } else if (state->remaining == 2) { + die(_("remote-curl: unexpected response end packet")); + } else if (state->remaining < 4) { + state->remaining = 0; + } else { + state->remaining -= 4; + } + state->len_filled = 0; + } + } + + if (state->remaining) { + int remaining = state->remaining; + if (remaining > size) + remaining = size; + ptr += remaining; + size -= remaining; + state->remaining -= remaining; + } + } +} + struct rpc_in_data { struct rpc_state *rpc; struct active_request_slot *slot; + int check_pktline; + struct check_pktline_state pktline_state; }; /* @@ -702,6 +750,8 @@ static size_t rpc_in(char *ptr, size_t eltsize, return size; if (size) data->rpc->any_written = 1; + if (data->check_pktline) + check_pktline(&data->pktline_state, ptr, size); write_or_die(data->rpc->in, ptr, size); return size; } @@ -778,7 +828,7 @@ static curl_off_t xcurl_off_t(size_t len) * If flush_received is true, do not attempt to read any more; just use what's * in rpc->buf. */ -static int post_rpc(struct rpc_state *rpc, int flush_received) +static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_received) { struct active_request_slot *slot; struct curl_slist *headers = http_copy_default_headers(); @@ -920,6 +970,8 @@ retry: curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in); rpc_in_data.rpc = rpc; rpc_in_data.slot = slot; + rpc_in_data.check_pktline = stateless_connect; + memset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state)); curl_easy_setopt(slot->curl, CURLOPT_FILE, &rpc_in_data); curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0); @@ -936,6 +988,14 @@ retry: if (!rpc->any_written) err = -1; + if (rpc_in_data.pktline_state.len_filled) + err = error(_("%d bytes of length header were received"), rpc_in_data.pktline_state.len_filled); + if (rpc_in_data.pktline_state.remaining) + err = error(_("%d bytes of body are still expected"), rpc_in_data.pktline_state.remaining); + + if (stateless_connect) + packet_response_end(rpc->in); + curl_slist_free_all(headers); free(gzip_body); return err; @@ -985,7 +1045,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, break; rpc->pos = 0; rpc->len = n; - err |= post_rpc(rpc, 0); + err |= post_rpc(rpc, 0, 0); } close(client.in); @@ -1276,7 +1336,7 @@ static void parse_push(struct strbuf *buf) if (ret) exit(128); /* error already reported */ - free_specs: +free_specs: argv_array_clear(&specs); } @@ -1342,7 +1402,7 @@ static int stateless_connect(const char *service_name) BUG("The entire rpc->buf should be larger than LARGE_PACKET_MAX"); if (status == PACKET_READ_EOF) break; - if (post_rpc(&rpc, status == PACKET_READ_FLUSH)) + if (post_rpc(&rpc, 1, status == PACKET_READ_FLUSH)) /* We would have an err here */ break; /* Reset the buffer for next request */ diff --git a/remote.h b/remote.h index 11d8719b58..5cc26c1b3b 100644 --- a/remote.h +++ b/remote.h @@ -179,7 +179,8 @@ struct ref **get_remote_heads(struct packet_reader *reader, struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, struct ref **list, int for_push, const struct argv_array *ref_prefixes, - const struct string_list *server_options); + const struct string_list *server_options, + int stateless_rpc); int resolve_remote_symref(struct ref *ref, struct ref *list); diff --git a/serve.c b/serve.c index 317256c1a4..c046926ba1 100644 --- a/serve.c +++ b/serve.c @@ -217,6 +217,8 @@ static int process_request(void) state = PROCESS_REQUEST_DONE; break; + case PACKET_READ_RESPONSE_END: + BUG("unexpected stateless separator packet"); } } diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c index 12ca698e17..69152958e5 100644 --- a/t/helper/test-pkt-line.c +++ b/t/helper/test-pkt-line.c @@ -46,6 +46,9 @@ static void unpack(void) case PACKET_READ_DELIM: printf("0001\n"); break; + case PACKET_READ_RESPONSE_END: + printf("0002\n"); + break; } } } @@ -75,6 +78,7 @@ static void unpack_sideband(void) case PACKET_READ_FLUSH: return; case PACKET_READ_DELIM: + case PACKET_READ_RESPONSE_END: break; } } diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 1449ee95e9..d2edfa4c50 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -129,6 +129,8 @@ install_script () { prepare_httpd() { mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH" cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" + install_script incomplete-length-upload-pack-v2-http.sh + install_script incomplete-body-upload-pack-v2-http.sh install_script broken-smart-http.sh install_script error-smart-http.sh install_script error.sh diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 994e5290d6..afa91e38b0 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -117,6 +117,8 @@ Alias /auth/dumb/ www/auth/dumb/ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL +ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/ +ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/ ScriptAliasMatch /error_git_upload_pack/(.*)/git-upload-pack error.sh/ ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1 ScriptAlias /broken_smart/ broken-smart-http.sh/ @@ -126,6 +128,12 @@ ScriptAliasMatch /one_time_perl/(.*) apply-one-time-perl.sh/$1 Options FollowSymlinks + + Options ExecCGI + + + Options ExecCGI + Options ExecCGI diff --git a/t/lib-httpd/incomplete-body-upload-pack-v2-http.sh b/t/lib-httpd/incomplete-body-upload-pack-v2-http.sh new file mode 100644 index 0000000000..90e73ef8d5 --- /dev/null +++ b/t/lib-httpd/incomplete-body-upload-pack-v2-http.sh @@ -0,0 +1,3 @@ +printf "Content-Type: text/%s\n" "application/x-git-upload-pack-result" +echo +printf "%s%s" "0079" "45" diff --git a/t/lib-httpd/incomplete-length-upload-pack-v2-http.sh b/t/lib-httpd/incomplete-length-upload-pack-v2-http.sh new file mode 100644 index 0000000000..dce552e348 --- /dev/null +++ b/t/lib-httpd/incomplete-length-upload-pack-v2-http.sh @@ -0,0 +1,3 @@ +printf "Content-Type: text/%s\n" "application/x-git-upload-pack-result" +echo +printf "%s" "00" diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 5039e66dc4..8da65e60de 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -586,6 +586,53 @@ test_expect_success 'clone with http:// using protocol v2' ' ! grep "Send header: Transfer-Encoding: chunked" log ' +test_expect_success 'clone repository with http:// using protocol v2 with incomplete pktline length' ' + test_when_finished "rm -f log" && + + git init "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_length" && + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_length" file && + + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git -c protocol.version=2 \ + clone "$HTTPD_URL/smart/incomplete_length" incomplete_length_child 2>err && + + # Client requested to use protocol v2 + grep "Git-Protocol: version=2" log && + # Server responded using protocol v2 + grep "git< version 2" log && + # Client reported appropriate failure + test_i18ngrep "bytes of length header were received" err +' + +test_expect_success 'clone repository with http:// using protocol v2 with incomplete pktline body' ' + test_when_finished "rm -f log" && + + git init "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_body" && + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_body" file && + + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git -c protocol.version=2 \ + clone "$HTTPD_URL/smart/incomplete_body" incomplete_body_child 2>err && + + # Client requested to use protocol v2 + grep "Git-Protocol: version=2" log && + # Server responded using protocol v2 + grep "git< version 2" log && + # Client reported appropriate failure + test_i18ngrep "bytes of body are still expected" err +' + +test_expect_success 'clone with http:// using protocol v2 and invalid parameters' ' + test_when_finished "rm -f log" && + + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" \ + git -c protocol.version=2 \ + clone --shallow-since=20151012 "$HTTPD_URL/smart/http_parent" http_child_invalid && + + # Client requested to use protocol v2 + grep "Git-Protocol: version=2" log && + # Server responded using protocol v2 + grep "git< version 2" log +' + test_expect_success 'clone big repository with http:// using protocol v2' ' test_when_finished "rm -f log" && diff --git a/transport.c b/transport.c index 15f5ba4e8f..7d50c502ad 100644 --- a/transport.c +++ b/transport.c @@ -297,7 +297,8 @@ static struct ref *handshake(struct transport *transport, int for_push, if (must_list_refs) get_remote_refs(data->fd[1], &reader, &refs, for_push, ref_prefixes, - transport->server_options); + transport->server_options, + transport->stateless_rpc); break; case protocol_v1: case protocol_v0: @@ -369,24 +370,15 @@ static int fetch_refs_via_pack(struct transport *transport, refs_tmp = handshake(transport, 0, NULL, must_list_refs); } - switch (data->version) { - case protocol_v2: - refs = fetch_pack(&args, data->fd, - refs_tmp ? refs_tmp : transport->remote_refs, - to_fetch, nr_heads, &data->shallow, - &transport->pack_lockfile, data->version); - break; - case protocol_v1: - case protocol_v0: - die_if_server_options(transport); - refs = fetch_pack(&args, data->fd, - refs_tmp ? refs_tmp : transport->remote_refs, - to_fetch, nr_heads, &data->shallow, - &transport->pack_lockfile, data->version); - break; - case protocol_unknown_version: + if (data->version == protocol_unknown_version) BUG("unknown protocol version"); - } + else if (data->version <= protocol_v1) + die_if_server_options(transport); + + refs = fetch_pack(&args, data->fd, + refs_tmp ? refs_tmp : transport->remote_refs, + to_fetch, nr_heads, &data->shallow, + &transport->pack_lockfile, data->version); close(data->fd[0]); close(data->fd[1]);