[PATCH] Do curl option disabling before enabling new options

[PATCH] Do curl option disabling before enabling new options

[Martin Storsjö]

This works around a bug in curl versions up to 7.19.4, where
disabling the CURLOPT_NOBODY option sets the internal state
incorrectly considering that CURLOPT_PUT was enabled earlier.

The bug is discussed at http://curl.haxx.se/bug/view.cgi?id=2727981
and is corrected in the latest version of curl in CVS.

This bug usually has no impact on git, but may surface if using
multi-pass authentication methods.

Signed-off-by: Martin Storsjo <martin@martin.st>
---
 http-push.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/http-push.c b/http-push.c
index 5138224..3629492 100644
--- a/http-push.c
+++ b/http-push.c
@@ -599,9 +599,9 @@ static void start_put(struct transfer_request *request)
 #endif
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
 
 	if (start_active_slot(slot)) {
-- 
1.6.0.2

[PATCH 1/2] Do curl option disabling before enabling new options

[Tay Ray Chuan]

From: =?ISO-8859-15?Q?Martin_Storsj=F6?= <martin@martin.st>

This works around a bug in curl versions up to 7.19.4, where
disabling the CURLOPT_NOBODY option sets the internal state
incorrectly considering that CURLOPT_PUT was enabled earlier.

The bug is discussed at http://curl.haxx.se/bug/view.cgi?id=2727981
and is corrected in the latest version of curl in CVS.

This bug usually has no impact on git, but may surface if using
multi-pass authentication methods.

Signed-off-by: Martin Storsjo <martin@martin.st>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

  Squashed in another potential trigger for this bug in remote-curl.c,
  introduced in 'next'.

 http-push.c   |    2 +-
 remote-curl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/http-push.c b/http-push.c
index f10803a..295f1fb 100644
--- a/http-push.c
+++ b/http-push.c
@@ -408,10 +408,10 @@ static void start_put(struct transfer_request *request)
 	curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, &request->buffer);
 #endif
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);

 	if (start_active_slot(slot)) {
diff --git a/remote-curl.c b/remote-curl.c
index 4f28c22..69eaf58 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -356,8 +356,8 @@ static int post_rpc(struct rpc_state *rpc)
 	slot = get_active_slot();
 	slot->results = &results;

-	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+	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, "");

--
1.6.4.4

[PATCH 0/2] http fixes

[Tay Ray Chuan]

Patch 1 ("Do curl option disabling before enabling new options"):
  This is a workaround for a fairly recent curl issue that affected
  versions up to 7.19.4.

Patch 2 ("remote-curl.c: fix rpc_out()"):
  Fixes issues that affect chunked encoding.

 http-push.c   |    2 +-
 remote-curl.c |   19 ++++++++++++-------
 2 files changed, 13 insertions(+), 8 deletions(-)

--
Cheers,
Ray Chuan

Re: [PATCH 1/2] Do curl option disabling before enabling new options

[Tay Ray Chuan]

Hi,

On Mon, Nov 23, 2009 at 11:03 AM, Tay Ray Chuan <rctay89@gmail.com> wrote:
>  Squashed in another potential trigger for this bug in remote-curl.c,
>  introduced in 'next'.

sorry, this should now read 'master'.

-- 
Cheers,
Ray Chuan

[PATCH v2 0/2] http fixes

[Tay Ray Chuan]

This series is a re-roll and is based on 'master'.

Patch 1 ("Do curl option disabling before enabling new options"):
 No changes from the previous version.

Patch 2 ("remote-curl.c: fix rpc_out()"):
 Focus solely on the extraneous semicolon.

 http-push.c   |    2 +-
 remote-curl.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

--
Cheers,
Ray Chuan

[PATCH v2 1/2] Do curl option disabling before enabling new options

[Tay Ray Chuan]

From: =?ISO-8859-15?Q?Martin_Storsj=F6?= <martin@martin.st>

This works around a bug in curl versions up to 7.19.4, where
disabling the CURLOPT_NOBODY option sets the internal state
incorrectly considering that CURLOPT_PUT was enabled earlier.

The bug is discussed at http://curl.haxx.se/bug/view.cgi?id=2727981
and is corrected in the latest version of curl in CVS.

This bug usually has no impact on git, but may surface if using
multi-pass authentication methods.

Signed-off-by: Martin Storsjo <martin@martin.st>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

  No changes from the previous version.

 http-push.c   |    2 +-
 remote-curl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/http-push.c b/http-push.c
index 0e040f8..432b20f 100644
--- a/http-push.c
+++ b/http-push.c
@@ -408,10 +408,10 @@ static void start_put(struct transfer_request *request)
 	curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, &request->buffer);
 #endif
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);

 	if (start_active_slot(slot)) {
diff --git a/remote-curl.c b/remote-curl.c
index 4f28c22..69eaf58 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -356,8 +356,8 @@ static int post_rpc(struct rpc_state *rpc)
 	slot = get_active_slot();
 	slot->results = &results;

-	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+	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, "");

--
1.6.4.4

[PATCH v2 2/2] remote-curl.c: fix rpc_out()

[Tay Ray Chuan]

Remove the extraneous semicolon (';') at the end of the if statement,
that prevented code in its block from executing.

This fixes pushing to a smart http backend with chunked encoding.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 remote-curl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 69eaf58..a331bae 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -307,7 +307,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 		rpc->len = avail;
 	}

-	if (max < avail);
+	if (max < avail)
 		avail = max;
 	memcpy(ptr, rpc->buf + rpc->pos, avail);
 	rpc->pos += avail;
--
1.6.4.4

[PATCH v3] remote-curl.c: fix rpc_out()

[Tay Ray Chuan]

Remove the extraneous semicolon (';') at the end of the if statement
that allowed the code in its block to execute regardless of the
condition.

This fixes pushing to a smart http backend with chunked encoding.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

  Reworded the part on code execution, so that it doesn't say "the code
  doesn't execute", but rather "the code always executes".

  Thanks to Paul for spotting that.

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

diff --git a/remote-curl.c b/remote-curl.c
index 69eaf58..a331bae 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -307,7 +307,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 		rpc->len = avail;
 	}

-	if (max < avail);
+	if (max < avail)
 		avail = max;
 	memcpy(ptr, rpc->buf + rpc->pos, avail);
 	rpc->pos += avail;
--
1.6.4.4

Re: [PATCH v3] remote-curl.c: fix rpc_out()

[Shawn O. Pearce]

Tay Ray Chuan <rctay89@gmail.com> wrote:
> Remove the extraneous semicolon (';') at the end of the if statement
> that allowed the code in its block to execute regardless of the
> condition.
> 
> This fixes pushing to a smart http backend with chunked encoding.
> 
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

Thanks for the fix, I can't believe I made this typo.  :-(

> -	if (max < avail);
> +	if (max < avail)
>  		avail = max;

-- 
Shawn.