Commits

Anonymous committed c21031b Merge

Merge branch 'sp/smart-http-content-type-check' into jch

The smart HTTP clients forgot to verify the content-type that comes
back from the server side to make sure that the request is being
handled properly.

Will merge to 'next'.

* sp/smart-http-content-type-check:
Verify Content-Type from smart HTTP servers

Comments (0)

Files changed (8)

 
 	sprintf(url, "%s%s", repo->url, path);
 
-	switch (http_get_strbuf(url, NULL, 0)) {
+	switch (http_get_strbuf(url, NULL, NULL, 0)) {
 	case HTTP_OK:
 		ret = 1;
 		break;
 	url = xmalloc(strlen(repo->url) + strlen(path) + 1);
 	sprintf(url, "%s%s", repo->url, path);
 
-	if (http_get_strbuf(url, &buffer, 0) != HTTP_OK)
+	if (http_get_strbuf(url, NULL, &buffer, 0) != HTTP_OK)
 		die("Couldn't get %s for remote symref\n%s", url,
 		    curl_errorstr);
 	free(url);
 #define HTTP_REQUEST_STRBUF	0
 #define HTTP_REQUEST_FILE	1
 
-static int http_request(const char *url, void *result, int target, int options)
+static int http_request(const char *url, struct strbuf *type,
+			void *result, int target, int options)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
 		ret = HTTP_START_FAILED;
 	}
 
+	if (type) {
+		char *t;
+		curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t);
+		if (t)
+			strbuf_addstr(type, t);
+	}
+
 	curl_slist_free_all(headers);
 	strbuf_release(&buf);
 
 	return ret;
 }
 
-static int http_request_reauth(const char *url, void *result, int target,
+static int http_request_reauth(const char *url,
+			       struct strbuf *type,
+			       void *result, int target,
 			       int options)
 {
-	int ret = http_request(url, result, target, options);
+	int ret = http_request(url, type, result, target, options);
 	if (ret != HTTP_REAUTH)
 		return ret;
-	return http_request(url, result, target, options);
+	return http_request(url, type, result, target, options);
 }
 
-int http_get_strbuf(const char *url, struct strbuf *result, int options)
+int http_get_strbuf(const char *url,
+		    struct strbuf *type,
+		    struct strbuf *result, int options)
 {
-	return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
+	return http_request_reauth(url, type, result,
+				   HTTP_REQUEST_STRBUF, options);
 }
 
 /*
 		goto cleanup;
 	}
 
-	ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
+	ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, options);
 	fclose(result);
 
 	if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
 	int ret = -1;
 
 	url = quote_ref_url(base, ref->name);
-	if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
+	if (http_get_strbuf(url, NULL, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
 		strbuf_rtrim(&buffer);
 		if (buffer.len == 40)
 			ret = get_sha1_hex(buffer.buf, ref->old_sha1);
 	strbuf_addstr(&buf, "objects/info/packs");
 	url = strbuf_detach(&buf, NULL);
 
-	ret = http_get_strbuf(url, &buf, HTTP_NO_CACHE);
+	ret = http_get_strbuf(url, NULL, &buf, HTTP_NO_CACHE);
 	if (ret != HTTP_OK)
 		goto cleanup;
 
  *
  * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
  */
-int http_get_strbuf(const char *url, struct strbuf *result, int options);
+int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options);
 
 /*
  * Prints an error message using error() containing url and curl_errorstr,
 
 static struct discovery* discover_refs(const char *service)
 {
+	struct strbuf exp = STRBUF_INIT;
+	struct strbuf type = STRBUF_INIT;
 	struct strbuf buffer = STRBUF_INIT;
 	struct discovery *last = last_discovery;
 	char *refs_url;
 	}
 	refs_url = strbuf_detach(&buffer, NULL);
 
-	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
+	http_ret = http_get_strbuf(refs_url, &type, &buffer, HTTP_NO_CACHE);
 	switch (http_ret) {
 	case HTTP_OK:
 		break;
 	last->buf = last->buf_alloc;
 
 	if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
-		/* smart HTTP response; validate that the service
+		/*
+		 * smart HTTP response; validate that the service
 		 * pkt-line matches our request.
 		 */
-		struct strbuf exp = STRBUF_INIT;
-
+		strbuf_addf(&exp, "application/x-%s-advertisement", service);
+		if (strbuf_cmp(&exp, &type))
+			die("invalid content-type %s", type.buf);
 		if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
 			die("%s has invalid packet header", refs_url);
 		if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
 			strbuf_setlen(&buffer, buffer.len - 1);
 
+		strbuf_reset(&exp);
 		strbuf_addf(&exp, "# service=%s", service);
 		if (strbuf_cmp(&exp, &buffer))
 			die("invalid server response; got '%s'", buffer.buf);
 	}
 
 	free(refs_url);
+	strbuf_release(&exp);
+	strbuf_release(&type);
 	strbuf_release(&buffer);
 	last_discovery = last;
 	return last;
 prepare_httpd() {
 	mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
 	cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
+	cp "$TEST_PATH"/broken-smart-http.sh "$HTTPD_ROOT_PATH"
 
 	ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules"
 

t/lib-httpd/apache.conf

 	SetEnv GIT_COMMITTER_EMAIL custom@example.com
 </LocationMatch>
 ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
+ScriptAlias /broken_smart/ broken-smart-http.sh/
 <Directory ${GIT_EXEC_PATH}>
 	Options FollowSymlinks
 </Directory>
+<Files broken-smart-http.sh>
+	Options ExecCGI
+</Files>
 <Files ${GIT_EXEC_PATH}/git-http-backend>
 	Options ExecCGI
 </Files>

t/lib-httpd/broken-smart-http.sh

+#!/bin/sh
+printf "Content-Type: text/%s\n" "html"
+echo
+printf "%s\n" "001e# service=git-upload-pack"
+printf "%s"   "0000"
+printf "%s%c%s%s\n" \
+	"00a58681d9f286a48b08f37b3a095330da16689e3693 HEAD" \
+	0 \
+	" include-tag multi_ack_detailed multi_ack ofs-delta" \
+	" side-band side-band-64k thin-pack no-progress shallow no-done "
+printf "%s"   "0000"

t/t5551-http-fetch.sh

 	 test_must_fail git fetch)
 '
 
+test_expect_success 'invalid Content-Type rejected' '
+	echo "fatal: invalid content-type text/html" >expect
+	test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual
+	test_cmp expect actual
+'
+
 test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
 
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '