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
diff --git a/remote-curl.c b/remote-curl.c
index 2e04d53..bb74210 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -331,9 +331,63 @@
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 @@
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 @@
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 e465116..216281e 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -131,6 +131,7 @@
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 5d63ed9..06a81b5 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -119,6 +119,7 @@
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 @@
<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 0000000..e65d447
--- /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 8630b0c..ba83e56 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -429,5 +429,10 @@
! 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
