[PATCH 1/2] t5540: test DAV push with authentication

[PATCH 1/2] t5540: test DAV push with authentication

[Jeff King]

We don't currently test this case at all, and instead just
test the DAV mechanism over an unauthenticated push. That
isn't very realistic, as most people will want to
authenticate pushes.

Two of the tests expect_failure as they reveal bugs:

  1. Pushing without a username in the URL fails to ask for
     credentials when we get an HTTP 401. This has always
     been the case, but it would be nice if it worked like
     smart-http.

  2. Pushing with a username fails to ask for the password
     since 986bbc0 (http: don't always prompt for password,
     2011-11-04). This is a severe regression in v1.7.8, as
     authenticated push-over-DAV is now totally unusable
     unless you have credentials in your .netrc.

Signed-off-by: Jeff King <peff@peff.net>
---
Nobody has mentioned the regression on git@vger yet, but there are two
threads already on msysgit:

  http://thread.gmane.org/gmane.comp.version-control.msysgit/14138

  http://thread.gmane.org/gmane.comp.version-control.msysgit/14161

 t/lib-httpd/apache.conf |    3 +++
 t/t5540-http-push.sh    |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0a4cdfa..3c12b05 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -92,6 +92,9 @@ SSLEngine On
 	<Location /dumb/>
 		Dav on
 	</Location>
+	<Location /auth/dumb>
+		Dav on
+	</Location>
 </IfDefine>
 
 <IfDefine SVN>
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 64767d8..3300227 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -40,6 +40,22 @@ test_expect_success 'setup remote repository' '
 	mv test_repo.git "$HTTPD_DOCUMENT_ROOT_PATH"
 '
 
+test_expect_success 'create password-protected repository' '
+	mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb" &&
+	cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
+	       "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git"
+'
+
+test_expect_success 'setup askpass helper' '
+	cat >askpass <<-\EOF &&
+	#!/bin/sh
+	echo user@host
+	EOF
+	chmod +x askpass &&
+	GIT_ASKPASS="$PWD/askpass" &&
+	export GIT_ASKPASS
+'
+
 test_expect_success 'clone remote repository' '
 	cd "$ROOT_PATH" &&
 	git clone $HTTPD_URL/dumb/test_repo.git test_repo_clone
@@ -144,6 +160,24 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
 test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
 	"$ROOT_PATH"/test_repo_clone master
 
+test_expect_failure 'push to password-protected repository (user in URL)' '
+	test_commit pw-user &&
+	git push "$HTTPD_URL_USER/auth/dumb/test_repo.git" HEAD &&
+	git rev-parse --verify HEAD >expect &&
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git" \
+		rev-parse --verify HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'push to password-protected repository (no user in URL)' '
+	test_commit pw-nouser &&
+	git push "$HTTPD_URL/auth/dumb/test_repo.git" HEAD &&
+	git rev-parse --verify HEAD >expect &&
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git" \
+		rev-parse --verify HEAD >actual &&
+	test_cmp expect actual
+'
+
 stop_httpd
 
 test_done
-- 
1.7.8.17.gfd3524

[PATCH] Revert "http: don't always prompt for password"

[Jeff King]

This reverts commit 986bbc0842334f0e07731fa37f2a55d2930a5b8c.

The rationale for that commit relied on the fact that asking
for the password up-front was merely an optimization,
because git will notice an HTTP 401 and prompt for the
password. However, that is only true for smart-http, and for
dumb fetching. Dumb push over DAV does not have this
feature; as a result, authenticated push-over-DAV does not
work at all, as it never prompts the user for a password.

Signed-off-by: Jeff King <peff@peff.net>
---
We need to deal with this regression for v1.7.8.1, I think.

There are basically three options for fixing it:

  1. Teach http-push the same retry-after-401 trick that the rest of the
     http code knows.

  2. Refactor the retry-after-401 logic from http.c into a common
     function that http-push can build on top of.

  3. Revert 986bbc08 and leave it alone; it only hurts .netrc users,
     there's a reasonable workaround (don't put the user in the URL) and
     hopefully those people will convert to using better storage via
     credential helper once it is available.

I looked at doing (1), but my first attempt[1] didn't quite work. So
it's not a huge amount of code, but it's annoyingly non-trivial. And as
a long-term solution, it's just making hack-y code hackier.

Doing (2) would be the best solution, but it's going to require some
pretty major surgery to http.c and http-push.c. I'll take a look, but if
it gets too complex, it may simply not be worth it (now that smart-http
is available, I would hope that push-over-DAV is slowly going away).

Doing (3) is obviously the easiest thing. And given the complexity of
the other two solutions, I think it makes sense to revert 986bbc08
(i.e., apply this patch), ship a working v1.7.8.1, and then look at
doing one of the other two solutions for v1.7.9.

[1] http://article.gmane.org/gmane.comp.version-control.msysgit/14153

 http.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index 008ad72..a4bc770 100644
--- a/http.c
+++ b/http.c
@@ -279,6 +279,8 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
+	init_curl_http_auth(result);
+
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
@@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
 		else if (missing_target(&results))
 			ret = HTTP_MISSING_TARGET;
 		else if (results.http_code == 401) {
-			if (user_name && user_pass) {
+			if (user_name) {
 				ret = HTTP_NOAUTH;
 			} else {
 				/*
@@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
 				 * but that is non-portable.  Using git_getpass() can at least be stubbed
 				 * on other platforms with a different implementation if/when necessary.
 				 */
-				if (!user_name)
-					user_name = xstrdup(git_getpass_with_description("Username", description));
+				user_name = xstrdup(git_getpass_with_description("Username", description));
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
-- 
1.7.8.17.gfd3524

Re: [PATCH] Revert "http: don't always prompt for password"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Doing (3) is obviously the easiest thing. And given the complexity of
> the other two solutions, I think it makes sense to revert 986bbc08
> (i.e., apply this patch), ship a working v1.7.8.1, and then look at
> doing one of the other two solutions for v1.7.9.

Or just let the "dumb HTTP" die.

I thought push over DAV has long been dead; is anybody using it for real?

Re: [PATCH] Revert "http: don't always prompt for password"

[Eric Advincula]

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

Is there an alternative to using git on windows?  I used windows, apache,
dav for git.
If there is a better solution please let me know.

Thanks
Eric

On Tue, Dec 13, 2011 at 2:09 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Jeff King <peff@peff.net> writes:
>
> > Doing (3) is obviously the easiest thing. And given the complexity of
> > the other two solutions, I think it makes sense to revert 986bbc08
> > (i.e., apply this patch), ship a working v1.7.8.1, and then look at
> > doing one of the other two solutions for v1.7.9.
>
> Or just let the "dumb HTTP" die.
>
> I thought push over DAV has long been dead; is anybody using it for real?
>

[-- Attachment #2: Type: text/html, Size: 1042 bytes --]

Re: [PATCH] Revert "http: don't always prompt for password"

[Jeff King]

On Tue, Dec 13, 2011 at 02:22:12PM -0700, Eric Advincula wrote:

> Is there an alternative to using git on windows?  I used windows, apache,
> dav for git.
> If there is a better solution please let me know.

I don't know the status of running the smart-http backend on Windows,
but that would be a preferable solution. It's way more efficient, and
the client half of the code is better maintained.

See "git help http-backend" for documentation.

-Peff

Re: [PATCH] Revert "http: don't always prompt for password"

[Jeff King]

On Tue, Dec 13, 2011 at 01:09:42PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Doing (3) is obviously the easiest thing. And given the complexity of
> > the other two solutions, I think it makes sense to revert 986bbc08
> > (i.e., apply this patch), ship a working v1.7.8.1, and then look at
> > doing one of the other two solutions for v1.7.9.
> 
> Or just let the "dumb HTTP" die.
> 
> I thought push over DAV has long been dead; is anybody using it for real?

For the record, I have no problem whatsoever with letting it die. I just
think we probably shouldn't do it accidentally during a release. ;)

-Peff

Re: [PATCH] Revert "http: don't always prompt for password"

[Jeff King]

On Tue, Dec 13, 2011 at 06:19:09PM -0500, Jeff King wrote:

> > Or just let the "dumb HTTP" die.
> > 
> > I thought push over DAV has long been dead; is anybody using it for real?
> 
> For the record, I have no problem whatsoever with letting it die. I just
> think we probably shouldn't do it accidentally during a release. ;)

BTW, one other solution, rather than reverting Stefan's patch, is to
just re-add the "unconditionally ask for a password" behavior back to
git-http-push, but not to the fetching side. Especially if we hope to
kill off git-http-push soon (after a deprecation period presumably),
then that lets it work in the meantime without hurting the other http
code. And it's really easy to do.

-Peff

Re: [PATCH] Revert "http: don't always prompt for password"

[Jeff King]

On Tue, Dec 13, 2011 at 06:20:53PM -0500, Jeff King wrote:

> BTW, one other solution, rather than reverting Stefan's patch, is to
> just re-add the "unconditionally ask for a password" behavior back to
> git-http-push, but not to the fetching side. Especially if we hope to
> kill off git-http-push soon (after a deprecation period presumably),
> then that lets it work in the meantime without hurting the other http
> code. And it's really easy to do.

And here's what that patch looks like.

Thinking on it more, this is probably better for a maint release than
reverting Stefan's patch. It un-breaks http-push, and they are no
differently off than they are with the revert. But it leaves the
enhancement in place for the smart-http code.

-- >8 --
Subject: [PATCH] http-push: enable "proactive auth"

Before commit 986bbc08, git was proactive about asking for
http passwords. It assumed that if you had a username in
your URL, you would also want a password, and asked for it
before making any http requests.

However, this could interfere with the use of .netrc (see
986bbc08 for details). And it was also unnecessary, since
the http fetching code had learned to recognize an HTTP 401
and prompt the user then. Furthermore, the proactive prompt
could interfere with the usage of .netrc (see 986bbc08 for
details).

Unfortunately, the http push-over-DAV code never learned to
recognize HTTP 401, and so was broken by this change. This
patch does a quick fix of re-enabling the "proactive auth"
strategy only for http-push, leaving the dumb http fetch and
smart-http as-is.

Signed-off-by: Jeff King <peff@peff.net>
---
 http-fetch.c         |    2 +-
 http-push.c          |    2 +-
 http.c               |    8 +++++++-
 http.h               |    3 ++-
 remote-curl.c        |    2 +-
 t/t5540-http-push.sh |    2 +-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/http-fetch.c b/http-fetch.c
index 69299b7..94d47cb 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -67,7 +67,7 @@ int main(int argc, const char **argv)
 
 	git_config(git_default_config, NULL);
 
-	http_init(NULL, url);
+	http_init(NULL, url, 0);
 	walker = get_http_walker(url);
 	walker->get_tree = get_tree;
 	walker->get_history = get_history;
diff --git a/http-push.c b/http-push.c
index 5d01be9..70283e6 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1820,7 +1820,7 @@ int main(int argc, char **argv)
 
 	memset(remote_dir_exists, -1, 256);
 
-	http_init(NULL, repo->url);
+	http_init(NULL, repo->url, 1);
 
 #ifdef USE_CURL_MULTI
 	is_running_queue = 0;
diff --git a/http.c b/http.c
index 008ad72..6c90092 100644
--- a/http.c
+++ b/http.c
@@ -43,6 +43,7 @@
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
 static char *user_name, *user_pass, *description;
+static int http_proactive_auth;
 static const char *user_agent;
 
 #if LIBCURL_VERSION_NUM >= 0x071700
@@ -279,6 +280,9 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
+	if (http_proactive_auth)
+		init_curl_http_auth(result);
+
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
@@ -367,7 +371,7 @@ static void set_from_env(const char **var, const char *envname)
 		*var = val;
 }
 
-void http_init(struct remote *remote, const char *url)
+void http_init(struct remote *remote, const char *url, int proactive_auth)
 {
 	char *low_speed_limit;
 	char *low_speed_time;
@@ -378,6 +382,8 @@ void http_init(struct remote *remote, const char *url)
 
 	curl_global_init(CURL_GLOBAL_ALL);
 
+	http_proactive_auth = proactive_auth;
+
 	if (remote && remote->http_proxy)
 		curl_http_proxy = xstrdup(remote->http_proxy);
 
diff --git a/http.h b/http.h
index 3c332a9..51f6ba7 100644
--- a/http.h
+++ b/http.h
@@ -86,7 +86,8 @@ struct buffer {
 extern void step_active_slots(void);
 #endif
 
-extern void http_init(struct remote *remote, const char *url);
+extern void http_init(struct remote *remote, const char *url,
+		      int proactive_auth);
 extern void http_cleanup(void);
 
 extern int data_received;
diff --git a/remote-curl.c b/remote-curl.c
index 0e720ee..0757b19 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -859,7 +859,7 @@ int main(int argc, const char **argv)
 
 	url = strbuf_detach(&buf, NULL);
 
-	http_init(remote, url);
+	http_init(remote, url, 0);
 
 	do {
 		if (strbuf_getline(&buf, stdin, '\n') == EOF) {
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 3300227..1eea647 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -160,7 +160,7 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
 test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
 	"$ROOT_PATH"/test_repo_clone master
 
-test_expect_failure 'push to password-protected repository (user in URL)' '
+test_expect_success 'push to password-protected repository (user in URL)' '
 	test_commit pw-user &&
 	git push "$HTTPD_URL_USER/auth/dumb/test_repo.git" HEAD &&
 	git rev-parse --verify HEAD >expect &&
-- 
1.7.8.17.gfd3524

Re: [PATCH] Revert "http: don't always prompt for password"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... This
> patch does a quick fix of re-enabling the "proactive auth"
> strategy only for http-push, leaving the dumb http fetch and
> smart-http as-is.

Yeah, I somehow like this better. Thanks.

Re: [PATCH] Revert "http: don't always prompt for password"

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> Doing (3) is obviously the easiest thing. And given the complexity of
>> the other two solutions, I think it makes sense to revert 986bbc08
>> (i.e., apply this patch), ship a working v1.7.8.1, and then look at
>> doing one of the other two solutions for v1.7.9.
>
> Or just let the "dumb HTTP" die.
>
> I thought push over DAV has long been dead; is anybody using it for real?

I am.

I'm working with people behind a firewall, hence HTTP is mandantory. My
lab has a webdav server, without Git installed on it. Being able to work
with this setup was one of the key feature of Git when I adopted it
(after a few years using GNU Arch the same way).

I could probably manage to convince my sysadmin to install Git on our
webserver, but I'd prefer if Git continues supporting my current setup.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Revert "http: don't always prompt for password"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> @@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
>  		else if (missing_target(&results))
>  			ret = HTTP_MISSING_TARGET;
>  		else if (results.http_code == 401) {
> -			if (user_name && user_pass) {
> +			if (user_name) {
>  				ret = HTTP_NOAUTH;
>  			} else {
>  				/*

In the credential series, this is where we declare the given credential is
to be rejected (if we have both username and password), or ask them to be
filled by calling credential_fill(), so I think the code in the credential
series does not need this revert. Right?

> @@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
>  				 * but that is non-portable.  Using git_getpass() can at least be stubbed
>  				 * on other platforms with a different implementation if/when necessary.
>  				 */
> -				if (!user_name)
> -					user_name = xstrdup(git_getpass_with_description("Username", description));
> +				user_name = xstrdup(git_getpass_with_description("Username", description));
>  				init_curl_http_auth(slot->curl);
>  				ret = HTTP_REAUTH;
>  			}

So is this one.

Re: [PATCH] Revert "http: don't always prompt for password"

[Jeff King]

On Tue, Dec 13, 2011 at 01:25:18PM -0800, Junio C Hamano wrote:

> > @@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
> >  		else if (missing_target(&results))
> >  			ret = HTTP_MISSING_TARGET;
> >  		else if (results.http_code == 401) {
> > -			if (user_name && user_pass) {
> > +			if (user_name) {
> >  				ret = HTTP_NOAUTH;
> >  			} else {
> >  				/*
> 
> In the credential series, this is where we declare the given credential is
> to be rejected (if we have both username and password), or ask them to be
> filled by calling credential_fill(), so I think the code in the credential
> series does not need this revert. Right?

Sort of. The point of this conditional is "did we actually send a
credential? If yes, then return HTTP_NOAUTH. Otherwise, ask for the
credential and return HTTP_REAUTH"[1].

Prior to Stefan's patch, if user_name was set, then we sent a credential
(because we always filled in the password if user_name was set). After,
we had to check both (actually, I think checking user_pass would have
been sufficient)).

The situation is the same with credentials, but the variable name is
different. Even though credential_fill will fill in both parts, we may
have set http_auth.username from the URL, but not actually called
credential_fill (and therefore not sent any credentials).

So logically, the revert in the credential series would be:

  -       if (http_auth.username && http_auth.password)
  +       if (http_auth.username)

except that I believe the former is a superset of the latter in both
cases (with or without the credential topic). So leaving it as-is would
be OK. In fact, when reverting Stefan's patch, you could just drop this
hunk entirely, which might be worth it to avoid conflicts with in-flight
topics.

[1] A much saner approach would be to always return HTTP_NOAUTH, and
    then let the caller decide to re-ask for credentials and re-try.
    But we need the magic curl slot object, which the caller doesn't
    have. So doing it that way would have taken some refactoring.

> > @@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
> >  				 * but that is non-portable.  Using git_getpass() can at least be stubbed
> >  				 * on other platforms with a different implementation if/when necessary.
> >  				 */
> > -				if (!user_name)
> > -					user_name = xstrdup(git_getpass_with_description("Username", description));
> > +				user_name = xstrdup(git_getpass_with_description("Username", description));
> >  				init_curl_http_auth(slot->curl);
> >  				ret = HTTP_REAUTH;
> >  			}
> 
> So is this one.

Yeah, this code just goes away, as credential_fill() does the right
thing.

But again, the "if (!user_name)" version post-986bbc08 handles both the
pre-986bbc08 condition (because it will always be NULL then) and the
post-986bbc08 (because we do need to check then). So I believe you could
drop the hunk entirely.

Here's a re-roll of the revert that touches as little as possible. I
believe it's correct from my analysis above, and it does pass the tests.
I also included the flipping of the "expect_failure" test, which I
forgot to put in my original patch.

-- >8 --
Subject: [PATCH] Revert "http: don't always prompt for password"

This is a partial revert of commit 986bbc0842334f0e07731fa37f2a55d2930a5b8c.

The rationale for that commit relied on the fact that asking
for the password up-front was merely an optimization,
because git will notice an HTTP 401 and prompt for the
password. However, that is only true for smart-http, and for
dumb fetching. Dumb push over DAV does not have this
feature; as a result, authenticated push-over-DAV does not
work at all, as it never prompts the user for a password.

This is a partial revert that just restores the "don't ask
for password bits". There were some other related
adjustments in 986bbc08 to handle the fact that the
user_name field might be set even if we didn't send a
credential on the first try. The new logic introduced by
986bbc08 handles both the old case and the new case, so we
can leave them intact. That will prevent unnecessary
conflicts with other in-flight topics that touch this code.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c               |    2 ++
 t/t5540-http-push.sh |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/http.c b/http.c
index 008ad72..33c63b5 100644
--- a/http.c
+++ b/http.c
@@ -279,6 +279,8 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
+	init_curl_http_auth(result);
+
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 3300227..1eea647 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -160,7 +160,7 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
 test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
 	"$ROOT_PATH"/test_repo_clone master
 
-test_expect_failure 'push to password-protected repository (user in URL)' '
+test_expect_success 'push to password-protected repository (user in URL)' '
 	test_commit pw-user &&
 	git push "$HTTPD_URL_USER/auth/dumb/test_repo.git" HEAD &&
 	git rev-parse --verify HEAD >expect &&
-- 
1.7.8.17.gfd3524

Re: [PATCH 1/2] t5540: test DAV push with authentication

[Sebastian Schuberth]

On Tue, Dec 13, 2011 at 21:17, Jeff King <peff@peff.net> wrote:

> We don't currently test this case at all, and instead just
> test the DAV mechanism over an unauthenticated push. That
> isn't very realistic, as most people will want to
> authenticate pushes.

Thanks for adding this, Peff!

-- 
Sebastian Schuberth

Re: [PATCH 1/2] t5540: test DAV push with authentication

[Jeff King]

On Tue, Dec 13, 2011 at 10:28:07PM +0100, Sebastian Schuberth wrote:

> On Tue, Dec 13, 2011 at 21:17, Jeff King <peff@peff.net> wrote:
> 
> > We don't currently test this case at all, and instead just
> > test the DAV mechanism over an unauthenticated push. That
> > isn't very realistic, as most people will want to
> > authenticate pushes.
> 
> Thanks for adding this, Peff!

You're welcome. Thank you for forwarding the bug report. I would never
have seen it on the msysgit list, and for some reason it seems that
msysgit people are more likely to use DAV.

Having looked a lot at the http code the past month or two, I knew it
was pretty flaky and I was nervous when we added Stefan's patch (and no,
I don't blame Stefan; his patch was completely reasonable, but just
happened to trigger a problem in a seldom-looked-at corner of the code).

But I hadn't looked at http-push at all until yesterday, and it didn't
even occur to me that there was another whole area of code relying in a
very obscure way on the http.c auth code. I'll take a look at some of
the refactoring I've done in http.c (both for the credentials topic as
well as the bundle topic) and see if we can't integrate http-push.c a
little more smoothly.

-Peff