All of lore.kernel.org
 help / color / mirror / Atom feed
* git "smart http" server vs. http redirects
@ 2010-09-18  4:33 Miles Bader
  2010-09-18  7:03 ` Ilari Liusvaara
  0 siblings, 1 reply; 9+ messages in thread
From: Miles Bader @ 2010-09-18  4:33 UTC (permalink / raw)
  To: git

The savannah.gnu.org admins are trying out the git "smart http" server,
but it doesn't seem to handle http redirects properly... should it?
Is this a bug with the git server?

The follow is from Sylvain Beucler;
> Hmmm, actually it works, but it doesn't support a HTTP redirect
> (namely git.sv.gnu.org -> git.savannah.gnu.org).
>
> $ git clone http://git.sv.gnu.org/r/freedink.git
> Initialized empty Git repository in /tmp/freedink/.git/
> error: RPC failed; result=22, HTTP code = 405
> ^C
>
> $ git clone http://git.savannah.gnu.org/r/freedink.git
> Initialized empty Git repository in /tmp/freedink/.git/
> remote: Counting objects: 5528, done.
> remote: Compressing objects:  11% (174/1578)

This is the response I get from git.sv.gnu.org manually:

  $ telnet git.sv.gnu.org http
  Trying 140.186.70.72...
  Connected to git.sv.gnu.org.
  Escape character is '^]'.
  GET /r/freedink.git HTTP/1.1
  Host: git.sv.gnu.org

  HTTP/1.1 301 Moved Permanently
  Date: Fri, 17 Sep 2010 06:03:27 GMT
  Server: Apache/2.2.9 (Debian) DAV/2 SVN/1.5.1 mod_python/3.2.10 Python/2.4.4 mod_wsgi/2.5
  Location: http://git.savannah.gnu.org/r/freedink.git
  Content-Length: 389
  Content-Type: elided-to-get-past-vger's-filter; charset=iso-8859-1

  <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
  <html><head>
  <title>301 Moved Permanently</title>
  </head><body>
  <h1>Moved Permanently</h1>
  <p>The document has moved <a href="http://git.savannah.gnu.org/r/freedink.git">here</a>.</p>
  <hr>
  <address>Apache/2.2.9 (Debian) DAV/2 SVN/1.5.1 mod_python/3.2.10 Python/2.4.4 mod_wsgi/2.5 Server at git.sv.gnu.org Port 80</address>
  </body></html>
  Connection closed by foreign host.

Thanks,

-Miles

-- 
XML is like violence.  If it doesn't solve your problem, you're not
using enough of it.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: git "smart http" server vs. http redirects
  2010-09-18  4:33 git "smart http" server vs. http redirects Miles Bader
@ 2010-09-18  7:03 ` Ilari Liusvaara
  2010-09-18  8:16   ` Andreas Schwab
  0 siblings, 1 reply; 9+ messages in thread
From: Ilari Liusvaara @ 2010-09-18  7:03 UTC (permalink / raw)
  To: Miles Bader; +Cc: git

On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:

> The savannah.gnu.org admins are trying out the git "smart http" server,
> but it doesn't seem to handle http redirects properly... should it?
> Is this a bug with the git server?
> 
> The follow is from Sylvain Beucler;
> > Hmmm, actually it works, but it doesn't support a HTTP redirect
> > (namely git.sv.gnu.org -> git.savannah.gnu.org).
> >
> > $ git clone http://git.sv.gnu.org/r/freedink.git
> > Initialized empty Git repository in /tmp/freedink/.git/
> > error: RPC failed; result=22, HTTP code = 405

405 (Method not allowed)? Can you see what request it is trying to send
and to where (the request that fails with 405 that is)?

-Ilari

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: git "smart http" server vs. http redirects
  2010-09-18  7:03 ` Ilari Liusvaara
@ 2010-09-18  8:16   ` Andreas Schwab
  2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2010-09-18  8:16 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: Miles Bader, git

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:
>
>> The savannah.gnu.org admins are trying out the git "smart http" server,
>> but it doesn't seem to handle http redirects properly... should it?
>> Is this a bug with the git server?
>> 
>> The follow is from Sylvain Beucler;
>> > Hmmm, actually it works, but it doesn't support a HTTP redirect
>> > (namely git.sv.gnu.org -> git.savannah.gnu.org).
>> >
>> > $ git clone http://git.sv.gnu.org/r/freedink.git
>> > Initialized empty Git repository in /tmp/freedink/.git/
>> > error: RPC failed; result=22, HTTP code = 405
>
> 405 (Method not allowed)? Can you see what request it is trying to send
> and to where (the request that fails with 405 that is)?

I think this is a bug in the apache setup at savannnah.  It is
responding to POST .../git-upload-pack with both 301 and 405.  This does
not happen if you use the redirected address directly.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH] smart-http: Don't change POST to GET when following redirect
  2010-09-18  8:16   ` Andreas Schwab
@ 2010-09-18  8:47     ` Andreas Schwab
  2010-09-18 14:09       ` Miles Bader
                         ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Andreas Schwab @ 2010-09-18  8:47 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: Miles Bader, git

When the remote HTTP server returns a redirect the default libcurl action
is to change a POST request into a GET request while following the
redirect, but the remote http backend does not expect that.  Tell libcurl
to always keep the POST request.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
Andreas Schwab <schwab@linux-m68k.org> writes:

> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
>
>> On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:
>>
>>> The savannah.gnu.org admins are trying out the git "smart http" server,
>>> but it doesn't seem to handle http redirects properly... should it?
>>> Is this a bug with the git server?
>>> 
>>> The follow is from Sylvain Beucler;
>>> > Hmmm, actually it works, but it doesn't support a HTTP redirect
>>> > (namely git.sv.gnu.org -> git.savannah.gnu.org).
>>> >
>>> > $ git clone http://git.sv.gnu.org/r/freedink.git
>>> > Initialized empty Git repository in /tmp/freedink/.git/
>>> > error: RPC failed; result=22, HTTP code = 405
>>
>> 405 (Method not allowed)? Can you see what request it is trying to send
>> and to where (the request that fails with 405 that is)?
>
> I think this is a bug in the apache setup at savannnah.  It is
> responding to POST .../git-upload-pack with both 301 and 405.  This does
> not happen if you use the redirected address directly.

Looking closer, this is actually a bug in git.  It does not set the
POSTREDIR option, so that libcurl changes the POST into GET when
following a redirection.

Andreas.

---
 remote-curl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 04d4813..309b024 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -393,6 +393,7 @@ static int post_rpc(struct rpc_state *rpc)
 	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
+	curl_easy_setopt(slot->curl, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
 
 	headers = curl_slist_append(headers, rpc->hdr_content_type);
 	headers = curl_slist_append(headers, rpc->hdr_accept);
-- 
1.7.2.3


-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] smart-http: Don't change POST to GET when following redirect
  2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
@ 2010-09-18 14:09       ` Miles Bader
  2010-09-18 21:00       ` Jay Soffian
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Miles Bader @ 2010-09-18 14:09 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Ilari Liusvaara, git

Thanks Andreas!

-miles

-- 
Acquaintance, n. A person whom we know well enough to borrow from, but not
well enough to lend to.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] smart-http: Don't change POST to GET when following redirect
  2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
  2010-09-18 14:09       ` Miles Bader
@ 2010-09-18 21:00       ` Jay Soffian
  2010-09-24  6:20       ` [PATCH v2] " Tay Ray Chuan
  2010-09-25  4:20       ` [PATCH v3] " Tay Ray Chuan
  3 siblings, 0 replies; 9+ messages in thread
From: Jay Soffian @ 2010-09-18 21:00 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Ilari Liusvaara, Miles Bader, git

On Sat, Sep 18, 2010 at 4:47 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> +       curl_easy_setopt(slot->curl, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);

Not sure what git's minimum supported libcurl is, but this define was
added in 7.19.1 (November 5 2008).

j.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v2] smart-http: Don't change POST to GET when following redirect
  2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
  2010-09-18 14:09       ` Miles Bader
  2010-09-18 21:00       ` Jay Soffian
@ 2010-09-24  6:20       ` Tay Ray Chuan
  2010-09-24 18:06         ` Andreas Schwab
  2010-09-25  4:20       ` [PATCH v3] " Tay Ray Chuan
  3 siblings, 1 reply; 9+ messages in thread
From: Tay Ray Chuan @ 2010-09-24  6:20 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Andreas Schwab, Jay Soffian, Ilari Liusvaara,
	Miles Bader

From: Andreas Schwab <schwab@linux-m68k.org>

For a long time (29508e1 "Isolate shared HTTP request functionality", Fri
Nov 18 11:02:58 2005), we've followed HTTP redirects with
CURLOPT_FOLLOWLOCATION.

However, when the remote HTTP server returns a redirect the default
libcurl action is to change a POST request into a GET request while
following the redirect, but the remote http backend does not expect
that.

Fix this by telling libcurl to always keep the request as type POST with
CURLOPT_POSTREDIR.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

Added simple tests and made some changes to the patch message.

Andreas:
  shifted the setopt to right after FOLLOWLOCATION, since they're linked
  closely.

Jay: added the usual hexadecimal version checks.

 http.c                  |    3 +++
 t/lib-httpd/apache.conf |    7 +++++++
 t/t5551-http-fetch.sh   |    8 ++++++++
 3 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index 1320c50..25f8b45 100644
--- a/http.c
+++ b/http.c
@@ -275,6 +275,9 @@ static CURL *get_curl_handle(void)
 	}

 	curl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);
+#if LIBCURL_VERSION_NUM >= 0x071301
+	curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#endif

 	if (getenv("GIT_CURL_VERBOSE"))
 		curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 4961505..f41c7c6 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -17,6 +17,9 @@ ErrorLog error.log
 <IfModule !mod_env.c>
 	LoadModule env_module modules/mod_env.so
 </IfModule>
+<IfModule !mod_rewrite.c>
+	LoadModule rewrite_module modules/mod_rewrite.so
+</IFModule>

 Alias /dumb/ www/

@@ -36,6 +39,10 @@ ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
 	Options ExecCGI
 </Files>

+RewriteEngine on
+RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
+RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
+
 <IfDefine SSL>
 LoadModule ssl_module modules/mod_ssl.so

diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index fd19121..26d3557 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -101,5 +101,13 @@ test_expect_success 'used upload-pack service' '
 	test_cmp exp act
 '

+test_expect_success 'follow redirects (301)' '
+	git clone $HTTPD_URL/smart-redir-perm/repo.git --quiet repo-p
+'
+
+test_expect_success 'follow redirects (302)' '
+	git clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t
+'
+
 stop_httpd
 test_done
--
1.7.3.67.g2a10b

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] smart-http: Don't change POST to GET when following redirect
  2010-09-24  6:20       ` [PATCH v2] " Tay Ray Chuan
@ 2010-09-24 18:06         ` Andreas Schwab
  0 siblings, 0 replies; 9+ messages in thread
From: Andreas Schwab @ 2010-09-24 18:06 UTC (permalink / raw)
  To: Tay Ray Chuan
  Cc: Git Mailing List, Junio C Hamano, Jay Soffian, Ilari Liusvaara,
	Miles Bader

Tay Ray Chuan <rctay89@gmail.com> writes:

> diff --git a/http.c b/http.c
> index 1320c50..25f8b45 100644
> --- a/http.c
> +++ b/http.c
> @@ -275,6 +275,9 @@ static CURL *get_curl_handle(void)
>  	}
>
>  	curl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);
> +#if LIBCURL_VERSION_NUM >= 0x071301
> +	curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
> +#endif

Should this fall back to CURLOPT_POST301 on older versions?  (Those
won't handle 302 though, I think.)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3] smart-http: Don't change POST to GET when following redirect
  2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
                         ` (2 preceding siblings ...)
  2010-09-24  6:20       ` [PATCH v2] " Tay Ray Chuan
@ 2010-09-25  4:20       ` Tay Ray Chuan
  3 siblings, 0 replies; 9+ messages in thread
From: Tay Ray Chuan @ 2010-09-25  4:20 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Andreas Schwab, Jay Soffian, Ilari Liusvaara,
	Miles Bader

For a long time (29508e1 "Isolate shared HTTP request functionality", Fri
Nov 18 11:02:58 2005), we've followed HTTP redirects with
CURLOPT_FOLLOWLOCATION.

However, when the remote HTTP server returns a redirect the default
libcurl action is to change a POST request into a GET request while
following the redirect, but the remote http backend does not expect
that.

Fix this by telling libcurl to always keep the request as type POST with
CURLOPT_POSTREDIR.

For users of libcurl older than 7.19.1, use CURLOPT_POST301 instead,
which only follows 301s instead of both 301s and 302s.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

Andreas: added CURLOPT_POST301 alternative. There doesn't seem to be a pretty
         constant, the docs just say a non-zero value [1] or 1 [2].

[1] http://github.com/bagder/curl/blob/curl-7_17_1/docs/libcurl/curl_easy_setopt.3#L646
[2] http://github.com/bagder/curl/blob/curl-7_19_0/docs/libcurl/curl_easy_setopt.3#L699

 http.c                  |    5 +++++
 t/lib-httpd/apache.conf |    7 +++++++
 t/t5551-http-fetch.sh   |    8 ++++++++
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index 1320c50..d10e2f0 100644
--- a/http.c
+++ b/http.c
@@ -275,6 +275,11 @@ static CURL *get_curl_handle(void)
 	}

 	curl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);
+#if LIBCURL_VERSION_NUM >= 0x071301
+	curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#elif LIBCURL_VERSION_NUM >= 0x071101
+	curl_easy_setopt(result, CURLOPT_POST301, 1);
+#endif

 	if (getenv("GIT_CURL_VERBOSE"))
 		curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 4961505..f41c7c6 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -17,6 +17,9 @@ ErrorLog error.log
 <IfModule !mod_env.c>
 	LoadModule env_module modules/mod_env.so
 </IfModule>
+<IfModule !mod_rewrite.c>
+	LoadModule rewrite_module modules/mod_rewrite.so
+</IFModule>

 Alias /dumb/ www/

@@ -36,6 +39,10 @@ ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
 	Options ExecCGI
 </Files>

+RewriteEngine on
+RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
+RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
+
 <IfDefine SSL>
 LoadModule ssl_module modules/mod_ssl.so

diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index fd19121..26d3557 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -101,5 +101,13 @@ test_expect_success 'used upload-pack service' '
 	test_cmp exp act
 '

+test_expect_success 'follow redirects (301)' '
+	git clone $HTTPD_URL/smart-redir-perm/repo.git --quiet repo-p
+'
+
+test_expect_success 'follow redirects (302)' '
+	git clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t
+'
+
 stop_httpd
 test_done
--
1.7.3.67.g2a10b

^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-09-25  4:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-18  4:33 git "smart http" server vs. http redirects Miles Bader
2010-09-18  7:03 ` Ilari Liusvaara
2010-09-18  8:16   ` Andreas Schwab
2010-09-18  8:47     ` [PATCH] smart-http: Don't change POST to GET when following redirect Andreas Schwab
2010-09-18 14:09       ` Miles Bader
2010-09-18 21:00       ` Jay Soffian
2010-09-24  6:20       ` [PATCH v2] " Tay Ray Chuan
2010-09-24 18:06         ` Andreas Schwab
2010-09-25  4:20       ` [PATCH v3] " Tay Ray Chuan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.