{"thread":{"id":"25138","subject":"git \"smart http\" server vs. http redirects","startedAt":"2010-09-18T04:33:48Z","lastAt":"2010-09-25T04:20:35Z","messageCount":9,"participants":["Miles Bader","Ilari Liusvaara","Andreas Schwab","Jay Soffian","Tay Ray Chuan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"150922","messageId":"877hijvff7.fsf@catnip.gol.com","threadId":"25138","inReplyTo":null,"subject":"git \"smart http\" server vs. http redirects","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-09-18T04:33:48Z","receivedAt":"2010-09-18T04:33:48Z","isPatch":false,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"The savannah.gnu.org admins are trying out the git \"smart http\" server,\nbut it doesn't seem to handle http redirects properly... should it?\nIs this a bug with the git server?\n\nThe follow is from Sylvain Beucler;\n> Hmmm, actually it works, but it doesn't support a HTTP redirect\n> (namely git.sv.gnu.org -> git.savannah.gnu.org).\n>\n> $ git clone http://git.sv.gnu.org/r/freedink.git\n> Initialized empty Git repository in /tmp/freedink/.git/\n> error: RPC failed; result=22, HTTP code = 405\n> ^C\n>\n> $ git clone http://git.savannah.gnu.org/r/freedink.git\n> Initialized empty Git repository in /tmp/freedink/.git/\n> remote: Counting objects: 5528, done.\n> remote: Compressing objects: 11% (174/1578)\n\nThis is the response I get from git.sv.gnu.org manually:\n\n $ telnet git.sv.gnu.org http\n Trying 140.186.70.72...\n Connected to git.sv.gnu.org.\n Escape character is '^]'.\n GET /r/freedink.git HTTP/1.1\n Host: git.sv.gnu.org\n\n HTTP/1.1 301 Moved Permanently\n Date: Fri, 17 Sep 2010 06:03:27 GMT\n 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\n Location: http://git.savannah.gnu.org/r/freedink.git\n Content-Length: 389\n Content-Type: elided-to-get-past-vger's-filter; charset=iso-8859-1\n\n \n \n 301 Moved Permanently\n \n
\n \n 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\n \n Connection closed by foreign host.\n\nThanks,\n\n-Miles\n\n-- \nXML is like violence. If it doesn't solve your problem, you're not\nusing enough of it.\n"},{"id":"150926","messageId":"20100918070315.GA30872@LK-Perkele-V2.elisa-laajakaista.fi","threadId":"25138","inReplyTo":"877hijvff7.fsf@catnip.gol.com","subject":"Re: git \"smart http\" server vs. http redirects","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-09-18T07:03:15Z","receivedAt":"2010-09-18T07:03:15Z","isPatch":false,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:\n\n> The savannah.gnu.org admins are trying out the git \"smart http\" server,\n> but it doesn't seem to handle http redirects properly... should it?\n> Is this a bug with the git server?\n> \n> The follow is from Sylvain Beucler;\n> > Hmmm, actually it works, but it doesn't support a HTTP redirect\n> > (namely git.sv.gnu.org -> git.savannah.gnu.org).\n> >\n> > $ git clone http://git.sv.gnu.org/r/freedink.git\n> > Initialized empty Git repository in /tmp/freedink/.git/\n> > error: RPC failed; result=22, HTTP code = 405\n\n405 (Method not allowed)? Can you see what request it is trying to send\nand to where (the request that fails with 405 that is)?\n\n-Ilari\n"},{"id":"150930","messageId":"m262y3cvpy.fsf@whitebox.home","threadId":"25138","inReplyTo":"20100918070315.GA30872@LK-Perkele-V2.elisa-laajakaista.fi","subject":"Re: git \"smart http\" server vs. http redirects","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-09-18T08:16:41Z","receivedAt":"2010-09-18T08:16:41Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ilari Liusvaara writes:\n\n> On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:\n>\n>> The savannah.gnu.org admins are trying out the git \"smart http\" server,\n>> but it doesn't seem to handle http redirects properly... should it?\n>> Is this a bug with the git server?\n>> \n>> The follow is from Sylvain Beucler;\n>> > Hmmm, actually it works, but it doesn't support a HTTP redirect\n>> > (namely git.sv.gnu.org -> git.savannah.gnu.org).\n>> >\n>> > $ git clone http://git.sv.gnu.org/r/freedink.git\n>> > Initialized empty Git repository in /tmp/freedink/.git/\n>> > error: RPC failed; result=22, HTTP code = 405\n>\n> 405 (Method not allowed)? Can you see what request it is trying to send\n> and to where (the request that fails with 405 that is)?\n\nI think this is a bug in the apache setup at savannnah. It is\nresponding to POST .../git-upload-pack with both 301 and 405. This does\nnot happen if you use the redirected address directly.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"150933","messageId":"m21v8rcua1.fsf_-_@whitebox.home","threadId":"25138","inReplyTo":"m262y3cvpy.fsf@whitebox.home","subject":"[PATCH] smart-http: Don't change POST to GET when following redirect","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-09-18T08:47:50Z","receivedAt":"2010-09-18T08:47:50Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"When the remote HTTP server returns a redirect the default libcurl action\nis to change a POST request into a GET request while following the\nredirect, but the remote http backend does not expect that. Tell libcurl\nto always keep the POST request.\n\nSigned-off-by: Andreas Schwab \n---\nAndreas Schwab writes:\n\n> Ilari Liusvaara writes:\n>\n>> On Sat, Sep 18, 2010 at 01:33:48PM +0900, Miles Bader wrote:\n>>\n>>> The savannah.gnu.org admins are trying out the git \"smart http\" server,\n>>> but it doesn't seem to handle http redirects properly... should it?\n>>> Is this a bug with the git server?\n>>> \n>>> The follow is from Sylvain Beucler;\n>>> > Hmmm, actually it works, but it doesn't support a HTTP redirect\n>>> > (namely git.sv.gnu.org -> git.savannah.gnu.org).\n>>> >\n>>> > $ git clone http://git.sv.gnu.org/r/freedink.git\n>>> > Initialized empty Git repository in /tmp/freedink/.git/\n>>> > error: RPC failed; result=22, HTTP code = 405\n>>\n>> 405 (Method not allowed)? Can you see what request it is trying to send\n>> and to where (the request that fails with 405 that is)?\n>\n> I think this is a bug in the apache setup at savannnah. It is\n> responding to POST .../git-upload-pack with both 301 and 405. This does\n> not happen if you use the redirected address directly.\n\nLooking closer, this is actually a bug in git. It does not set the\nPOSTREDIR option, so that libcurl changes the POST into GET when\nfollowing a redirection.\n\nAndreas.\n\n---\n remote-curl.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 04d4813..309b024 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -393,6 +393,7 @@ static int post_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n-- \n1.7.2.3\n\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"150948","messageId":"87vd639m9c.fsf@catnip.gol.com","threadId":"25138","inReplyTo":"m21v8rcua1.fsf_-_@whitebox.home","subject":"Re: [PATCH] smart-http: Don't change POST to GET when following redirect","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-09-18T14:09:19Z","receivedAt":"2010-09-18T14:09:19Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Thanks Andreas!\n\n-miles\n\n-- \nAcquaintance, n. A person whom we know well enough to borrow from, but not\nwell enough to lend to.\n"},{"id":"150968","messageId":"AANLkTimwkXTs==+zT=Ue3jFNyRLL+7A1FFhoDuF-5zZ3@mail.gmail.com","threadId":"25138","inReplyTo":"m21v8rcua1.fsf_-_@whitebox.home","subject":"Re: [PATCH] smart-http: Don't change POST to GET when following redirect","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-09-18T21:00:25Z","receivedAt":"2010-09-18T21:00:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Sep 18, 2010 at 4:47 AM, Andreas Schwab wrote:\n> + curl_easy_setopt(slot->curl, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n\nNot sure what git's minimum supported libcurl is, but this define was\nadded in 7.19.1 (November 5 2008).\n\nj.\n"},{"id":"151423","messageId":"1285309223-4348-1-git-send-email-rctay89@gmail.com","threadId":"25138","inReplyTo":"AANLkTimwkXTs==+zT=Ue3jFNyRLL+7A1FFhoDuF-5zZ3@mail.gmail.com","subject":"[PATCH v2] smart-http: Don't change POST to GET when following redirect","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-09-24T06:20:23Z","receivedAt":"2010-09-24T06:20:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From: Andreas Schwab \n\nFor a long time (29508e1 \"Isolate shared HTTP request functionality\", Fri\nNov 18 11:02:58 2005), we've followed HTTP redirects with\nCURLOPT_FOLLOWLOCATION.\n\nHowever, when the remote HTTP server returns a redirect the default\nlibcurl action is to change a POST request into a GET request while\nfollowing the redirect, but the remote http backend does not expect\nthat.\n\nFix this by telling libcurl to always keep the request as type POST with\nCURLOPT_POSTREDIR.\n\nSigned-off-by: Andreas Schwab \nSigned-off-by: Tay Ray Chuan \n---\n\nAdded simple tests and made some changes to the patch message.\n\nAndreas:\n shifted the setopt to right after FOLLOWLOCATION, since they're linked\n closely.\n\nJay: added the usual hexadecimal version checks.\n\n http.c | 3 +++\n t/lib-httpd/apache.conf | 7 +++++++\n t/t5551-http-fetch.sh | 8 ++++++++\n 3 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 1320c50..25f8b45 100644\n--- a/http.c\n+++ b/http.c\n@@ -275,6 +275,9 @@ static CURL *get_curl_handle(void)\n \t}\n\n \tcurl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);\n+#if LIBCURL_VERSION_NUM >= 0x071301\n+\tcurl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n+#endif\n\n \tif (getenv(\"GIT_CURL_VERBOSE\"))\n \t\tcurl_easy_setopt(result, CURLOPT_VERBOSE, 1);\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 4961505..f41c7c6 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -17,6 +17,9 @@ ErrorLog error.log\n \n \tLoadModule env_module modules/mod_env.so\n \n+\n+\tLoadModule rewrite_module modules/mod_rewrite.so\n+\n\n Alias /dumb/ www/\n\n@@ -36,6 +39,10 @@ ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/\n \tOptions ExecCGI\n \n\n+RewriteEngine on\n+RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]\n+RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]\n+\n \n LoadModule ssl_module modules/mod_ssl.so\n\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex fd19121..26d3557 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -101,5 +101,13 @@ test_expect_success 'used upload-pack service' '\n \ttest_cmp exp act\n '\n\n+test_expect_success 'follow redirects (301)' '\n+\tgit clone $HTTPD_URL/smart-redir-perm/repo.git --quiet repo-p\n+'\n+\n+test_expect_success 'follow redirects (302)' '\n+\tgit clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t\n+'\n+\n stop_httpd\n test_done\n--\n1.7.3.67.g2a10b\n"},{"id":"151518","messageId":"m21v8jaudw.fsf@igel.home","threadId":"25138","inReplyTo":"1285309223-4348-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2] smart-http: Don't change POST to GET when following redirect","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-09-24T18:06:35Z","receivedAt":"2010-09-24T18:06:35Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Tay Ray Chuan writes:\n\n> diff --git a/http.c b/http.c\n> index 1320c50..25f8b45 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -275,6 +275,9 @@ static CURL *get_curl_handle(void)\n> \t}\n>\n> \tcurl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);\n> +#if LIBCURL_VERSION_NUM >= 0x071301\n> +\tcurl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n> +#endif\n\nShould this fall back to CURLOPT_POST301 on older versions? (Those\nwon't handle 302 though, I think.)\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"151605","messageId":"1285388435-5336-1-git-send-email-rctay89@gmail.com","threadId":"25138","inReplyTo":"m21v8jaudw.fsf@igel.home","subject":"[PATCH v3] smart-http: Don't change POST to GET when following redirect","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-09-25T04:20:35Z","receivedAt":"2010-09-25T04:20:35Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"For a long time (29508e1 \"Isolate shared HTTP request functionality\", Fri\nNov 18 11:02:58 2005), we've followed HTTP redirects with\nCURLOPT_FOLLOWLOCATION.\n\nHowever, when the remote HTTP server returns a redirect the default\nlibcurl action is to change a POST request into a GET request while\nfollowing the redirect, but the remote http backend does not expect\nthat.\n\nFix this by telling libcurl to always keep the request as type POST with\nCURLOPT_POSTREDIR.\n\nFor users of libcurl older than 7.19.1, use CURLOPT_POST301 instead,\nwhich only follows 301s instead of both 301s and 302s.\n\nSigned-off-by: Andreas Schwab \nSigned-off-by: Tay Ray Chuan \n---\n\nAndreas: added CURLOPT_POST301 alternative. There doesn't seem to be a pretty\n constant, the docs just say a non-zero value [1] or 1 [2].\n\n[1] http://github.com/bagder/curl/blob/curl-7_17_1/docs/libcurl/curl_easy_setopt.3#L646\n[2] http://github.com/bagder/curl/blob/curl-7_19_0/docs/libcurl/curl_easy_setopt.3#L699\n\n http.c | 5 +++++\n t/lib-httpd/apache.conf | 7 +++++++\n t/t5551-http-fetch.sh | 8 ++++++++\n 3 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 1320c50..d10e2f0 100644\n--- a/http.c\n+++ b/http.c\n@@ -275,6 +275,11 @@ static CURL *get_curl_handle(void)\n \t}\n\n \tcurl_easy_setopt(result, CURLOPT_FOLLOWLOCATION, 1);\n+#if LIBCURL_VERSION_NUM >= 0x071301\n+\tcurl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n+#elif LIBCURL_VERSION_NUM >= 0x071101\n+\tcurl_easy_setopt(result, CURLOPT_POST301, 1);\n+#endif\n\n \tif (getenv(\"GIT_CURL_VERBOSE\"))\n \t\tcurl_easy_setopt(result, CURLOPT_VERBOSE, 1);\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 4961505..f41c7c6 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -17,6 +17,9 @@ ErrorLog error.log\n \n \tLoadModule env_module modules/mod_env.so\n \n+\n+\tLoadModule rewrite_module modules/mod_rewrite.so\n+\n\n Alias /dumb/ www/\n\n@@ -36,6 +39,10 @@ ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/\n \tOptions ExecCGI\n \n\n+RewriteEngine on\n+RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]\n+RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]\n+\n \n LoadModule ssl_module modules/mod_ssl.so\n\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex fd19121..26d3557 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -101,5 +101,13 @@ test_expect_success 'used upload-pack service' '\n \ttest_cmp exp act\n '\n\n+test_expect_success 'follow redirects (301)' '\n+\tgit clone $HTTPD_URL/smart-redir-perm/repo.git --quiet repo-p\n+'\n+\n+test_expect_success 'follow redirects (302)' '\n+\tgit clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t\n+'\n+\n stop_httpd\n test_done\n--\n1.7.3.67.g2a10b\n"}]}