[BUG] Push fails with dumb http to SabreDAV

[BUG] Push fails with dumb http to SabreDAV

[Abbaad Haider]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

Background:
I'm trying to host a git repository at my Box.com account, which
offers DAV access using a server called SabreDAV. I ran into a few
issues along the way. The first is that Box.com apparently doesn't
like it if a directory has a dot in the name. No big deal, just use
"foo" instead of "foo.git" for a bare repository. Next, it gives 404
errors when you try to GET a URL with an extraneous query string.
Solution: use GIT_SMART_HTTP=0. Authentication is handled by libcurl
automatically reading .netrc. I've gotten to the point where I can
clone a repository, but pushing changes back to origin hangs followed
by timeout. I don't know if this bug is in git or libcurl, but I think
it's the former (as explained below).

Problem (see also attachment):
When you try to push changes to the remote, it hangs, followed by
erroring out with "code 52", which is CURLE_GOT_NOTHING. Further
investigation shows that cURL successfully uses basic authentication
to GET a few files before trying a PROPFIND request on the
repository's root directory. The body of the request is in a separate
XML file asking the server about the supportedlock property.

The first time, cURL sends the request without authentication and
receives back 401 Unauthorized. The second time it tries the same
request using Basic authentication (this is the same pattern which
worked on the earlier GET requests). It gets back a 100-Continue, but
fails to upload the XML file like it did before (if you're following
along on the typescript, the verbose cURL output should say "* We are
completely uploaded and fine"). Instead it hangs for 60 seconds and
errors out with "Empty reply from server".

I don't think this bug is in libcurl, since it successfully executes
all the other commands. I will try to investigate further, but I think
it has something to do with the multi-pass authentication. Attached is a
typescript of the behavior (editorial comments are placed inside [[ ]],
and all user info has been scrubbed). In case the attachment doesn't go
through, I have also pasted the typescript to gist [1]. If anyone else
wants to reproduce the bug, Box.com does offer a free account.

[1] https://gist.githubusercontent.com/abbaad/d4a8ba1e36000eeb8e02/raw/dd5b10ae42b1936d8da33bd384c9affdb103f686/typescript.txt

--
Abbaad Haider
abbaad@gmail.com

[-- Attachment #2: typescript.txt --]
[-- Type: text/plain, Size: 6658 bytes --]

$ git --version && uname -a && curl-config --version
git version 2.0.1
Linux localhost 3.15.3-1-ARCH #1 SMP PREEMPT Tue Jul 1 07:32:45 CEST 2014 x86_64 GNU/Linux
libcurl 7.37.0
$ mkdir foo; cd foo
$ git --bare init
Initialized empty Git repository in /home/user/foo/
$ git update-server-info
$ cd ..
$ sudo mount -t davfs https://dav.box.com/dav ./box
$ cp -r foo box
$ sudo umount box
$ rm -r foo
$ cat > .netrc << EOF
> machine dav.box.com
> login [[ redacted ]]
> password [[ redacted ]]
> EOF
$ chmod 600 .netrc
$ GIT_SMART_HTTP=0 git clone https://dav.box.com/dav/foo
Cloning into 'foo'...
warning: You appear to have cloned an empty repository.
Checking connectivity... done.
$ cd foo/.git/hooks 
$ mv post-update.sample post-update        
$ cd ../..
$ touch bar
$ git add .
$ git commit -a -m "foo bar"                      
[master (root-commit) 2031b47] foo bar
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 bar
$ GIT_SMART_HTTP=0 GIT_CURL_VERBOSE=1 GIT_TRACE=1 git push origin master
trace: built-in: git 'push' 'origin' 'master'
trace: run_command: 'git-remote-https' 'origin' 'https://dav.box.com/dav/foo'
* Hostname was NOT found in DNS cache
*   Trying 74.112.185.70...
* Connected to dav.box.com (74.112.185.70) port 443 (#0)
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: none
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* Server certificate:
* 	 subject: serialNumber=jde9iuVneOEaNkFotOvLFaWx0vb5-Qpv; C=US; ST=California; L=Los Altos; O=Box, Inc.; CN=*.box.com
* 	 start date: 2014-04-07 05:14:35 GMT
* 	 expire date: 2017-10-31 20:06:48 GMT
* 	 subjectAltName: dav.box.com matched
* 	 issuer: C=US; O=GeoTrust, Inc.; CN=GeoTrust SSL CA
* 	 SSL certificate verify ok.
> GET /dav/foo/info/refs HTTP/1.1
User-Agent: git/2.0.1
Host: dav.box.com
Accept: */*
Accept-Encoding: gzip
Pragma: no-cache

< HTTP/1.1 401 Unauthorized
* Server nginx is not blacklisted
< Server: nginx
< Date: Fri, 04 Jul 2014 23:34:18 GMT
< Content-Type: application/xml; charset=utf-8
< Content-Length: 242
< Connection: keep-alive
< Vary: Host
< WWW-Authenticate: Basic realm="dav.box.com"
< 
* Ignoring the response-body
* Connection #0 to host dav.box.com left intact
* Issue another request to this URL: 'https://dav.box.com/dav/foo/info/refs'
* Found bundle for host dav.box.com: 0x123ff40
* Re-using existing connection! (#0) with host dav.box.com
* Connected to dav.box.com (74.112.185.70) port 443 (#0)
* Server auth using Basic with user '[[ redacted ]]'
> GET /dav/foo/info/refs HTTP/1.1
Authorization: Basic [[ redacted ]]
User-Agent: git/2.0.1
Host: dav.box.com
Accept: */*
Accept-Encoding: gzip
Pragma: no-cache

< HTTP/1.1 200 OK
* Server nginx is not blacklisted
< Server: nginx
< Date: Fri, 04 Jul 2014 23:34:19 GMT
< Content-Type: application/octet-stream
< Content-Length: 0
< Connection: keep-alive
< Vary: Host
< Cache-control: private
< Accept-Ranges: bytes
< Content-Disposition: attachment;filename="refs";filename*=UTF-8''refs
< X-Content-Type-Options: nosniff
< Last-Modified: Fri, 04 Jul 2014 23:23:51 GMT
< ETag: "da39a3ee5e6b4b0d3255bfef95601890afd80709"
< Accept-Ranges: bytes
< 
* Connection #0 to host dav.box.com left intact
* Found bundle for host dav.box.com: 0x123ff40
* Re-using existing connection! (#0) with host dav.box.com
* Connected to dav.box.com (74.112.185.70) port 443 (#0)
* Server auth using Basic with user '[[ redacted ]]'
> GET /dav/foo/HEAD HTTP/1.1
Authorization: Basic [[ redacted ]]
User-Agent: git/2.0.1
Host: dav.box.com
Accept: */*
Accept-Encoding: gzip
Pragma: no-cache

< HTTP/1.1 200 OK
* Server nginx is not blacklisted
< Server: nginx
< Date: Fri, 04 Jul 2014 23:34:20 GMT
< Content-Type: application/octet-stream
< Content-Length: 23
< Connection: keep-alive
< Vary: Host
< Cache-control: private
< Accept-Ranges: bytes
< Content-Disposition: attachment;filename="HEAD";filename*=UTF-8''HEAD
< X-Content-Type-Options: nosniff
< Last-Modified: Fri, 04 Jul 2014 23:24:12 GMT
< ETag: "acbaef275e46a7f14c1ef456fff2c8bbe8c84724"
< Accept-Ranges: bytes
< 
* Connection #0 to host dav.box.com left intact
trace: run_command: 'http-push' '--helper-status' 'https://dav.box.com/dav/foo/' 'refs/heads/master:refs/heads/master'
trace: exec: 'git' 'http-push' '--helper-status' 'https://dav.box.com/dav/foo/' 'refs/heads/master:refs/heads/master'
trace: exec: 'git-http-push' '--helper-status' 'https://dav.box.com/dav/foo/' 'refs/heads/master:refs/heads/master'
trace: run_command: 'git-http-push' '--helper-status' 'https://dav.box.com/dav/foo/' 'refs/heads/master:refs/heads/master'
* Hostname was NOT found in DNS cache
*   Trying 74.112.184.70...
* Connected to dav.box.com (74.112.184.70) port 443 (#0)
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: none
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* Server certificate:
* 	 subject: serialNumber=jde9iuVneOEaNkFotOvLFaWx0vb5-Qpv; C=US; ST=California; L=Los Altos; O=Box, Inc.; CN=*.box.com
* 	 start date: 2014-04-07 05:14:35 GMT
* 	 expire date: 2017-10-31 20:06:48 GMT
* 	 subjectAltName: dav.box.com matched
* 	 issuer: C=US; O=GeoTrust, Inc.; CN=GeoTrust SSL CA
* 	 SSL certificate verify ok.
> PROPFIND /dav/foo/ HTTP/1.1
User-Agent: git/2.0.1
Host: dav.box.com
Accept: */*
Depth: 0
Content-Type: text/xml
Content-Length: 158
Expect: 100-continue

< HTTP/1.1 100 Continue
* We are completely uploaded and fine
< HTTP/1.1 401 Unauthorized
* Server nginx is not blacklisted
< Server: nginx
< Date: Fri, 04 Jul 2014 23:34:21 GMT
< Content-Type: application/xml; charset=utf-8
< Content-Length: 242
< Connection: keep-alive
< Vary: Host
< WWW-Authenticate: Basic realm="dav.box.com"
* the ioctl callback returned 0
< 
* Ignoring the response-body
* Connection #0 to host dav.box.com left intact
* Issue another request to this URL: 'https://dav.box.com/dav/foo/'
* Found bundle for host dav.box.com: 0x20469c0
* Re-using existing connection! (#0) with host dav.box.com
* Connected to dav.box.com (74.112.184.70) port 443 (#0)
* Server auth using Basic with user '[[ redacted ]]'
> PROPFIND /dav/foo/ HTTP/1.1
Authorization: Basic [[ redacted ]]
User-Agent: git/2.0.1
Host: dav.box.com
Accept: */*
Depth: 0
Content-Type: text/xml
Content-Length: 158
Expect: 100-continue

< HTTP/1.1 100 Continue
[[ This is where it hangs for about 60 seconds ]]
* Empty reply from server
* Connection #0 to host dav.box.com left intact
error: Cannot access URL https://dav.box.com/dav/foo/, return code 52
fatal: git-http-push failed
error: failed to push some refs to 'https://dav.box.com/dav/foo'

[PATCH] http-push.c: make CURLOPT_IOCTLDATA a usable pointer

[Abbaad Haider]

Fixes a small bug affecting push to remotes which use some sort of
multi-pass authentication. In particular the bug affected SabreDAV as
configured by Box.com [1].

It must be a weird server configuration for the bug to have survived
this long. Someone should write a test for it.

[1] http://marc.info/?l=git&m=140460482604482

Signed-off-by: Abbaad Haider <abbaad@gmail.com>
---
 http-push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/http-push.c b/http-push.c
index f2c56c8..bd42895 100644
--- a/http-push.c
+++ b/http-push.c
@@ -199,7 +199,7 @@ static void curl_setup_http(CURL *curl, const char *url,
 	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
 #ifndef NO_CURL_IOCTL
 	curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
-	curl_easy_setopt(curl, CURLOPT_IOCTLDATA, &buffer);
+	curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
 #endif
 	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
 	curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
-- 
2.0.1

Re: [PATCH] http-push.c: make CURLOPT_IOCTLDATA a usable pointer

[Jeff King]

On Sat, Jul 05, 2014 at 08:43:48PM -0400, Abbaad Haider wrote:

> Fixes a small bug affecting push to remotes which use some sort of
> multi-pass authentication. In particular the bug affected SabreDAV as
> configured by Box.com [1].

Thanks. This looks like it was caused by the refactor in ebaaf31
(http-push: refactor curl_easy_setup madness, 2011-05-03), which moved
the curl_easy call into a sub-function which took the buffer as a
pointer, rather than accessing it as a local variable.

> It must be a weird server configuration for the bug to have survived
> this long. Someone should write a test for it.

I think both dumb-http push-over-DAV and multi-pass authentication are
rare, so finding a combination of the two took a while. I do not know
enough about the server setup to know whether we could replicate this in
our test apache setup (and nor do I particularly want to spend a lot of
time figuring it out for the sake of testing push-over-DAV).

> diff --git a/http-push.c b/http-push.c
> index f2c56c8..bd42895 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -199,7 +199,7 @@ static void curl_setup_http(CURL *curl, const char *url,
>  	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
>  #ifndef NO_CURL_IOCTL
>  	curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
> -	curl_easy_setopt(curl, CURLOPT_IOCTLDATA, &buffer);
> +	curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);

Patch looks obviously correct.