* [PATCH 10/13] credentials: add "cache" helper
From: Jeff King @ 2011-11-24 11:07 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
If you access repositories over smart-http using http
authentication, then it can be annoying to have git ask you
for your password repeatedly. We cache credentials in
memory, of course, but git is composed of many small
programs. Having to input your password for each one can be
frustrating.
This patch introduces a credential helper that will cache
passwords in memory for a short period of time.
Signed-off-by: Jeff King <peff@peff.net>
---
This required some rewriting to handle the new helper protocol. I also
incorporated suggestions from Ramsay Jones to handle cleanup of the
socket better.
I suspect this will all have to get turned off for msysgit, unless
somebody feels like writing a unix-sockets abstraction layer.
.gitignore | 2 +
Documentation/git-credential-cache--daemon.txt | 26 +++
Documentation/git-credential-cache.txt | 77 +++++++
Documentation/gitcredentials.txt | 17 +-
Makefile | 3 +
credential-cache--daemon.c | 269 ++++++++++++++++++++++++
credential-cache.c | 120 +++++++++++
git-compat-util.h | 1 +
t/lib-credential.sh | 219 +++++++++++++++++++
t/t0301-credential-cache.sh | 18 ++
unix-socket.c | 56 +++++
unix-socket.h | 7 +
12 files changed, 810 insertions(+), 5 deletions(-)
create mode 100644 Documentation/git-credential-cache--daemon.txt
create mode 100644 Documentation/git-credential-cache.txt
create mode 100644 credential-cache--daemon.c
create mode 100644 credential-cache.c
create mode 100755 t/t0301-credential-cache.sh
create mode 100644 unix-socket.c
create mode 100644 unix-socket.h
diff --git a/.gitignore b/.gitignore
index 7d2fefc..a6b0bd4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,8 @@
/git-commit-tree
/git-config
/git-count-objects
+/git-credential-cache
+/git-credential-cache--daemon
/git-cvsexportcommit
/git-cvsimport
/git-cvsserver
diff --git a/Documentation/git-credential-cache--daemon.txt b/Documentation/git-credential-cache--daemon.txt
new file mode 100644
index 0000000..11edc5a
--- /dev/null
+++ b/Documentation/git-credential-cache--daemon.txt
@@ -0,0 +1,26 @@
+git-credential-cache--daemon(1)
+===============================
+
+NAME
+----
+git-credential-cache--daemon - temporarily store user credentials in memory
+
+SYNOPSIS
+--------
+[verse]
+git credential-cache--daemon <socket>
+
+DESCRIPTION
+-----------
+
+NOTE: You probably don't want to invoke this command yourself; it is
+started automatically when you use linkgit:git-credential-cache[1].
+
+This command listens on the Unix domain socket specified by `<socket>`
+for `git-credential-cache` clients. Clients may store and retrieve
+credentials. Each credential is held for a timeout specified by the
+client; once no credentials are held, the daemon exits.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/git-credential-cache.txt b/Documentation/git-credential-cache.txt
new file mode 100644
index 0000000..f3d09c5
--- /dev/null
+++ b/Documentation/git-credential-cache.txt
@@ -0,0 +1,77 @@
+git-credential-cache(1)
+=======================
+
+NAME
+----
+git-credential-cache - helper to temporarily store passwords in memory
+
+SYNOPSIS
+--------
+-----------------------------
+git config credential.helper 'cache [options]'
+-----------------------------
+
+DESCRIPTION
+-----------
+
+This command caches credentials in memory for use by future git
+programs. The stored credentials never touch the disk, and are forgotten
+after a configurable timeout. The cache is accessible over a Unix
+domain socket, restricted to the current user by filesystem permissions.
+
+You probably don't want to invoke this command directly; it is meant to
+be used as a credential helper by other parts of git. See
+linkgit:gitcredentials[7] or `EXAMPLES` below.
+
+OPTIONS
+-------
+
+--timeout <seconds>::
+
+ Number of seconds to cache credentials (default: 900).
+
+--socket <path>::
+
+ Use `<path>` to contact a running cache daemon (or start a new
+ cache daemon if one is not started). Defaults to
+ `~/.git-credential-cache/socket`. If your home directory is on a
+ network-mounted filesystem, you may need to change this to a
+ local filesystem.
+
+CONTROLLING THE DAEMON
+----------------------
+
+If you would like the daemon to exit early, forgetting all cached
+credentials before their timeout, you can issue an `exit` action:
+
+--------------------------------------
+git credential-cache exit
+--------------------------------------
+
+EXAMPLES
+--------
+
+The point of this helper is to reduce the number of times you must type
+your username or password. For example:
+
+------------------------------------
+$ git config credential.helper cache
+$ git push http://example.com/repo.git
+Username: <type your username>
+Password: <type your password>
+
+[work for 5 more minutes]
+$ git push http://example.com/repo.git
+[your credentials are used automatically]
+------------------------------------
+
+You can provide options via the credential.helper configuration
+variable (this example drops the cache time to 5 minutes):
+
+-------------------------------------------------------
+$ git config credential.helper 'cache --timeout=300'
+-------------------------------------------------------
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 1e54117..a1592ed 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -63,11 +63,18 @@ Credential helpers, on the other hand, are external programs from which git can
request both usernames and passwords; they typically interface with secure
storage provided by the OS or other programs.
-To use a helper, you must first select one to use. Git does not yet
-include any credential helpers, but you may have third-party helpers
-installed; search for `credential-*` in the output of `git help -a`, and
-consult the documentation of individual helpers. Once you have selected
-a helper, you can tell git to use it by putting its name into the
+To use a helper, you must first select one to use. Git currently
+includes the following helpers:
+
+cache::
+
+ Cache credentials in memory for a short period of time. See
+ linkgit:git-credential-cache[1] for details.
+
+You may may also have third-party helpers installed; search for
+`credential-*` in the output of `git help -a`, and consult the
+documentation of individual helpers. Once you have selected a helper,
+you can tell git to use it by putting its name into the
credential.helper variable.
1. Find a helper.
diff --git a/Makefile b/Makefile
index 5ca363b..5d41c29 100644
--- a/Makefile
+++ b/Makefile
@@ -427,6 +427,8 @@ PROGRAM_OBJS += show-index.o
PROGRAM_OBJS += upload-pack.o
PROGRAM_OBJS += http-backend.o
PROGRAM_OBJS += sh-i18n--envsubst.o
+PROGRAM_OBJS += credential-cache.o
+PROGRAM_OBJS += credential-cache--daemon.o
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
@@ -699,6 +701,7 @@ LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
LIB_OBJS += tree.o
LIB_OBJS += tree-walk.o
+LIB_OBJS += unix-socket.o
LIB_OBJS += unpack-trees.o
LIB_OBJS += url.o
LIB_OBJS += usage.o
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
new file mode 100644
index 0000000..390f194
--- /dev/null
+++ b/credential-cache--daemon.c
@@ -0,0 +1,269 @@
+#include "cache.h"
+#include "credential.h"
+#include "unix-socket.h"
+#include "sigchain.h"
+
+static const char *socket_path;
+
+static void cleanup_socket(void)
+{
+ if (socket_path)
+ unlink(socket_path);
+}
+
+static void cleanup_socket_on_signal(int sig)
+{
+ cleanup_socket();
+ sigchain_pop(sig);
+ raise(sig);
+}
+
+struct credential_cache_entry {
+ struct credential item;
+ unsigned long expiration;
+};
+static struct credential_cache_entry *entries;
+static int entries_nr;
+static int entries_alloc;
+
+static void cache_credential(struct credential *c, int timeout)
+{
+ struct credential_cache_entry *e;
+
+ ALLOC_GROW(entries, entries_nr + 1, entries_alloc);
+ e = &entries[entries_nr++];
+
+ /* take ownership of pointers */
+ memcpy(&e->item, c, sizeof(*c));
+ memset(c, 0, sizeof(*c));
+ e->expiration = time(NULL) + timeout;
+}
+
+static struct credential_cache_entry *lookup_credential(const struct credential *c)
+{
+ int i;
+ for (i = 0; i < entries_nr; i++) {
+ struct credential *e = &entries[i].item;
+ if (credential_match(c, e))
+ return &entries[i];
+ }
+ return NULL;
+}
+
+static void remove_credential(const struct credential *c)
+{
+ struct credential_cache_entry *e;
+
+ e = lookup_credential(c);
+ if (e)
+ e->expiration = 0;
+}
+
+static int check_expirations(void)
+{
+ static unsigned long wait_for_entry_until;
+ int i = 0;
+ unsigned long now = time(NULL);
+ unsigned long next = (unsigned long)-1;
+
+ /*
+ * Initially give the client 30 seconds to actually contact us
+ * and store a credential before we decide there's no point in
+ * keeping the daemon around.
+ */
+ if (!wait_for_entry_until)
+ wait_for_entry_until = now + 30;
+
+ while (i < entries_nr) {
+ if (entries[i].expiration <= now) {
+ entries_nr--;
+ credential_clear(&entries[i].item);
+ if (i != entries_nr)
+ memcpy(&entries[i], &entries[entries_nr], sizeof(*entries));
+ /*
+ * Stick around 30 seconds in case a new credential
+ * shows up (e.g., because we just removed a failed
+ * one, and we will soon get the correct one).
+ */
+ wait_for_entry_until = now + 30;
+ }
+ else {
+ if (entries[i].expiration < next)
+ next = entries[i].expiration;
+ i++;
+ }
+ }
+
+ if (!entries_nr) {
+ if (wait_for_entry_until <= now)
+ return 0;
+ next = wait_for_entry_until;
+ }
+
+ return next - now;
+}
+
+static int read_request(FILE *fh, struct credential *c,
+ struct strbuf *action, int *timeout) {
+ static struct strbuf item = STRBUF_INIT;
+ const char *p;
+
+ strbuf_getline(&item, fh, '\n');
+ p = skip_prefix(item.buf, "action=");
+ if (!p)
+ return error("client sent bogus action line: %s", item.buf);
+ strbuf_addstr(action, p);
+
+ strbuf_getline(&item, fh, '\n');
+ p = skip_prefix(item.buf, "timeout=");
+ if (!p)
+ return error("client sent bogus timeout line: %s", item.buf);
+ *timeout = atoi(p);
+
+ if (credential_read(c, fh) < 0)
+ return -1;
+ return 0;
+}
+
+static void serve_one_client(FILE *in, FILE *out)
+{
+ struct credential c = CREDENTIAL_INIT;
+ struct strbuf action = STRBUF_INIT;
+ int timeout = -1;
+
+ if (read_request(in, &c, &action, &timeout) < 0)
+ /* ignore error */ ;
+ else if (!strcmp(action.buf, "get")) {
+ struct credential_cache_entry *e = lookup_credential(&c);
+ if (e) {
+ fprintf(out, "username=%s\n", e->item.username);
+ fprintf(out, "password=%s\n", e->item.password);
+ }
+ }
+ else if (!strcmp(action.buf, "exit"))
+ exit(0);
+ else if (!strcmp(action.buf, "erase"))
+ remove_credential(&c);
+ else if (!strcmp(action.buf, "store")) {
+ if (timeout < 0)
+ warning("cache client didn't specify a timeout");
+ else if (!c.username || !c.password)
+ warning("cache client gave us a partial credential");
+ else {
+ remove_credential(&c);
+ cache_credential(&c, timeout);
+ }
+ }
+ else
+ warning("cache client sent unknown action: %s", action.buf);
+
+ credential_clear(&c);
+ strbuf_release(&action);
+}
+
+static int serve_cache_loop(int fd)
+{
+ struct pollfd pfd;
+ unsigned long wakeup;
+
+ wakeup = check_expirations();
+ if (!wakeup)
+ return 0;
+
+ pfd.fd = fd;
+ pfd.events = POLLIN;
+ if (poll(&pfd, 1, 1000 * wakeup) < 0) {
+ if (errno != EINTR)
+ die_errno("poll failed");
+ return 1;
+ }
+
+ if (pfd.revents & POLLIN) {
+ int client, client2;
+ FILE *in, *out;
+
+ client = accept(fd, NULL, NULL);
+ if (client < 0) {
+ warning("accept failed: %s", strerror(errno));
+ return 1;
+ }
+ client2 = dup(client);
+ if (client2 < 0) {
+ warning("dup failed: %s", strerror(errno));
+ close(client);
+ return 1;
+ }
+
+ in = xfdopen(client, "r");
+ out = xfdopen(client2, "w");
+ serve_one_client(in, out);
+ fclose(in);
+ fclose(out);
+ }
+ return 1;
+}
+
+static void serve_cache(const char *socket_path)
+{
+ int fd;
+
+ fd = unix_stream_listen(socket_path);
+ if (fd < 0)
+ die_errno("unable to bind to '%s'", socket_path);
+
+ printf("ok\n");
+ fclose(stdout);
+
+ while (serve_cache_loop(fd))
+ ; /* nothing */
+
+ close(fd);
+ unlink(socket_path);
+}
+
+static const char permissions_advice[] =
+"The permissions on your socket directory are too loose; other\n"
+"users may be able to read your cached credentials. Consider running:\n"
+"\n"
+" chmod 0700 %s";
+static void check_socket_directory(const char *path)
+{
+ struct stat st;
+ char *path_copy = xstrdup(path);
+ char *dir = dirname(path_copy);
+
+ if (!stat(dir, &st)) {
+ if (st.st_mode & 077)
+ die(permissions_advice, dir);
+ free(path_copy);
+ return;
+ }
+
+ /*
+ * We must be sure to create the directory with the correct mode,
+ * not just chmod it after the fact; otherwise, there is a race
+ * condition in which somebody can chdir to it, sleep, then try to open
+ * our protected socket.
+ */
+ if (safe_create_leading_directories_const(dir) < 0)
+ die_errno("unable to create directories for '%s'", dir);
+ if (mkdir(dir, 0700) < 0)
+ die_errno("unable to mkdir '%s'", dir);
+ free(path_copy);
+}
+
+int main(int argc, const char **argv)
+{
+ socket_path = argv[1];
+
+ if (!socket_path)
+ die("usage: git-credential-cache--daemon <socket_path>");
+ check_socket_directory(socket_path);
+
+ atexit(cleanup_socket);
+ sigchain_push_common(cleanup_socket_on_signal);
+
+ serve_cache(socket_path);
+
+ return 0;
+}
diff --git a/credential-cache.c b/credential-cache.c
new file mode 100644
index 0000000..482c4b1
--- /dev/null
+++ b/credential-cache.c
@@ -0,0 +1,120 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+#include "unix-socket.h"
+#include "run-command.h"
+
+#define FLAG_SPAWN 0x1
+#define FLAG_RELAY 0x2
+
+static int send_request(const char *socket, const struct strbuf *out)
+{
+ int got_data = 0;
+ int fd = unix_stream_connect(socket);
+
+ if (fd < 0)
+ return -1;
+
+ if (write_in_full(fd, out->buf, out->len) < 0)
+ die_errno("unable to write to cache daemon");
+ shutdown(fd, SHUT_WR);
+
+ while (1) {
+ char in[1024];
+ int r;
+
+ r = read_in_full(fd, in, sizeof(in));
+ if (r == 0)
+ break;
+ if (r < 0)
+ die_errno("read error from cache daemon");
+ write_or_die(1, in, r);
+ got_data = 1;
+ }
+ return got_data;
+}
+
+static void spawn_daemon(const char *socket)
+{
+ struct child_process daemon;
+ const char *argv[] = { NULL, NULL, NULL };
+ char buf[128];
+ int r;
+
+ memset(&daemon, 0, sizeof(daemon));
+ argv[0] = "git-credential-cache--daemon";
+ argv[1] = socket;
+ daemon.argv = argv;
+ daemon.no_stdin = 1;
+ daemon.out = -1;
+
+ if (start_command(&daemon))
+ die_errno("unable to start cache daemon");
+ r = read_in_full(daemon.out, buf, sizeof(buf));
+ if (r < 0)
+ die_errno("unable to read result code from cache daemon");
+ if (r != 3 || memcmp(buf, "ok\n", 3))
+ die("cache daemon did not start: %.*s", r, buf);
+ close(daemon.out);
+}
+
+static void do_cache(const char *socket, const char *action, int timeout,
+ int flags)
+{
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "action=%s\n", action);
+ strbuf_addf(&buf, "timeout=%d\n", timeout);
+ if (flags & FLAG_RELAY) {
+ if (strbuf_read(&buf, 0, 0) < 0)
+ die_errno("unable to relay credential");
+ }
+
+ if (!send_request(socket, &buf))
+ return;
+ if (flags & FLAG_SPAWN) {
+ spawn_daemon(socket);
+ send_request(socket, &buf);
+ }
+ strbuf_release(&buf);
+}
+
+int main(int argc, const char **argv)
+{
+ char *socket_path = NULL;
+ int timeout = 900;
+ const char *op;
+ const char * const usage[] = {
+ "git credential-cache [options] <action>",
+ NULL
+ };
+ struct option options[] = {
+ OPT_INTEGER(0, "timeout", &timeout,
+ "number of seconds to cache credentials"),
+ OPT_STRING(0, "socket", &socket_path, "path",
+ "path of cache-daemon socket"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, NULL, options, usage, 0);
+ if (!argc)
+ usage_with_options(usage, options);
+ op = argv[0];
+
+ if (!socket_path)
+ socket_path = expand_user_path("~/.git-credential-cache/socket");
+ if (!socket_path)
+ die("unable to find a suitable socket path; use --socket");
+
+ if (!strcmp(op, "exit"))
+ do_cache(socket_path, op, timeout, 0);
+ else if (!strcmp(op, "get") || !strcmp(op, "erase"))
+ do_cache(socket_path, op, timeout, FLAG_RELAY);
+ else if (!strcmp(op, "store"))
+ do_cache(socket_path, op, timeout, FLAG_RELAY|FLAG_SPAWN);
+ else
+ die("unknown operation: %s", op);
+
+ return 0;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 8b4dd5c..5c238bd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -130,6 +130,7 @@
#include <arpa/inet.h>
#include <netdb.h>
#include <pwd.h>
+#include <sys/un.h>
#ifndef NO_INTTYPES_H
#include <inttypes.h>
#else
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 54ae1f4..ac54b38 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -21,6 +21,225 @@ read_chunk() {
done
}
+# Clear any residual data from previous tests. We only
+# need this when testing third-party helpers which read and
+# write outside of our trash-directory sandbox.
+#
+# Don't bother checking for success here, as it is
+# outside the scope of tests and represents a best effort to
+# clean up after ourselves.
+helper_test_clean() {
+ reject $1 https example.com store-user
+ reject $1 https example.com user1
+ reject $1 https example.com user2
+ reject $1 ftp other.tld user
+ reject $1 https timeout.tld user
+}
+
+reject() {
+ (
+ echo protocol=$2
+ echo host=$3
+ echo username=$4
+ ) | test-credential reject $1
+}
+
+helper_test() {
+ HELPER=$1
+
+ test_expect_success "helper ($HELPER) has no existing data" '
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''https://example.com'\'':
+ askpass: Password for '\''https://askpass-username@example.com'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) stores password" '
+ check approve $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ username=store-user
+ password=store-pass
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) can retrieve password" '
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ username=store-user
+ password=store-pass
+ --
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) requires matching protocol" '
+ check fill $HELPER <<-\EOF
+ protocol=http
+ host=example.com
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''http://example.com'\'':
+ askpass: Password for '\''http://askpass-username@example.com'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) requires matching host" '
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=other.tld
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''https://other.tld'\'':
+ askpass: Password for '\''https://askpass-username@other.tld'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) requires matching username" '
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ username=other
+ --
+ username=other
+ password=askpass-password
+ --
+ askpass: Password for '\''https://other@example.com'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) requires matching path" '
+ check approve $HELPER <<-\EOF &&
+ protocol=ftp
+ host=other.tld
+ path=foo.git
+ username=user
+ password=pass
+ EOF
+ check fill $HELPER <<-\EOF
+ protocol=ftp
+ host=other.tld
+ path=bar.git
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''ftp://other.tld/bar.git'\'':
+ askpass: Password for '\''ftp://askpass-username@other.tld/bar.git'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) can forget host" '
+ check reject $HELPER <<-\EOF &&
+ protocol=https
+ host=example.com
+ EOF
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''https://example.com'\'':
+ askpass: Password for '\''https://askpass-username@example.com'\'':
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) can store multiple users" '
+ check approve $HELPER <<-\EOF &&
+ protocol=https
+ host=example.com
+ username=user1
+ password=pass1
+ EOF
+ check approve $HELPER <<-\EOF &&
+ protocol=https
+ host=example.com
+ username=user2
+ password=pass2
+ EOF
+ check fill $HELPER <<-\EOF &&
+ protocol=https
+ host=example.com
+ username=user1
+ --
+ username=user1
+ password=pass1
+ EOF
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ username=user2
+ --
+ username=user2
+ password=pass2
+ EOF
+ '
+
+ test_expect_success "helper ($HELPER) can forget user" '
+ check reject $HELPER <<-\EOF &&
+ protocol=https
+ host=example.com
+ username=user1
+ EOF
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ username=user1
+ --
+ username=user1
+ password=askpass-password
+ --
+ askpass: Password for '\''https://user1@example.com'\'':
+ '
+
+ test_expect_success "helper ($HELPER) remembers other user" '
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
+ username=user2
+ --
+ username=user2
+ password=pass2
+ EOF
+ '
+}
+
+helper_test_timeout() {
+ HELPER="$*"
+
+ test_expect_success "helper ($HELPER) times out" '
+ check approve "$HELPER" <<-\EOF &&
+ protocol=https
+ host=timeout.tld
+ username=user
+ password=pass
+ EOF
+ sleep 2 &&
+ check fill "$HELPER" <<-\EOF
+ protocol=https
+ host=timeout.tld
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username for '\''https://timeout.tld'\'':
+ askpass: Password for '\''https://askpass-username@timeout.tld'\'':
+ EOF
+ '
+}
cat >askpass <<\EOF
#!/bin/sh
diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
new file mode 100755
index 0000000..3a65f99
--- /dev/null
+++ b/t/t0301-credential-cache.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+test_description='credential-cache tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+# don't leave a stale daemon running
+trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT
+
+helper_test cache
+helper_test_timeout cache --timeout=1
+
+# we can't rely on our "trap" above working after test_done,
+# as test_done will delete the trash directory containing
+# our socket, leaving us with no way to access the daemon.
+git credential-cache exit
+
+test_done
diff --git a/unix-socket.c b/unix-socket.c
new file mode 100644
index 0000000..84b1509
--- /dev/null
+++ b/unix-socket.c
@@ -0,0 +1,56 @@
+#include "cache.h"
+#include "unix-socket.h"
+
+static int unix_stream_socket(void)
+{
+ int fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd < 0)
+ die_errno("unable to create socket");
+ return fd;
+}
+
+static void unix_sockaddr_init(struct sockaddr_un *sa, const char *path)
+{
+ int size = strlen(path) + 1;
+ if (size > sizeof(sa->sun_path))
+ die("socket path is too long to fit in sockaddr");
+ memset(sa, 0, sizeof(*sa));
+ sa->sun_family = AF_UNIX;
+ memcpy(sa->sun_path, path, size);
+}
+
+int unix_stream_connect(const char *path)
+{
+ int fd;
+ struct sockaddr_un sa;
+
+ unix_sockaddr_init(&sa, path);
+ fd = unix_stream_socket();
+ if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+ close(fd);
+ return -1;
+ }
+ return fd;
+}
+
+int unix_stream_listen(const char *path)
+{
+ int fd;
+ struct sockaddr_un sa;
+
+ unix_sockaddr_init(&sa, path);
+ fd = unix_stream_socket();
+
+ unlink(path);
+ if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+ close(fd);
+ return -1;
+ }
+
+ if (listen(fd, 5) < 0) {
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
diff --git a/unix-socket.h b/unix-socket.h
new file mode 100644
index 0000000..e271aee
--- /dev/null
+++ b/unix-socket.h
@@ -0,0 +1,7 @@
+#ifndef UNIX_SOCKET_H
+#define UNIX_SOCKET_H
+
+int unix_stream_connect(const char *path);
+int unix_stream_listen(const char *path);
+
+#endif /* UNIX_SOCKET_H */
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 11/13] strbuf: add strbuf_add*_urlencode
From: Jeff King @ 2011-11-24 11:07 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
This just follows the rfc3986 rules for percent-encoding
url data into a strbuf.
Signed-off-by: Jeff King <peff@peff.net>
---
strbuf.c | 37 +++++++++++++++++++++++++++++++++++++
strbuf.h | 5 +++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 3ad2cc0..60e5e59 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -397,3 +397,40 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
return len;
}
+
+static int is_rfc3986_reserved(char ch)
+{
+ switch (ch) {
+ case '!': case '*': case '\'': case '(': case ')': case ';':
+ case ':': case '@': case '&': case '=': case '+': case '$':
+ case ',': case '/': case '?': case '#': case '[': case ']':
+ return 1;
+ }
+ return 0;
+}
+
+static int is_rfc3986_unreserved(char ch)
+{
+ return isalnum(ch) ||
+ ch == '-' || ch == '_' || ch == '.' || ch == '~';
+}
+
+void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
+ int reserved)
+{
+ strbuf_grow(sb, len);
+ while (len--) {
+ char ch = *s++;
+ if (is_rfc3986_unreserved(ch) ||
+ (!reserved && is_rfc3986_reserved(ch)))
+ strbuf_addch(sb, ch);
+ else
+ strbuf_addf(sb, "%%%02x", ch);
+ }
+}
+
+void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,
+ int reserved)
+{
+ strbuf_add_urlencode(sb, s, strlen(s), reserved);
+}
diff --git a/strbuf.h b/strbuf.h
index 46a33f8..cecd48c 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -115,4 +115,9 @@ struct strbuf_expand_dict_entry {
extern int strbuf_branchname(struct strbuf *sb, const char *name);
extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+extern void strbuf_add_urlencode(struct strbuf *, const char *, size_t,
+ int reserved);
+extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
+ int reserved);
+
#endif /* STRBUF_H */
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 12/13] credentials: add "store" helper
From: Jeff King @ 2011-11-24 11:07 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
This is like "cache", except that we actually put the
credentials on disk. This can be terribly insecure, of
course, but we do what we can to protect them by filesystem
permissions, and we warn the user in the documentation.
This is not unlike using .netrc to store entries, but it's a
little more user-friendly. Instead of putting credentials in
place ahead of time, we transparently store them after
prompting the user for them once.
Signed-off-by: Jeff King <peff@peff.net>
---
.gitignore | 1 +
Documentation/git-credential-store.txt | 75 +++++++++++++++++
Documentation/gitcredentials.txt | 5 +
Makefile | 1 +
credential-store.c | 144 ++++++++++++++++++++++++++++++++
t/t0302-credential-store.sh | 9 ++
6 files changed, 235 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-credential-store.txt
create mode 100644 credential-store.c
create mode 100755 t/t0302-credential-store.sh
diff --git a/.gitignore b/.gitignore
index a6b0bd4..2b7a3f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@
/git-count-objects
/git-credential-cache
/git-credential-cache--daemon
+/git-credential-store
/git-cvsexportcommit
/git-cvsimport
/git-cvsserver
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
new file mode 100644
index 0000000..3dcffbd
--- /dev/null
+++ b/Documentation/git-credential-store.txt
@@ -0,0 +1,75 @@
+git-credential-store(1)
+=======================
+
+NAME
+----
+git-credential-store - helper to store credentials on disk
+
+SYNOPSIS
+--------
+-------------------
+git config credential.helper 'store [options]'
+-------------------
+
+DESCRIPTION
+-----------
+
+NOTE: Using this helper will store your passwords unencrypted on disk,
+protected only by filesystem permissions. If this is not an acceptable
+security tradeoff, try linkgit:git-credential-cache[1], or find a helper
+that integrates with secure storage provided by your operating system.
+
+This command stores credentials indefinitely on disk for use by future
+git programs.
+
+You probably don't want to invoke this command directly; it is meant to
+be used as a credential helper by other parts of git. See
+linkgit:gitcredentials[7] or `EXAMPLES` below.
+
+OPTIONS
+-------
+
+--store=<path>::
+
+ Use `<path>` to store credentials. The file will have its
+ filesystem permissions set to prevent other users on the system
+ from reading it, but will not be encrypted or otherwise
+ protected. Defaults to `~/.git-credentials`.
+
+EXAMPLES
+--------
+
+The point of this helper is to reduce the number of times you must type
+your username or password. For example:
+
+------------------------------------------
+$ git config credential.helper store
+$ git push http://example.com/repo.git
+Username: <type your username>
+Password: <type your password>
+
+[several days later]
+$ git push http://example.com/repo.git
+[your credentials are used automatically]
+------------------------------------------
+
+STORAGE FORMAT
+--------------
+
+The `.git-credentials` file is stored in plaintext. Each credential is
+stored on its own line as a URL like:
+
+------------------------------
+https://user:pass@example.com
+------------------------------
+
+When git needs authentication for a particular context URL context,
+credential-store will consider that context a pattern to match against
+each entry in the credentials file. If the protocol, hostname, and
+username (if we already have one) match, then the password is returned
+to git. See the discussion of configuration in linkgit:gitcredentials[7]
+for more information.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index a1592ed..519d2bb 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -71,6 +71,11 @@ cache::
Cache credentials in memory for a short period of time. See
linkgit:git-credential-cache[1] for details.
+store::
+
+ Store credentials indefinitely on disk. See
+ linkgit:git-credential-store[1] for details.
+
You may may also have third-party helpers installed; search for
`credential-*` in the output of `git help -a`, and consult the
documentation of individual helpers. Once you have selected a helper,
diff --git a/Makefile b/Makefile
index 5d41c29..2537128 100644
--- a/Makefile
+++ b/Makefile
@@ -429,6 +429,7 @@ PROGRAM_OBJS += http-backend.o
PROGRAM_OBJS += sh-i18n--envsubst.o
PROGRAM_OBJS += credential-cache.o
PROGRAM_OBJS += credential-cache--daemon.o
+PROGRAM_OBJS += credential-store.o
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
diff --git a/credential-store.c b/credential-store.c
new file mode 100644
index 0000000..a6ddb0c
--- /dev/null
+++ b/credential-store.c
@@ -0,0 +1,144 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+
+static struct lock_file credential_lock;
+
+static void parse_credential_file(const char *fn,
+ struct credential *c,
+ void (*match_cb)(struct credential *),
+ void (*other_cb)(struct strbuf *))
+{
+ FILE *fh;
+ struct strbuf line = STRBUF_INIT;
+ struct credential entry = CREDENTIAL_INIT;
+
+ fh = fopen(fn, "r");
+ if (!fh) {
+ if (errno != ENOENT)
+ die_errno("unable to open %s", fn);
+ return;
+ }
+
+ while (strbuf_getline(&line, fh, '\n') != EOF) {
+ credential_from_url(&entry, line.buf);
+ if (entry.username && entry.password &&
+ credential_match(c, &entry)) {
+ if (match_cb) {
+ match_cb(&entry);
+ break;
+ }
+ }
+ else if (other_cb)
+ other_cb(&line);
+ }
+
+ credential_clear(&entry);
+ strbuf_release(&line);
+ fclose(fh);
+}
+
+static void print_entry(struct credential *c)
+{
+ printf("username=%s\n", c->username);
+ printf("password=%s\n", c->password);
+}
+
+static void print_line(struct strbuf *buf)
+{
+ strbuf_addch(buf, '\n');
+ write_or_die(credential_lock.fd, buf->buf, buf->len);
+}
+
+static void rewrite_credential_file(const char *fn, struct credential *c,
+ struct strbuf *extra)
+{
+ umask(077);
+ if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
+ die_errno("unable to get credential storage lock");
+ parse_credential_file(fn, c, NULL, print_line);
+ if (extra)
+ print_line(extra);
+ if (commit_lock_file(&credential_lock) < 0)
+ die_errno("unable to commit credential store");
+}
+
+static void store_credential(const char *fn, struct credential *c)
+{
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!c->protocol || !(c->host || c->path) ||
+ !c->username || !c->password)
+ return;
+
+ strbuf_addf(&buf, "%s://", c->protocol);
+ strbuf_addstr_urlencode(&buf, c->username, 1);
+ strbuf_addch(&buf, ':');
+ strbuf_addstr_urlencode(&buf, c->password, 1);
+ strbuf_addch(&buf, '@');
+ if (c->host)
+ strbuf_addstr_urlencode(&buf, c->host, 1);
+ if (c->path) {
+ strbuf_addch(&buf, '/');
+ strbuf_addstr_urlencode(&buf, c->path, 0);
+ }
+
+ rewrite_credential_file(fn, c, &buf);
+ strbuf_release(&buf);
+}
+
+static void remove_credential(const char *fn, struct credential *c)
+{
+ if (!c->protocol || !(c->host || c->path))
+ return;
+ rewrite_credential_file(fn, c, NULL);
+}
+
+static int lookup_credential(const char *fn, struct credential *c)
+{
+ if (!c->protocol || !(c->host || c->path))
+ return 0;
+ parse_credential_file(fn, c, print_entry, NULL);
+ return c->username && c->password;
+}
+
+int main(int argc, const char **argv)
+{
+ const char * const usage[] = {
+ "git credential-store [options] <action>",
+ NULL
+ };
+ const char *op;
+ struct credential c = CREDENTIAL_INIT;
+ char *store = NULL;
+ struct option options[] = {
+ OPT_STRING_LIST(0, "store", &store, "file",
+ "fetch and store credentials in <file>"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, NULL, options, usage, 0);
+ if (argc != 1)
+ usage_with_options(usage, options);
+ op = argv[0];
+
+ if (!store)
+ store = expand_user_path("~/.git-credentials");
+ if (!store)
+ die("unable to set up default store; use --store");
+
+ if (credential_read(&c, stdin) < 0)
+ die("unable to read credential");
+
+ if (!strcmp(op, "get"))
+ lookup_credential(store, &c);
+ else if (!strcmp(op, "erase"))
+ remove_credential(store, &c);
+ else if (!strcmp(op, "store"))
+ store_credential(store, &c);
+ else
+ die("unknown operation: %s", op);
+
+ return 0;
+}
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
new file mode 100755
index 0000000..f61b40c
--- /dev/null
+++ b/t/t0302-credential-store.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+test_description='credential-store tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+helper_test store
+
+test_done
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 13/13] t: add test harness for external credential helpers
From: Jeff King @ 2011-11-24 11:09 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
We already have tests for the internal helpers, but it's
nice to give authors of external tools an easy way to
sanity-check their helpers.
If you have written the "git-credential-foo" helper, you can
do so with:
GIT_TEST_CREDENTIAL_HELPER=foo \
make t0303-credential-external.sh
This assumes that your helper is capable of both storing and
retrieving credentials (some helpers may be read-only, and
they will fail these tests).
If your helper supports time-based expiration with a
configurable timeout, you can test that feature like this:
GIT_TEST_CREDENTIAL_HELPER_TIMEOUT="foo --timeout=1" \
make t0303-credential-external.sh
Signed-off-by: Jeff King <peff@peff.net>
---
I haven't tried porting or testing any of the older helpers to the new
interface. So this script has only been lightly tested with:
GIT_TEST_CREDENTIAL_HELPER=cache
t/t0303-credential-external.sh | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
create mode 100755 t/t0303-credential-external.sh
diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh
new file mode 100755
index 0000000..f3953d4
--- /dev/null
+++ b/t/t0303-credential-external.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+test_description='external credential helper tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+if test -z "$GIT_TEST_CREDENTIAL_HELPER"; then
+ say "# skipping external helper tests (set GIT_TEST_CREDENTIAL_HELPER)"
+else
+ helper_test "$GIT_TEST_CREDENTIAL_HELPER"
+fi
+
+if test -z "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"; then
+ say "# skipping external helper timeout tests"
+else
+ helper_test_timeout "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"
+fi
+
+test_done
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* Re: [PATCH 0/13] credential helpers, take two
From: Erik Faye-Lund @ 2011-11-24 11:45 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
On Thu, Nov 24, 2011 at 11:58 AM, Jeff King <peff@peff.net> wrote:
> Here's a revised version of the http-auth / credential-helper series.
>
Nice. Do you have the branch somewhere public I can pull it from?
^ permalink raw reply
* Re: git-bisect working only from toplevel dir
From: Junio C Hamano @ 2011-11-24 11:50 UTC (permalink / raw)
To: Peter Baumann; +Cc: Jeff King, Adam Borowski, git
In-Reply-To: <20111124070659.GC6291@m62s10.vlinux.de>
Peter Baumann <waste.manager@gmx.de> writes:
> On Wed, Nov 23, 2011 at 12:45:18PM -0800, Junio C Hamano wrote:
> ...
>> Also didn't we make bisect workable in a bare repository recently? So the
>> start-up sequence has to be something more elaborate like...
>> ...
>> and then inside bisect_next() you would check if $prefix exists, and go
>> there to run bisect--helper (or fail to go there and say "cannot test").
>
> But is the "cannot test" aka exit(127) the best we can do in this case?
Yeah, thinking about it a bit more, it may probably be better to make it a
failure. The user explicitly asked "be in _this_ directory and run make;
it should succeed for the bisection test to pass". If the bisection test
criterion the user was interested in was a successful build of the whole
project (not the subpart of the current directory), the user would have
gone up to the top-level and "bisect run make" there.
^ permalink raw reply
* Re: [PATCH 0/13] credential helpers, take two
From: Jeff King @ 2011-11-24 11:53 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git
In-Reply-To: <CABPQNSaT+4F=EW_agAh_FY0h_ZRgMCzpukdKuvZTdfmz5_Nueg@mail.gmail.com>
On Thu, Nov 24, 2011 at 12:45:39PM +0100, Erik Faye-Lund wrote:
> On Thu, Nov 24, 2011 at 11:58 AM, Jeff King <peff@peff.net> wrote:
> > Here's a revised version of the http-auth / credential-helper series.
> >
>
> Nice. Do you have the branch somewhere public I can pull it from?
git://github.com/peff/git.git jk/http-auth-simple
But be aware that I do rebase my topic branches frequently.
-Peff
^ permalink raw reply
* Re: [PATCH 0/13] credential helpers, take two
From: Erik Faye-Lund @ 2011-11-24 12:08 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111124115351.GA29465@sigill.intra.peff.net>
On Thu, Nov 24, 2011 at 12:53 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 24, 2011 at 12:45:39PM +0100, Erik Faye-Lund wrote:
>
>> On Thu, Nov 24, 2011 at 11:58 AM, Jeff King <peff@peff.net> wrote:
>> > Here's a revised version of the http-auth / credential-helper series.
>> >
>>
>> Nice. Do you have the branch somewhere public I can pull it from?
>
> git://github.com/peff/git.git jk/http-auth-simple
>
Thanks.
> But be aware that I do rebase my topic branches frequently.
>
That's fine by me, I just didn't want to have to download each patch
as a raw e-mail and apply it manually ;)
^ permalink raw reply
* Re: [PATCH] make git-svn resilient to log.abbrevcommit = true
From: Thomas Rast @ 2011-11-24 13:11 UTC (permalink / raw)
To: Shahid Alam; +Cc: git, gitster, Eric Wong
In-Reply-To: <3A78B8F7-803C-4278-8B5F-4A1B02C9FF04@gmail.com>
[add Eric to Cc]
Shahid Alam wrote:
> Add --no-abbrev-commit arg to working_head_Info()'s invocation
> of git log.
[...]
> @@ -1803,7 +1803,7 @@ sub cmt_sha2rev_batch {
> sub working_head_info {
> my ($head, $refs) = @_;
> my @args = qw/log --no-color --no-decorate --first-parent
> - --pretty=medium/;
> + --no-abbrev-commit --pretty=medium/;
Undeniably a bug, but to prevent the same from happening again,
wouldn't it be better to rewrite it using simply
rev-list --first-parent --pretty=medium
?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 12/13] credentials: add "store" helper
From: Eric Sunshine @ 2011-11-24 14:29 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111124110756.GJ8417@sigill.intra.peff.net>
On Thu, Nov 24, 2011 at 6:07 AM, Jeff King <peff@peff.net> wrote:
> diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
> new file mode 100644
> index 0000000..3dcffbd
> --- /dev/null
> +++ b/Documentation/git-credential-store.txt
> @@ -0,0 +1,75 @@
> [snip]
> +The `.git-credentials` file is stored in plaintext. Each credential is
> +stored on its own line as a URL like:
> +
> +------------------------------
> +https://user:pass@example.com
> +------------------------------
> +
> +When git needs authentication for a particular context URL context,
"context URL context"?
-- ES
^ permalink raw reply
* Re: [PATCH 10/13] credentials: add "cache" helper
From: Eric Sunshine @ 2011-11-24 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111124110710.GH8417@sigill.intra.peff.net>
On Thu, Nov 24, 2011 at 6:07 AM, Jeff King <peff@peff.net> wrote:
> diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
> index 1e54117..a1592ed 100644
> --- a/Documentation/gitcredentials.txt
> +++ b/Documentation/gitcredentials.txt
> @@ -63,11 +63,18 @@ Credential helpers, on the other hand, are external programs from which git can
> [snip]
> +cache::
> +
> + Cache credentials in memory for a short period of time. See
> + linkgit:git-credential-cache[1] for details.
> +
> +You may may also have third-party helpers installed; search for
s/may may/may/
-- ES
^ permalink raw reply
* Re: [PATCH] make git-svn resilient to log.abbrevcommit = true
From: Eric Wong @ 2011-11-24 18:32 UTC (permalink / raw)
To: Thomas Rast; +Cc: Shahid Alam, git, gitster
In-Reply-To: <201111241411.05655.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> wrote:
> Shahid Alam wrote:
> > Add --no-abbrev-commit arg to working_head_Info()'s invocation
> > of git log.
> [...]
> > @@ -1803,7 +1803,7 @@ sub cmt_sha2rev_batch {
> > sub working_head_info {
> > my ($head, $refs) = @_;
> > my @args = qw/log --no-color --no-decorate --first-parent
> > - --pretty=medium/;
> > + --no-abbrev-commit --pretty=medium/;
>
> Undeniably a bug, but to prevent the same from happening again,
> wouldn't it be better to rewrite it using simply
>
> rev-list --first-parent --pretty=medium
Yes, I've never been happy with using "git log" for any internals since
it's a porcelain. I'll gladly accept a tested patch which uses rev-list
instead. Thanks!
^ permalink raw reply
* Re: [PATCH 12/13] credentials: add "store" helper
From: Jeff King @ 2011-11-24 20:09 UTC (permalink / raw)
To: Eric Sunshine; +Cc: git
In-Reply-To: <CAPig+cQ_eP0wG=AFB0rt-SueW-XxrFXKYAXr+4PneYC-dciyuw@mail.gmail.com>
On Thu, Nov 24, 2011 at 09:29:36AM -0500, Eric Sunshine wrote:
> > +When git needs authentication for a particular context URL context,
>
> "context URL context"?
Thanks. That is what I get for proofreading right before bed. :)
I squashed this and your other typo fix into the appropriate patches.
-Peff
^ permalink raw reply
* Re: Incremental use of fast-import may cause conflicting notes
From: Jonathan Nieder @ 2011-11-24 23:09 UTC (permalink / raw)
To: Henrik Grubbström; +Cc: Git Mailing list, Johan Herland, David Barr
In-Reply-To: <Pine.GSO.4.63.1111231137350.5099@shipon.roxen.com>
Hi Henrik,
Henrik Grubbström wrote:
> Background: I have an incremental repository-walker creating a corresponding
> documentation repository from a source repository
> that uses git-notes to store its state, a use for which notes
> seem very suitable.
Nice.
[...]
> when fast-import is restarted
> it won't remember the fanout, and will start writing files in the root
> again. This means that there may be multiple notes-files for the same
> commit, eg both de/adbeef and deadbeef.
[...]
> The problem is probably due to b->num_notes not being initialized properly
> when the old non-empty root commit for the notes branch is loaded in
> parse_from()/parse_new_commit().
Sounds like a bug. Can you suggest a reproduction recipe (ideally as
a patch to t/t9301-fast-import-notes.sh), a fix, or both?
Thanks.
Regards,
Jonathan
^ permalink raw reply
* [RFC/PATCH 1/3] t9301: Fix testcase covering up a bug in fast-import's notes fanout handling
From: Johan Herland @ 2011-11-25 0:09 UTC (permalink / raw)
To: grubba; +Cc: git, jrnieder, johan
In-Reply-To: <1322179787-4422-1-git-send-email-johan@herland.net>
There is a bug in fast-import where the fanout levels of an existing notes
tree being loaded into the fast-import machinery is disregarded. Instead, any
tree loaded is assumed to have a fanout level of 0. If the true fanout level
is deeper, any attempt to remove a note from that tree will silently fail
(as the note will not be found at fanout level 0).
However, this bug was covered up by the way in which the t9301 testcase was
written: When generating the fast-import commands to test mass removal of
notes, we appended these commands to an already existing 'input' file which
happened to already contain the fast-import commands used in the previous
subtest to generate the very same notes tree. This would normally be harmless
(but suboptimal) as the notes created were identical to the notes already
present in the notes tree. But the act of repeating all the notes additions
caused the internal fast-import data structures to recalculate the fanout,
instead of hanging on to the initial (incorrect) fanout (that causes the bug
described above). Thus, the subsequent removal of notes in the same 'input'
file would succeed, thereby covering up the bug described above.
This patch creates a new 'input' file instead of appending to the file from
the previous subtest. Thus, we end up properly testing removal of notes that
were added by a previous fast-import command. As a side effect, the notes
removal can no longer refer to commits using the marks set by the previous
fast-import run, instead the commits names must be referenced directly.
The underlying fast-import bug is still present after this patch, but now we
have at least uncovered it. Therefore, the affected subtests are labeled as
expected failures until the underlying bug is fixed.
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t9301-fast-import-notes.sh | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
index 463254c..fd08161 100755
--- a/t/t9301-fast-import-notes.sh
+++ b/t/t9301-fast-import-notes.sh
@@ -507,7 +507,7 @@ test_expect_success 'verify that non-notes are untouched by a fanout change' '
'
remaining_notes=10
test_tick
-cat >>input <<INPUT_END
+cat >input <<INPUT_END
commit refs/notes/many_notes
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
data <<COMMIT
@@ -516,12 +516,11 @@ COMMIT
from refs/notes/many_notes^0
INPUT_END
-i=$remaining_notes
-while test $i -lt $num_commits
+i=$(($num_commits - $remaining_notes))
+for sha1 in $(git rev-list -n $i refs/heads/many_commits)
do
- i=$(($i + 1))
cat >>input <<INPUT_END
-N 0000000000000000000000000000000000000000 :$i
+N 0000000000000000000000000000000000000000 $sha1
INPUT_END
done
@@ -541,7 +540,7 @@ EXPECT_END
i=$(($i - 1))
done
-test_expect_success 'remove lots of notes' '
+test_expect_failure 'remove lots of notes' '
git fast-import <input &&
GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
@@ -550,7 +549,7 @@ test_expect_success 'remove lots of notes' '
'
-test_expect_success 'verify that removing notes trigger fanout consolidation' '
+test_expect_failure 'verify that removing notes trigger fanout consolidation' '
# All entries in the top-level notes tree should be a full SHA1
git ls-tree --name-only -r refs/notes/many_notes |
--
1.7.5.rc1.3.g4d7b
^ permalink raw reply related
* [RFC/PATCH 3/3] fast-import: Fix incorrect fanout level when modifying existing notes refs
From: Johan Herland @ 2011-11-25 0:09 UTC (permalink / raw)
To: grubba; +Cc: git, jrnieder, johan
In-Reply-To: <1322179787-4422-1-git-send-email-johan@herland.net>
This fixes the bug uncovered by the tests added in the previous two patches.
When an existing notes ref was loaded into the fast-import machinery, the
num_notes counter associated with that ref remained == 0, even though the
true number of notes in the loaded ref was higher. This caused a fanout
level of 0 to be used, although the actual fanout of the tree could be > 0.
Manipulating the notes tree at an incorrect fanout level causes removals to
silently fail, and modifications of existing notes to instead produce an
additional note (leaving the old object in place at a different fanout level).
This patch fixes the bug by explicitly counting the number of notes in the
notes tree whenever it looks like the num_notes counter could be wrong (when
num_notes == 0). There may be false positives (i.e. triggering the counting
when the notes tree is truly empty), but in those cases, the counting should
not take long.
Signed-off-by: Johan Herland <johan@herland.net>
---
fast-import.c | 28 +++++++++++++++++++++++++---
t/t9301-fast-import-notes.sh | 8 ++++----
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..f4bfe0f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2173,6 +2173,11 @@ static uintmax_t do_change_note_fanout(
if (tmp_hex_sha1_len == 40 && !get_sha1_hex(hex_sha1, sha1)) {
/* This is a note entry */
+ if (fanout == 0xff) {
+ /* Counting mode, no rename */
+ num_notes++;
+ continue;
+ }
construct_path_with_fanout(hex_sha1, fanout, realpath);
if (!strcmp(fullpath, realpath)) {
/* Note entry is in correct location */
@@ -2379,7 +2384,7 @@ static void file_change_cr(struct branch *b, int rename)
leaf.tree);
}
-static void note_change_n(struct branch *b, unsigned char old_fanout)
+static void note_change_n(struct branch *b, unsigned char *old_fanout)
{
const char *p = command_buf.buf + 2;
static struct strbuf uq = STRBUF_INIT;
@@ -2390,6 +2395,23 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
uint16_t inline_data = 0;
unsigned char new_fanout;
+ /*
+ * When loading a branch, we don't traverse its tree to count the real
+ * number of notes (too expensive to do this for all non-note refs).
+ * This means that recently loaded notes refs might incorrectly have
+ * b->num_notes == 0, and consequently, old_fanout might be wrong.
+ *
+ * Fix this by traversing the tree and counting the number of notes
+ * when b->num_notes == 0. If the notes tree is truly empty, the
+ * calculation should not take long.
+ */
+ if (b->num_notes == 0 && *old_fanout == 0) {
+ /* Invoke change_note_fanout() in "counting mode". */
+ b->num_notes = change_note_fanout(&b->branch_tree, 0xff);
+ *old_fanout = convert_num_notes_to_fanout(b->num_notes);
+ }
+
+ /* Now parse the notemodify command. */
/* <dataref> or 'inline' */
if (*p == ':') {
char *x;
@@ -2450,7 +2472,7 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
typename(type), command_buf.buf);
}
- construct_path_with_fanout(sha1_to_hex(commit_sha1), old_fanout, path);
+ construct_path_with_fanout(sha1_to_hex(commit_sha1), *old_fanout, path);
if (tree_content_remove(&b->branch_tree, path, NULL))
b->num_notes--;
@@ -2637,7 +2659,7 @@ static void parse_new_commit(void)
else if (!prefixcmp(command_buf.buf, "C "))
file_change_cr(b, 0);
else if (!prefixcmp(command_buf.buf, "N "))
- note_change_n(b, prev_fanout);
+ note_change_n(b, &prev_fanout);
else if (!strcmp("deleteall", command_buf.buf))
file_change_deleteall(b);
else if (!prefixcmp(command_buf.buf, "ls "))
diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
index 57d85a6..83acf68 100755
--- a/t/t9301-fast-import-notes.sh
+++ b/t/t9301-fast-import-notes.sh
@@ -536,7 +536,7 @@ EXPECT_END
j=$(($j + 1))
done
-test_expect_failure 'change a few existing notes' '
+test_expect_success 'change a few existing notes' '
git fast-import <input &&
GIT_NOTES_REF=refs/notes/many_notes git log -n3 refs/heads/many_commits |
@@ -545,7 +545,7 @@ test_expect_failure 'change a few existing notes' '
'
-test_expect_failure 'verify that changing notes respect existing fanout' '
+test_expect_success 'verify that changing notes respect existing fanout' '
# None of the entries in the top-level notes tree should be a full SHA1
git ls-tree --name-only refs/notes/many_notes |
@@ -594,7 +594,7 @@ EXPECT_END
i=$(($i - 1))
done
-test_expect_failure 'remove lots of notes' '
+test_expect_success 'remove lots of notes' '
git fast-import <input &&
GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
@@ -603,7 +603,7 @@ test_expect_failure 'remove lots of notes' '
'
-test_expect_failure 'verify that removing notes trigger fanout consolidation' '
+test_expect_success 'verify that removing notes trigger fanout consolidation' '
# All entries in the top-level notes tree should be a full SHA1
git ls-tree --name-only -r refs/notes/many_notes |
--
1.7.5.rc1.3.g4d7b
^ permalink raw reply related
* [RFC/PATCH 0/3] fast-import: Fix incremental use of notes
From: Johan Herland @ 2011-11-25 0:09 UTC (permalink / raw)
To: grubba; +Cc: git, jrnieder, johan
In-Reply-To: <Pine.GSO.4.63.1111231137350.5099@shipon.roxen.com>
Hi,
This is a first attempt at fixing the bug reported by Henrik.
The first two patches provide testcases for the bug, and the last patch
provides a fix.
Have fun! :)
...Johan
Johan Herland (3):
t9301: Fix testcase covering up a bug in fast-import's notes fanout handling
t9301: Add 2nd testcase exposing bugs in fast-import's notes fanout handling
fast-import: Fix incorrect fanout level when modifying existing notes refs
fast-import.c | 28 ++++++++++++++++--
t/t9301-fast-import-notes.sh | 63 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 83 insertions(+), 8 deletions(-)
--
1.7.5.rc1.3.g4d7b
^ permalink raw reply
* [RFC/PATCH 2/3] t9301: Add 2nd testcase exposing bugs in fast-import's notes fanout handling
From: Johan Herland @ 2011-11-25 0:09 UTC (permalink / raw)
To: grubba; +Cc: git, jrnieder, johan
In-Reply-To: <1322179787-4422-1-git-send-email-johan@herland.net>
The previous patch exposed a bug in fast-import where _removing_ an existing
note fails (when that note resides on a non-zero fanout level, and was added
prior to this fast-import run).
This patch demostrates the same issue when _changing_ an existing note
(subject to the same circumstances).
Discovered-by: Henrik Grubbström <grubba@roxen.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t9301-fast-import-notes.sh | 54 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
index fd08161..57d85a6 100755
--- a/t/t9301-fast-import-notes.sh
+++ b/t/t9301-fast-import-notes.sh
@@ -505,6 +505,60 @@ test_expect_success 'verify that non-notes are untouched by a fanout change' '
test_cmp expect_non-note3 actual
'
+
+# Change the notes for the three top commits
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/many_notes
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+changing notes for the top three commits
+COMMIT
+from refs/notes/many_notes^0
+INPUT_END
+
+rm expect
+i=$num_commits
+j=0
+while test $j -lt 3
+do
+ cat >>input <<INPUT_END
+N inline refs/heads/many_commits~$j
+data <<EOF
+changed note for commit #$i
+EOF
+INPUT_END
+ cat >>expect <<EXPECT_END
+ commit #$i
+ changed note for commit #$i
+EXPECT_END
+ i=$(($i - 1))
+ j=$(($j + 1))
+done
+
+test_expect_failure 'change a few existing notes' '
+
+ git fast-import <input &&
+ GIT_NOTES_REF=refs/notes/many_notes git log -n3 refs/heads/many_commits |
+ grep "^ " > actual &&
+ test_cmp expect actual
+
+'
+
+test_expect_failure 'verify that changing notes respect existing fanout' '
+
+ # None of the entries in the top-level notes tree should be a full SHA1
+ git ls-tree --name-only refs/notes/many_notes |
+ while read path
+ do
+ if test $(expr length "$path") -ge 40
+ then
+ return 1
+ fi
+ done
+
+'
+
remaining_notes=10
test_tick
cat >input <<INPUT_END
--
1.7.5.rc1.3.g4d7b
^ permalink raw reply related
* what are the chances of a 'pre-upload' hook?
From: Sitaram Chamarty @ 2011-11-25 3:16 UTC (permalink / raw)
To: Git Mailing List
(...and/or a post-upload hook)
Has this ever come up?
--
Sitaram
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Martin Fick @ 2011-11-25 3:18 UTC (permalink / raw)
To: Sitaram Chamarty, Git Mailing List
In-Reply-To: <CAMK1S_jaEWV=F6iHKZw_6u5ncDW0bPosNx-03W9bOLOfEEEY1Q@mail.gmail.com>
Sitaram Chamarty <sitaramc@gmail.com> wrote:
>(...and/or a post-upload hook)
>
>Has this ever come up?
>
Yes, previously it was seen as problematic (potentially too slow), but we are now open to having one. Patches welcome. :)
Employee of Qualcomm Innovation Center,Inc. which is a member of Code Aurora Forum
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Martin Fick @ 2011-11-25 3:22 UTC (permalink / raw)
To: Sitaram Chamarty, Git Mailing List, Sitaram Chamarty,
Git Mailing List
In-Reply-To: <dbf5142a-184b-46a2-9815-9ed82c95dfa7@email.android.com>
>Martin Fick <mfick@codeaurora.org> wrote:
>Sitaram Chamarty <sitaramc@gmail.com> wrote:
>
>>(...and/or a post-upload hook)
>>
>>Has this ever come up?
>
>Yes, previously it was seen as problematic (potentially too slow), but
>we are now open to having one. Patches welcome. :)
Doh sorry, I thought this was a Gerrit question! (Wrong mailing list)
Employee of Qualcomm Innovation Center,Inc. which is a member of Code Aurora Forum
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Sitaram Chamarty @ 2011-11-25 4:13 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CAMK1S_jaEWV=F6iHKZw_6u5ncDW0bPosNx-03W9bOLOfEEEY1Q@mail.gmail.com>
On Fri, Nov 25, 2011 at 8:46 AM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> (...and/or a post-upload hook)
>
> Has this ever come up?
Sorry for the google-fu fail and for replying to my own post.
http://git.661346.n2.nabble.com/Removal-of-post-upload-hook-td4394312.html
is the longest thread I (later) found.
The quick summary, in the words of Jeff (second post in that link) is:
"Because [upload]-pack runs as the user who is [fetching], not as the
repository owner. So by convincing you to [fetch from] my repository
in a multi-user environment, I convince you to run some arbitrary code
of mine."
My contention (today) is:
- you're already taking that risk for push
- so this would add a new risk only for people who fetch but don't push
- which, I submit, is a very small (if not almost empty) set of people
I may be wrong but I imagine shared environments are those where
almost everyone will push at least once in a while. It's a closed
group of people, probably all developers, etc etc etc...
Thanks for listening.
^ permalink raw reply
* Re: [PATCH] Fix https interactive authentication problem
From: Ruedi Steinmann @ 2011-11-25 12:10 UTC (permalink / raw)
To: git
In-Reply-To: <20111123225121.GA23357@sigill.intra.peff.net>
Jeff King <peff <at> peff.net> writes:
>
> Hmm. What version of git are you using?
>
Hi Jeff,
I'm a little short of time at the moment, I'll come to this back later. So just
the quick infos. I discovered the error in my preinstalled version of git
(1.7.5.4). I reproduced the error with the version of git I have the source code
of: (which I should have done before, sorry)
../git/git/git clone https://n.ethz.ch/...
Cloning into 'masterthesis'...
warning: templates not found /home/ruedi/share/git-core/templates
Username for 'n.ethz.ch':
Password for 'n.ethz.ch':
error: The requested URL returned error: 401 (curl_result = 22, http_code = 401,
sha1 = 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e)
error: Unable to find 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e under
https://n.ethz.ch/.../masterthesis.git
Cannot obtain needed commit 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e
while processing commit 0fa41a9d4b3282891173ec54c04e0e3763867807.
error: Fetch failed.
I am working with the git repository from https://github.com/gitster/git.git.
>From the first few lines of my commit from gitk:
Parent: 017d1e134545db0d162908f3538077eaa1f34fb6 (Update 1.7.8 draft release
notes in preparation for rc4)
Branch: master
Follows: v1.7.7.4, v1.7.8-rc3
Hope this helps
Cheers
Ruedi
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Andreas Ericsson @ 2011-11-25 13:09 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Git Mailing List
In-Reply-To: <CAMK1S_gh_CsWc-DnbOuUwn+H1i3skm99xzDbWe-wxsKKS0Qw-w@mail.gmail.com>
On 11/25/2011 05:13 AM, Sitaram Chamarty wrote:
> On Fri, Nov 25, 2011 at 8:46 AM, Sitaram Chamarty<sitaramc@gmail.com> wrote:
>> (...and/or a post-upload hook)
>>
>> Has this ever come up?
>
> Sorry for the google-fu fail and for replying to my own post.
> http://git.661346.n2.nabble.com/Removal-of-post-upload-hook-td4394312.html
> is the longest thread I (later) found.
>
> The quick summary, in the words of Jeff (second post in that link) is:
> "Because [upload]-pack runs as the user who is [fetching], not as the
> repository owner. So by convincing you to [fetch from] my repository
> in a multi-user environment, I convince you to run some arbitrary code
> of mine."
>
> My contention (today) is:
>
> - you're already taking that risk for push
> - so this would add a new risk only for people who fetch but don't push
> - which, I submit, is a very small (if not almost empty) set of people
>
People who fetch but don't push is, by far, the vast majority of git users.
Think of everyone fetching from any public software repository without
having write access to it. If you think of git.git and linux.git alone
I think it's safe to assume the number of "fetch-no-push" outnumber the
"push-and-whatnot" group by some quarter million to one.
> I may be wrong but I imagine shared environments are those where
> almost everyone will push at least once in a while. It's a closed
> group of people, probably all developers, etc etc etc...
>
Not really. We fetch from each other quite a lot at work, and from
each others semi-public repositories on a shared server where we've
all got accounts (ie, write access), but we very, very rarely push
into each others repositories. The sharepoint is the "official" repo
on the repo-server, which the buildbots gets its code from and where
everything to be released, maintained or handled in some way in the
future resides.
Anyways. Shooting down the arguments *against* pre-upload hooks are
quite silly if it's not combined with some fresh arguments *for* such
a hook.
So... What usecase do you envision where you'd need one?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
^ permalink raw reply
* Re: Infinite loop in cascade_filter_fn()
From: Carlos Martín Nieto @ 2011-11-25 14:31 UTC (permalink / raw)
To: Henrik Grubbström; +Cc: Git Mailing list, Junio C Hamano
In-Reply-To: <Pine.GSO.4.63.1111231801580.5099@shipon.roxen.com>
[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]
On Wed, Nov 23, 2011 at 06:40:47PM +0100, Henrik Grubbström wrote:
> Hi.
>
> My git repository walker just got bitten by what seems to be a
> reasonably new bug in convert.c:cascade_filter_fn() (git 1.7.8.rc3
> (gentoo)).
It looks like it's a bug between cascade_filter_fn and the actual
filter function lf_to_crlf_filter_fn that gets triggered when the
output buffer is too small. In this particular case, *isize_p=378 and
*osize_p=1 which causes cascade_filter_fn to feed the filter data
which it can't process because it doesn't have anywhere to put it.
I think that the function assumes that the output buffer is always
large enough, but there are many indirections, so it might be an
off-by-one.
>
> How to reproduce:
>
> git clone git@github.com:pikelang/Pike.git
>
> git checkout -f 0e2080f838c6f0bc7d670ac7549676a353451dca^
>
> git checkout -f 0e2080f838c6f0bc7d670ac7549676a353451dca
>
> The first two commands complete as expected, while the last hangs forever.
> Performing the same with git 1.7.6.4 works as expected.
>
> The problematic file seems to be
> /src/modules/_Crypto/rijndael_ecb_vt.txt which has the attributes:
> text ident eol=crlf
>
> Thanks,
>
> --
> Henrik Grubbström grubba@grubba.org
> Roxen Internet Software AB grubba@roxen.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox