diff options
author | Junio C Hamano <gitster@pobox.com> | 2019-02-09 07:44:51 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-02-09 07:44:51 +0300 |
commit | fd357c4270ea4b48e95fb475fdc9b6ce2ee447e1 (patch) | |
tree | 77630863e8b1fc6b42536e1945213922e2ee1c76 | |
parent | 5a5f40881d6dcebec36190115779eeb4ae89b6c9 (diff) | |
parent | 30dea56536e78ad8a9533d51ad1d8c9c85c3c7bd (diff) |
Merge branch 'js/smart-http-detect-remote-error'
Some errors from the other side coming over smart HTTP transport
were not noticed, which has been corrected.
* js/smart-http-detect-remote-error:
t5551: test server-side ERR packet
remote-curl: tighten "version 2" check for smart-http
remote-curl: refactor smart-http discovery
-rw-r--r-- | remote-curl.c | 100 | ||||
-rw-r--r-- | t/lib-httpd.sh | 1 | ||||
-rw-r--r-- | t/lib-httpd/apache.conf | 4 | ||||
-rw-r--r-- | t/lib-httpd/error-smart-http.sh | 3 | ||||
-rwxr-xr-x | t/t5551-http-fetch-smart.sh | 5 |
5 files changed, 70 insertions, 43 deletions
diff --git a/remote-curl.c b/remote-curl.c index 2e04d53ac8..bb7421023b 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -331,9 +331,63 @@ static int get_protocol_http_header(enum protocol_version version, return 0; } +static void check_smart_http(struct discovery *d, const char *service, + struct strbuf *type) +{ + const char *p; + struct packet_reader reader; + + /* + * If we don't see x-$service-advertisement, then it's not smart-http. + * But once we do, we commit to it and assume any other protocol + * violations are hard errors. + */ + if (!skip_prefix(type->buf, "application/x-", &p) || + !skip_prefix(p, service, &p) || + strcmp(p, "-advertisement")) + return; + + packet_reader_init(&reader, -1, d->buf, d->len, + PACKET_READ_CHOMP_NEWLINE | + PACKET_READ_DIE_ON_ERR_PACKET); + if (packet_reader_read(&reader) != PACKET_READ_NORMAL) + die("invalid server response; expected service, got flush packet"); + + if (skip_prefix(reader.line, "# service=", &p) && !strcmp(p, service)) { + /* + * The header can include additional metadata lines, up + * until a packet flush marker. Ignore these now, but + * in the future we might start to scan them. + */ + for (;;) { + packet_reader_read(&reader); + if (reader.pktlen <= 0) { + break; + } + } + + /* + * v0 smart http; callers expect us to soak up the + * service and header packets + */ + d->buf = reader.src_buffer; + d->len = reader.src_len; + d->proto_git = 1; + + } else if (!strcmp(reader.line, "version 2")) { + /* + * v2 smart http; do not consume version packet, which will + * be handled elsewhere. + */ + d->proto_git = 1; + + } else { + die("invalid server response; got '%s'", reader.line); + } +} + static struct discovery *discover_refs(const char *service, int for_push) { - struct strbuf exp = STRBUF_INIT; struct strbuf type = STRBUF_INIT; struct strbuf charset = STRBUF_INIT; struct strbuf buffer = STRBUF_INIT; @@ -405,47 +459,8 @@ static struct discovery *discover_refs(const char *service, int for_push) last->buf_alloc = strbuf_detach(&buffer, &last->len); last->buf = last->buf_alloc; - strbuf_addf(&exp, "application/x-%s-advertisement", service); - if (maybe_smart && - (5 <= last->len && last->buf[4] == '#') && - !strbuf_cmp(&exp, &type)) { - struct packet_reader reader; - packet_reader_init(&reader, -1, last->buf, last->len, - PACKET_READ_CHOMP_NEWLINE | - PACKET_READ_DIE_ON_ERR_PACKET); - - /* - * smart HTTP response; validate that the service - * pkt-line matches our request. - */ - if (packet_reader_read(&reader) != PACKET_READ_NORMAL) - die("invalid server response; expected service, got flush packet"); - - strbuf_reset(&exp); - strbuf_addf(&exp, "# service=%s", service); - if (strcmp(reader.line, exp.buf)) - die("invalid server response; got '%s'", reader.line); - strbuf_release(&exp); - - /* The header can include additional metadata lines, up - * until a packet flush marker. Ignore these now, but - * in the future we might start to scan them. - */ - for (;;) { - packet_reader_read(&reader); - if (reader.pktlen <= 0) { - break; - } - } - - last->buf = reader.src_buffer; - last->len = reader.src_len; - - last->proto_git = 1; - } else if (maybe_smart && - last->len > 5 && starts_with(last->buf + 4, "version 2")) { - last->proto_git = 1; - } + if (maybe_smart) + check_smart_http(last, service, &type); if (last->proto_git) last->refs = parse_git_refs(last, for_push); @@ -453,7 +468,6 @@ static struct discovery *discover_refs(const char *service, int for_push) last->refs = parse_info_refs(last); strbuf_release(&refs_url); - strbuf_release(&exp); strbuf_release(&type); strbuf_release(&charset); strbuf_release(&effective_url); diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index e465116ef9..216281eabc 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -131,6 +131,7 @@ prepare_httpd() { mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH" cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" install_script broken-smart-http.sh + install_script error-smart-http.sh install_script error.sh install_script apply-one-time-sed.sh diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 5d63ed90c5..06a81b54c7 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -119,6 +119,7 @@ Alias /auth/dumb/ www/auth/dumb/ 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/ +ScriptAlias /error_smart/ error-smart-http.sh/ ScriptAlias /error/ error.sh/ ScriptAliasMatch /one_time_sed/(.*) apply-one-time-sed.sh/$1 <Directory ${GIT_EXEC_PATH}> @@ -127,6 +128,9 @@ ScriptAliasMatch /one_time_sed/(.*) apply-one-time-sed.sh/$1 <Files broken-smart-http.sh> Options ExecCGI </Files> +<Files error-smart-http.sh> + Options ExecCGI +</Files> <Files error.sh> Options ExecCGI </Files> diff --git a/t/lib-httpd/error-smart-http.sh b/t/lib-httpd/error-smart-http.sh new file mode 100644 index 0000000000..e65d447fc4 --- /dev/null +++ b/t/lib-httpd/error-smart-http.sh @@ -0,0 +1,3 @@ +echo "Content-Type: application/x-git-upload-pack-advertisement" +echo +printf "%s" "0019ERR server-side error" diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 8630b0cc39..ba83e567e5 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -429,5 +429,10 @@ test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' ' ! grep "=> Send data" err ' +test_expect_success 'server-side error detected' ' + test_must_fail git clone $HTTPD_URL/error_smart/repo.git 2>actual && + grep "server-side error" actual +' + stop_httpd test_done |