git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Masaya Suzuki <masayasuzuki@google.com>
To: masayasuzuki@google.com
Cc: git@vger.kernel.org, jrnieder@gmail.com, peff@peff.net,
	sunshine@sunshineco.com
Subject: [PATCH v2 2/2] Unset CURLOPT_FAILONERROR
Date: Sat, 29 Dec 2018 11:44:47 -0800
Message-ID: <20181229194447.157763-3-masayasuzuki@google.com> (raw)
In-Reply-To: <20181229194447.157763-1-masayasuzuki@google.com>

When GIT_CURL_VERBOSE is set, libcurl produces request/response headers
to stderr. However, if the response is an error response and
CURLOPT_FAILONERROR is set, libcurl stops parsing the response, and it
won't dump the headers. Showing HTTP response headers is useful for
debugging, especially for non-OK responses.

This is substantially same as setting http_options.keep_error to all
requests. Hence, remove this option.

Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
 http.c                       |  4 ----
 http.h                       |  1 -
 remote-curl.c                |  1 -
 t/lib-httpd/apache.conf      |  1 +
 t/t5581-http-curl-verbose.sh | 28 ++++++++++++++++++++++++++++
 5 files changed, 29 insertions(+), 6 deletions(-)
 create mode 100755 t/t5581-http-curl-verbose.sh

diff --git a/http.c b/http.c
index d23417670..8f8101da3 100644
--- a/http.c
+++ b/http.c
@@ -1269,7 +1269,6 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
 
 	/*
@@ -1848,8 +1847,6 @@ static int http_request(const char *url,
 	strbuf_addstr(&buf, "Pragma:");
 	if (options && options->no_cache)
 		strbuf_addstr(&buf, " no-cache");
-	if (options && options->keep_error)
-		curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
 	if (options && options->initial_request &&
 	    http_follow_config == HTTP_FOLLOW_INITIAL)
 		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
@@ -2415,7 +2412,6 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	freq->slot = get_active_slot();
 
 	curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
-	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
diff --git a/http.h b/http.h
index d305ca1dc..eebf40688 100644
--- a/http.h
+++ b/http.h
@@ -146,7 +146,6 @@ extern char *get_remote_object_url(const char *url, const char *hex,
 /* Options for http_get_*() */
 struct http_get_options {
 	unsigned no_cache:1,
-		 keep_error:1,
 		 initial_request:1;
 
 	/* If non-NULL, returns the content-type of the response. */
diff --git a/remote-curl.c b/remote-curl.c
index 48656bf18..43e7a1d80 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -380,7 +380,6 @@ static struct discovery *discover_refs(const char *service, int for_push)
 	http_options.extra_headers = &extra_headers;
 	http_options.initial_request = 1;
 	http_options.no_cache = 1;
-	http_options.keep_error = 1;
 
 	http_ret = http_get_strbuf(refs_url.buf, &buffer, &http_options);
 	switch (http_ret) {
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 581c010d8..cc4b87507 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -115,6 +115,7 @@ Alias /auth/dumb/ www/auth/dumb/
 	SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
 	SetEnv GIT_HTTP_EXPORT_ALL
 </LocationMatch>
+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/ error.sh/
diff --git a/t/t5581-http-curl-verbose.sh b/t/t5581-http-curl-verbose.sh
new file mode 100755
index 000000000..73148eeba
--- /dev/null
+++ b/t/t5581-http-curl-verbose.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='test GIT_CURL_VERBOSE'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success 'setup repository' '
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git -C "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" --bare init &&
+	git config push.default matching &&
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master
+'
+
+test_expect_success 'failure in git-upload-pack is shown' '
+	test_might_fail env GIT_CURL_VERBOSE=1 \
+		git clone "$HTTPD_URL/error_git_upload_pack/smart/repo.git" \
+		2>curl_log &&
+	cat curl_log | grep "< HTTP/1.1 500 Intentional Breakage"
+'
+
+stop_httpd
+
+test_done
-- 
2.20.1.415.g653613c723-goog


  parent reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-28  1:47 [PATCH 1/2] Change how HTTP response body is returned Masaya Suzuki
2018-12-28  1:47 ` [PATCH 2/2] Unset CURLOPT_FAILONERROR Masaya Suzuki
2018-12-28 19:36   ` Eric Sunshine
2018-12-28 19:51     ` Masaya Suzuki
2018-12-28 19:58       ` Eric Sunshine
2018-12-28 20:00         ` Masaya Suzuki
2018-12-29 19:44 ` [PATCH v2 0/2] Show HTTP headers of failed requests with GIT_CURL_VERBOSE Masaya Suzuki
2018-12-29 19:44   ` [PATCH v2 1/2] Change how HTTP response body is returned Masaya Suzuki
2019-01-03 19:09     ` Junio C Hamano
2019-01-04 10:11       ` Jeff King
2019-01-04 20:13         ` Junio C Hamano
2019-01-04 10:30     ` Jeff King
2018-12-29 19:44   ` Masaya Suzuki [this message]
2019-01-04 10:49     ` [PATCH v2 2/2] Unset CURLOPT_FAILONERROR Jeff King
2019-01-07 23:24       ` Masaya Suzuki
2019-01-08  2:47   ` [PATCH v3 0/5] Show HTTP headers of failed requests with GIT_CURL_VERBOSE Masaya Suzuki
2019-01-08  2:47     ` [PATCH v3 1/5] http: support file handles for HTTP_KEEP_ERROR Masaya Suzuki
2019-01-09 12:15       ` SZEDER Gábor
2019-01-08  2:47     ` [PATCH v3 2/5] http: enable keep_error for HTTP requests Masaya Suzuki
2019-01-08  2:47     ` [PATCH v3 3/5] remote-curl: define struct for CURLOPT_WRITEFUNCTION Masaya Suzuki
2019-01-08  2:47     ` [PATCH v3 4/5] remote-curl: unset CURLOPT_FAILONERROR Masaya Suzuki
2019-01-08  2:47     ` [PATCH v3 5/5] test: test GIT_CURL_VERBOSE=1 shows an error Masaya Suzuki
2019-01-10 19:33     ` [PATCH v4 0/5] Show HTTP headers of failed requests with GIT_CURL_VERBOSE Masaya Suzuki
2019-01-10 19:33       ` [PATCH v4 1/5] http: support file handles for HTTP_KEEP_ERROR Masaya Suzuki
2019-01-10 19:33       ` [PATCH v4 2/5] http: enable keep_error for HTTP requests Masaya Suzuki
2019-01-10 19:33       ` [PATCH v4 3/5] remote-curl: define struct for CURLOPT_WRITEFUNCTION Masaya Suzuki
2019-01-10 19:33       ` [PATCH v4 4/5] remote-curl: unset CURLOPT_FAILONERROR Masaya Suzuki
2019-01-10 19:33       ` [PATCH v4 5/5] test: test GIT_CURL_VERBOSE=1 shows an error Masaya Suzuki
2019-01-10 23:06       ` [PATCH v4 0/5] Show HTTP headers of failed requests with GIT_CURL_VERBOSE Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181229194447.157763-3-masayasuzuki@google.com \
    --to=masayasuzuki@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox