git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] http: Add http.savecookies option to write out HTTP cookies
@ 2013-07-23 21:07 dborowitz
  2013-07-23 22:27 ` Junio C Hamano
  2013-07-23 22:40 ` dborowitz
  0 siblings, 2 replies; 3+ messages in thread
From: dborowitz @ 2013-07-23 21:07 UTC (permalink / raw)
  To: git; +Cc: Dave Borowitz

From: Dave Borowitz <dborowitz@google.com>

HTTP servers may send Set-Cookie headers in a response and expect them
to be set on subsequent requests. By default, libcurl behavior is to
store such cookies in memory and reuse them across requests within a
single session. However, it may also make sense, depending on the
server and the cookies, to store them across sessions. Provide users
an option to enable this behavior, writing cookies out to the same
file specified in http.cookiefile.
---
 Documentation/config.txt |  6 +++++-
 http.c                   |  7 +++++++
 t/lib-httpd/apache.conf  |  8 ++++++++
 t/t5551-http-fetch.sh    | 18 ++++++++++++++++++
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e0b923f..e935447 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1456,7 +1456,11 @@ http.cookiefile::
 	of the file to read cookies from should be plain HTTP headers or
 	the Netscape/Mozilla cookie file format (see linkgit:curl[1]).
 	NOTE that the file specified with http.cookiefile is only used as
-	input. No cookies will be stored in the file.
+	input unless http.saveCookies is set.
+
+http.savecookies::
+	If set, store cookies received during requests to the file specified by
+	http.cookiefile. Has no effect if http.cookiefile is unset.
 
 http.sslVerify::
 	Whether to verify the SSL certificate when fetching or pushing
diff --git a/http.c b/http.c
index 2d086ae..2fbf986 100644
--- a/http.c
+++ b/http.c
@@ -45,6 +45,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
+static int curl_save_cookies;
 static struct credential http_auth = CREDENTIAL_INIT;
 static int http_proactive_auth;
 static const char *user_agent;
@@ -200,6 +201,10 @@ static int http_options(const char *var, const char *value, void *cb)
 
 	if (!strcmp("http.cookiefile", var))
 		return git_config_string(&curl_cookie_file, var, value);
+	if (!strcmp("http.savecookies", var)) {
+		curl_save_cookies = git_config_bool(var, value);
+		return 0;
+	}
 
 	if (!strcmp("http.postbuffer", var)) {
 		http_post_buffer = git_config_int(var, value);
@@ -513,6 +518,8 @@ struct active_request_slot *get_active_slot(void)
 	slot->callback_data = NULL;
 	slot->callback_func = NULL;
 	curl_easy_setopt(slot->curl, CURLOPT_COOKIEFILE, curl_cookie_file);
+	if (curl_save_cookies)
+		curl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index dd17e3a..397c480 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -22,6 +22,9 @@ ErrorLog error.log
 <IfModule !mod_version.c>
 	LoadModule version_module modules/mod_version.so
 </IfModule>
+<IfModule !mod_headers.c>
+	LoadModule headers_module modules/mod_headers.so
+</IfModule>
 
 <IfVersion < 2.4>
 LockFile accept.lock
@@ -87,6 +90,11 @@ Alias /auth/dumb/ www/auth/dumb/
 	SetEnv GIT_HTTP_EXPORT_ALL
 	SetEnv GIT_NAMESPACE ns
 </LocationMatch>
+<LocationMatch /smart_cookies/>
+	SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+	SetEnv GIT_HTTP_EXPORT_ALL
+	Header set Set-Cookie name=value
+</LocationMatch>
 ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
 ScriptAlias /broken_smart/ broken-smart-http.sh/
 <Directory ${GIT_EXEC_PATH}>
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 55a866a..287d22b 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -187,6 +187,24 @@ test_expect_success 'dumb clone via http-backend respects namespace' '
 	test_cmp expect actual
 '
 
+cat >cookies.txt <<EOF
+127.0.0.1	FALSE	/smart_cookies/	FALSE	0	othername	othervalue
+EOF
+cat >expect_cookies.txt <<EOF
+# Netscape HTTP Cookie File
+# http://curl.haxx.se/docs/http-cookies.html
+# This file was generated by libcurl! Edit at your own risk.
+
+127.0.0.1	FALSE	/smart_cookies/	FALSE	0	othername	othervalue
+127.0.0.1	FALSE	/smart_cookies/repo.git/info/	FALSE	0	name	value
+EOF
+test_expect_success 'cookies stored in http.cookiefile when http.savecookies set' '
+	git config http.cookiefile cookies.txt &&
+	git config http.savecookies true &&
+	git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
+	test_cmp expect_cookies.txt cookies.txt
+'
+
 test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
 
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] http: Add http.savecookies option to write out HTTP cookies
  2013-07-23 21:07 [PATCH] http: Add http.savecookies option to write out HTTP cookies dborowitz
@ 2013-07-23 22:27 ` Junio C Hamano
  2013-07-23 22:40 ` dborowitz
  1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2013-07-23 22:27 UTC (permalink / raw)
  To: dborowitz; +Cc: git

dborowitz@google.com writes:

> From: Dave Borowitz <dborowitz@google.com>
>
> HTTP servers may send Set-Cookie headers in a response and expect them
> to be set on subsequent requests. By default, libcurl behavior is to
> store such cookies in memory and reuse them across requests within a
> single session. However, it may also make sense, depending on the
> server and the cookies, to store them across sessions. Provide users
> an option to enable this behavior, writing cookies out to the same
> file specified in http.cookiefile.
> ---

Makes sense.

I briefly wondered if users want to be able to selectively store
cookies only from certain sites but not from others.  But if we are
going to build this on top of Kyle J. McKay's "Per URL http.<url>.* 
configuration" series, that will fall out as a natural consequence,
I think.

Please sign-off your patch.  Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] http: Add http.savecookies option to write out HTTP cookies
  2013-07-23 21:07 [PATCH] http: Add http.savecookies option to write out HTTP cookies dborowitz
  2013-07-23 22:27 ` Junio C Hamano
@ 2013-07-23 22:40 ` dborowitz
  1 sibling, 0 replies; 3+ messages in thread
From: dborowitz @ 2013-07-23 22:40 UTC (permalink / raw)
  To: git; +Cc: Dave Borowitz

From: Dave Borowitz <dborowitz@google.com>

HTTP servers may send Set-Cookie headers in a response and expect them
to be set on subsequent requests. By default, libcurl behavior is to
store such cookies in memory and reuse them across requests within a
single session. However, it may also make sense, depending on the
server and the cookies, to store them across sessions. Provide users
an option to enable this behavior, writing cookies out to the same
file specified in http.cookiefile.

Signed-off-by: Dave Borowitz <dborowitz@google.com>
---
 Documentation/config.txt |  6 +++++-
 http.c                   |  7 +++++++
 t/lib-httpd/apache.conf  |  8 ++++++++
 t/t5551-http-fetch.sh    | 18 ++++++++++++++++++
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e0b923f..e935447 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1456,7 +1456,11 @@ http.cookiefile::
 	of the file to read cookies from should be plain HTTP headers or
 	the Netscape/Mozilla cookie file format (see linkgit:curl[1]).
 	NOTE that the file specified with http.cookiefile is only used as
-	input. No cookies will be stored in the file.
+	input unless http.saveCookies is set.
+
+http.savecookies::
+	If set, store cookies received during requests to the file specified by
+	http.cookiefile. Has no effect if http.cookiefile is unset.
 
 http.sslVerify::
 	Whether to verify the SSL certificate when fetching or pushing
diff --git a/http.c b/http.c
index 2d086ae..2fbf986 100644
--- a/http.c
+++ b/http.c
@@ -45,6 +45,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
+static int curl_save_cookies;
 static struct credential http_auth = CREDENTIAL_INIT;
 static int http_proactive_auth;
 static const char *user_agent;
@@ -200,6 +201,10 @@ static int http_options(const char *var, const char *value, void *cb)
 
 	if (!strcmp("http.cookiefile", var))
 		return git_config_string(&curl_cookie_file, var, value);
+	if (!strcmp("http.savecookies", var)) {
+		curl_save_cookies = git_config_bool(var, value);
+		return 0;
+	}
 
 	if (!strcmp("http.postbuffer", var)) {
 		http_post_buffer = git_config_int(var, value);
@@ -513,6 +518,8 @@ struct active_request_slot *get_active_slot(void)
 	slot->callback_data = NULL;
 	slot->callback_func = NULL;
 	curl_easy_setopt(slot->curl, CURLOPT_COOKIEFILE, curl_cookie_file);
+	if (curl_save_cookies)
+		curl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index dd17e3a..397c480 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -22,6 +22,9 @@ ErrorLog error.log
 <IfModule !mod_version.c>
 	LoadModule version_module modules/mod_version.so
 </IfModule>
+<IfModule !mod_headers.c>
+	LoadModule headers_module modules/mod_headers.so
+</IfModule>
 
 <IfVersion < 2.4>
 LockFile accept.lock
@@ -87,6 +90,11 @@ Alias /auth/dumb/ www/auth/dumb/
 	SetEnv GIT_HTTP_EXPORT_ALL
 	SetEnv GIT_NAMESPACE ns
 </LocationMatch>
+<LocationMatch /smart_cookies/>
+	SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+	SetEnv GIT_HTTP_EXPORT_ALL
+	Header set Set-Cookie name=value
+</LocationMatch>
 ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
 ScriptAlias /broken_smart/ broken-smart-http.sh/
 <Directory ${GIT_EXEC_PATH}>
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 55a866a..287d22b 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -187,6 +187,24 @@ test_expect_success 'dumb clone via http-backend respects namespace' '
 	test_cmp expect actual
 '
 
+cat >cookies.txt <<EOF
+127.0.0.1	FALSE	/smart_cookies/	FALSE	0	othername	othervalue
+EOF
+cat >expect_cookies.txt <<EOF
+# Netscape HTTP Cookie File
+# http://curl.haxx.se/docs/http-cookies.html
+# This file was generated by libcurl! Edit at your own risk.
+
+127.0.0.1	FALSE	/smart_cookies/	FALSE	0	othername	othervalue
+127.0.0.1	FALSE	/smart_cookies/repo.git/info/	FALSE	0	name	value
+EOF
+test_expect_success 'cookies stored in http.cookiefile when http.savecookies set' '
+	git config http.cookiefile cookies.txt &&
+	git config http.savecookies true &&
+	git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
+	test_cmp expect_cookies.txt cookies.txt
+'
+
 test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
 
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-07-23 22:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-23 21:07 [PATCH] http: Add http.savecookies option to write out HTTP cookies dborowitz
2013-07-23 22:27 ` Junio C Hamano
2013-07-23 22:40 ` dborowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).