Merge branch 'jk/http-auth'

* jk/http-auth:
http_init: accept separate URL parameter
http: use hostname in credential description
http: retry authentication failures for all http requests
remote-curl: don't retry auth failures with dumb protocol
improve httpd auth tests
url: decode buffers that are not NUL-terminated

diff --git a/http-fetch.c b/http-fetch.c
index 8c4c5d2224a2493a648e6a34257bc150f2712dd0..69299b7bd2a956266bf581df9c23589a97fca805 100644 (file)
--- 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);
+       http_init(NULL, url);
        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 44f814cda2a9756a55cb7f332d5fef0e5484256b..5d01be93440cdb343379fe0b05ff9155727f125c 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -1747,7 +1747,6 @@ int main(int argc, char **argv)
        int i;
        int new_refs;
        struct ref *ref, *local_refs;
-       struct remote *remote;
 
        git_extract_argv0_path(argv[0]);
 
@@ -1821,14 +1820,7 @@ int main(int argc, char **argv)
 
        memset(remote_dir_exists, -1, 256);
 
-       /*
-        * Create a minimum remote by hand to give to http_init(),
-        * primarily to allow it to look at the URL.
-        */
-       remote = xcalloc(sizeof(*remote), 1);
-       ALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);
-       remote->url[remote->url_nr++] = repo->url;
-       http_init(remote);
+       http_init(NULL, repo->url);
 
 #ifdef USE_CURL_MULTI
        is_running_queue = 0;

diff --git a/http.c b/http.c
index fb3465f50c95ab9cb3e6fae1d1e594f6ae0b9c42..a4bc770e2d6196958ec5b795ca89be24be182a34 100644 (file)
--- a/http.c
+++ b/http.c
@@ -42,7 +42,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
-static char *user_name, *user_pass;
+static char *user_name, *user_pass, *description;
 static const char *user_agent;
 
 #if LIBCURL_VERSION_NUM >= 0x071700
@@ -139,6 +139,27 @@ static void process_curl_messages(void)
 }
 #endif
 
+static char *git_getpass_with_description(const char *what, const char *desc)
+{
+       struct strbuf prompt = STRBUF_INIT;
+       char *r;
+
+       if (desc)
+               strbuf_addf(&prompt, "%s for '%s': ", what, desc);
+       else
+               strbuf_addf(&prompt, "%s: ", what);
+       /*
+        * NEEDSWORK: for usernames, we should do something less magical that
+        * actually echoes the characters. However, we need to read from
+        * /dev/tty and not stdio, which is not portable (but getpass will do
+        * it for us). http.c uses the same workaround.
+        */
+       r = git_getpass(prompt.buf);
+
+       strbuf_release(&prompt);
+       return xstrdup(r);
+}
+
 static int http_options(const char *var, const char *value, void *cb)
 {
        if (!strcmp("http.sslverify", var)) {
@@ -214,7 +235,7 @@ static void init_curl_http_auth(CURL *result)
        if (user_name) {
                struct strbuf up = STRBUF_INIT;
                if (!user_pass)
-                       user_pass = xstrdup(git_getpass("Password: "));
+                       user_pass = xstrdup(git_getpass_with_description("Password", description));
                strbuf_addf(&up, "%s:%s", user_name, user_pass);
                curl_easy_setopt(result, CURLOPT_USERPWD,
                                 strbuf_detach(&up, NULL));
@@ -229,7 +250,7 @@ static int has_cert_password(void)
                return 0;
        /* Only prompt the user once. */
        ssl_cert_password_required = -1;
-       ssl_cert_password = git_getpass("Certificate Password: ");
+       ssl_cert_password = git_getpass_with_description("Certificate Password", description);
        if (ssl_cert_password != NULL) {
                ssl_cert_password = xstrdup(ssl_cert_password);
                return 1;
@@ -307,8 +328,7 @@ static CURL *get_curl_handle(void)
 
 static void http_auth_init(const char *url)
 {
-       char *at, *colon, *cp, *slash, *decoded;
-       int len;
+       const char *at, *colon, *cp, *slash, *host;
 
        cp = strstr(url, "://");
        if (!cp)
@@ -324,34 +344,22 @@ static void http_auth_init(const char *url)
        at = strchr(cp, '@');
        colon = strchr(cp, ':');
        slash = strchrnul(cp, '/');
-       if (!at || slash <= at)
-               return; /* No credentials */
-       if (!colon || at <= colon) {
+       if (!at || slash <= at) {
+               /* No credentials, but we may have to ask for some later */
+               host = cp;
+       }
+       else if (!colon || at <= colon) {
                /* Only username */
-               len = at - cp;
-               user_name = xmalloc(len + 1);
-               memcpy(user_name, cp, len);
-               user_name[len] = '\0';
-               decoded = url_decode(user_name);
-               free(user_name);
-               user_name = decoded;
+               user_name = url_decode_mem(cp, at - cp);
                user_pass = NULL;
+               host = at + 1;
        } else {
-               len = colon - cp;
-               user_name = xmalloc(len + 1);
-               memcpy(user_name, cp, len);
-               user_name[len] = '\0';
-               decoded = url_decode(user_name);
-               free(user_name);
-               user_name = decoded;
-               len = at - (colon + 1);
-               user_pass = xmalloc(len + 1);
-               memcpy(user_pass, colon + 1, len);
-               user_pass[len] = '\0';
-               decoded = url_decode(user_pass);
-               free(user_pass);
-               user_pass = decoded;
+               user_name = url_decode_mem(cp, colon - cp);
+               user_pass = url_decode_mem(colon + 1, at - (colon + 1));
+               host = at + 1;
        }
+
+       description = url_decode_mem(host, slash - host);
 }
 
 static void set_from_env(const char **var, const char *envname)
@@ -361,7 +369,7 @@ static void set_from_env(const char **var, const char *envname)
                *var = val;
 }
 
-void http_init(struct remote *remote)
+void http_init(struct remote *remote, const char *url)
 {
        char *low_speed_limit;
        char *low_speed_time;
@@ -425,11 +433,11 @@ void http_init(struct remote *remote)
        if (getenv("GIT_CURL_FTP_NO_EPSV"))
                curl_ftp_no_epsv = 1;
 
-       if (remote && remote->url && remote->url[0]) {
-               http_auth_init(remote->url[0]);
+       if (url) {
+               http_auth_init(url);
                if (!ssl_cert_password_required &&
                    getenv("GIT_SSL_CERT_PASSWORD_PROTECTED") &&
-                   !prefixcmp(remote->url[0], "https://"))
+                   !prefixcmp(url, "https://"))
                        ssl_cert_password_required = 1;
        }
 
@@ -847,7 +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.
                                 */
-                               user_name = xstrdup(git_getpass("Username: "));
+                               user_name = xstrdup(git_getpass_with_description("Username", description));
                                init_curl_http_auth(slot->curl);
                                ret = HTTP_REAUTH;
                        }
@@ -870,13 +878,18 @@ static int http_request(const char *url, void *result, int target, int options)
        return ret;
 }
 
+static int http_request_reauth(const char *url, void *result, int target,
+                              int options)
+{
+       int ret = http_request(url, result, target, options);
+       if (ret != HTTP_REAUTH)
+               return ret;
+       return http_request(url, result, target, options);
+}
+
 int http_get_strbuf(const char *url, struct strbuf *result, int options)
 {
-       int http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);
-       if (http_ret == HTTP_REAUTH) {
-               http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);
-       }
-       return http_ret;
+       return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
 }
 
 /*
@@ -899,7 +912,7 @@ static int http_get_file(const char *url, const char *filename, int options)
                goto cleanup;
        }
 
-       ret = http_request(url, result, HTTP_REQUEST_FILE, options);
+       ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
        fclose(result);
 
        if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))

diff --git a/http.h b/http.h
index 0bf8592dc45209c9be5b8ddac3a53f6fc11e29be..3c332a98e9358a296b937453351018ac1042120e 100644 (file)
--- a/http.h
+++ b/http.h
@@ -86,7 +86,7 @@ extern void add_fill_function(void *data, int (*fill)(void *));
 extern void step_active_slots(void);
 #endif
 
-extern void http_init(struct remote *remote);
+extern void http_init(struct remote *remote, const char *url);
 extern void http_cleanup(void);
 
 extern int data_received;

diff --git a/remote-curl.c b/remote-curl.c
index 0aa4bfed309d6c439fac4ff2a0df6a468307e7bf..0e720ee8bbf4cbc6a50336a1f1c93bfc63842fe3 100644 (file)
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -115,7 +115,7 @@ static struct discovery* discover_refs(const char *service)
        http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
 
        /* try again with "plain" url (no ? or & appended) */
-       if (http_ret != HTTP_OK) {
+       if (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {
                free(refs_url);
                strbuf_reset(&buffer);
 
@@ -859,7 +859,7 @@ int main(int argc, const char **argv)
 
        url = strbuf_detach(&buf, NULL);
 
-       http_init(remote);
+       http_init(remote, url);
 
        do {
                if (strbuf_getline(&buf, stdin, '\n') == EOF) {

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index b8996a373a7444b54823f36159ea1a8634e4aeee..f7dc0781d5d0ec5afd7f1d0898bffa17a9b1b00e 100644 (file)
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -81,8 +81,7 @@ prepare_httpd() {
 
        if test -n "$LIB_HTTPD_SSL"
        then
-               HTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT
-               AUTH_HTTPD_URL=https://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_PROTO=https
 
                RANDFILE_PATH="$HTTPD_ROOT_PATH"/.rnd openssl req \
                        -config "$TEST_PATH/ssl.cnf" \
@@ -93,9 +92,12 @@ prepare_httpd() {
                export GIT_SSL_NO_VERIFY
                HTTPD_PARA="$HTTPD_PARA -DSSL"
        else
-               HTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT
-               AUTH_HTTPD_URL=http://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_PROTO=http
        fi
+       HTTPD_DEST=127.0.0.1:$LIB_HTTPD_PORT
+       HTTPD_URL=$HTTPD_PROTO://$HTTPD_DEST
+       HTTPD_URL_USER=$HTTPD_PROTO://user%40host@$HTTPD_DEST
+       HTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:user%40host@$HTTPD_DEST
 
        if test -n "$LIB_HTTPD_DAV" -o -n "$LIB_HTTPD_SVN"
        then

diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index a1883ca6b649a236e5e08a5e4b76d1479353dcac..d1ab4d0e4c8e0c669a88c2a52a6592643a3a99c6 100755 (executable)
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -35,11 +35,54 @@ test_expect_success 'clone http repository' '
        test_cmp file clone/file
 '
 
-test_expect_success 'clone http repository with authentication' '
+test_expect_success 'create password-protected repository' '
        mkdir "$HTTPD_DOCUMENT_ROOT_PATH/auth/" &&
-       cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" "$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git" &&
-       git clone $AUTH_HTTPD_URL/auth/repo.git clone-auth &&
-       test_cmp file clone-auth/file
+       cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+              "$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git"
+'
+
+test_expect_success 'setup askpass helpers' '
+       cat >askpass <<-EOF &&
+       #!/bin/sh
+       echo >>"$PWD/askpass-query" "askpass: \$*" &&
+       cat "$PWD/askpass-response"
+       EOF
+       chmod +x askpass &&
+       GIT_ASKPASS="$PWD/askpass" &&
+       export GIT_ASKPASS &&
+       >askpass-expect-none &&
+       echo "askpass: Password for '\''$HTTPD_DEST'\'': " >askpass-expect-pass &&
+       { echo "askpass: Username for '\''$HTTPD_DEST'\'': " &&
+         cat askpass-expect-pass
+       } >askpass-expect-both
+'
+
+test_expect_success 'cloning password-protected repository can fail' '
+       >askpass-query &&
+       echo wrong >askpass-response &&
+       test_must_fail git clone "$HTTPD_URL/auth/repo.git" clone-auth-fail &&
+       test_cmp askpass-expect-both askpass-query
+'
+
+test_expect_success 'http auth can use user/pass in URL' '
+       >askpass-query &&
+       echo wrong >askpass-reponse &&
+       git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
+       test_cmp askpass-expect-none askpass-query
+'
+
+test_expect_success 'http auth can use just user in URL' '
+       >askpass-query &&
+       echo user@host >askpass-response &&
+       git clone "$HTTPD_URL_USER/auth/repo.git" clone-auth-pass &&
+       test_cmp askpass-expect-pass askpass-query
+'
+
+test_expect_success 'http auth can request both user and pass' '
+       >askpass-query &&
+       echo user@host >askpass-response &&
+       git clone "$HTTPD_URL/auth/repo.git" clone-auth-both &&
+       test_cmp askpass-expect-both askpass-query
 '
 
 test_expect_success 'fetch changes via http' '

diff --git a/url.c b/url.c
index e4262a0d7a9ef71924b117f4cbf9fe12d0239f0d..335d97d3f74e5b7d7139e223fbe1d19837a72e81 100644 (file)
--- a/url.c
+++ b/url.c
@@ -48,18 +48,20 @@ static int url_decode_char(const char *q)
        return val;
 }
 
-static char *url_decode_internal(const char **query, const char *stop_at,
-                                struct strbuf *out, int decode_plus)
+static char *url_decode_internal(const char **query, int len,
+                                const char *stop_at, struct strbuf *out,
+                                int decode_plus)
 {
        const char *q = *query;
 
-       do {
+       while (len) {
                unsigned char c = *q;
 
                if (!c)
                        break;
                if (stop_at && strchr(stop_at, c)) {
                        q++;
+                       len--;
                        break;
                }
 
@@ -68,6 +70,7 @@ static char *url_decode_internal(const char **query, const char *stop_at,
                        if (0 <= val) {
                                strbuf_addch(out, val);
                                q += 3;
+                               len -= 3;
                                continue;
                        }
                }
@@ -77,34 +80,41 @@ static char *url_decode_internal(const char **query, const char *stop_at,
                else
                        strbuf_addch(out, c);
                q++;
-       } while (1);
+               len--;
+       }
        *query = q;
        return strbuf_detach(out, NULL);
 }
 
 char *url_decode(const char *url)
+{
+       return url_decode_mem(url, strlen(url));
+}
+
+char *url_decode_mem(const char *url, int len)
 {
        struct strbuf out = STRBUF_INIT;
-       const char *colon = strchr(url, ':');
+       const char *colon = memchr(url, ':', len);
 
        /* Skip protocol part if present */
        if (colon && url < colon) {
                strbuf_add(&out, url, colon - url);
+               len -= colon - url;
                url = colon;
        }
-       return url_decode_internal(&url, NULL, &out, 0);
+       return url_decode_internal(&url, len, NULL, &out, 0);
 }
 
 char *url_decode_parameter_name(const char **query)
 {
        struct strbuf out = STRBUF_INIT;
-       return url_decode_internal(query, "&=", &out, 1);
+       return url_decode_internal(query, -1, "&=", &out, 1);
 }
 
 char *url_decode_parameter_value(const char **query)
 {
        struct strbuf out = STRBUF_INIT;
-       return url_decode_internal(query, "&", &out, 1);
+       return url_decode_internal(query, -1, "&", &out, 1);
 }
 
 void end_url_with_slash(struct strbuf *buf, const char *url)

diff --git a/url.h b/url.h
index 7100e3215a97b234c8ab93b76f7f86518f7c3382..abdaf6fa30b68767f48b056c977e498f9cfe7de2 100644 (file)
--- a/url.h
+++ b/url.h
@@ -4,6 +4,7 @@
 extern int is_url(const char *url);
 extern int is_urlschemechar(int first_flag, int ch);
 extern char *url_decode(const char *url);
+extern char *url_decode_mem(const char *url, int len);
 extern char *url_decode_parameter_name(const char **query);
 extern char *url_decode_parameter_value(const char **query);