{"thread":{"id":"29017","subject":"[PATCH 0/13] credential helpers, take two","startedAt":"2011-11-24T10:58:02Z","lastAt":"2011-11-30T05:41:37Z","messageCount":49,"participants":["Jeff King","Erik Faye-Lund","Eric Sunshine","Junio C Hamano","Frans Klaver","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"179932","messageId":"20111124105801.GA6168@sigill.intra.peff.net","threadId":"29017","inReplyTo":null,"subject":"[PATCH 0/13] credential helpers, take two","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T10:58:02Z","receivedAt":"2011-11-24T10:58:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's a revised version of the http-auth / credential-helper series.\n\nIt has the same basic premise as the first series (git contacts external\nprograms to access secure storage, thus enabling secure password\ncaching), but there are a number of tweaks in the implementation.\n\nThe main departures are:\n\n 1. Helpers now _only_ act as storage; they never ask for credentials\n directly from the user. This makes writing them much simpler.\n\n It also means that \"askpass\" remains the only way to get input from\n the user. However, I've left room in the helper protocol to add an\n \"ask\" action. If people really want something like a classy\n username/password dialog from the helpers, it will be easy to add.\n\n 2. The helper protocol now happens completely over pipes. In the first\n round, we gave information to helpers on the command line. That was\n OK, since we never gave them a password; they only gave them to us\n (and cached them if they wanted to). But now that git is asking for\n the password itself, it has to send the password to the helper to\n store. And that definitely shouldn't go on the command line.\n\n The parsing load on the helper is therefore a little higher.\n However, it's still really quite easy.\n\n 3. The old \"unique\" token has been broken into components. That means\n less parsing for most helpers, which wanted the broken-down fields.\n Helpers that want a single token can pretty easily reassemble.\n\n 4. I dropped the \"description\" field. I noticed that all of the\n components of a credential context are actually parts of a URL. So\n we can just show the URL (or a subset thereof) to the user in the\n prompt. See the discussion in patches 05 and 08.\n\n 5. Config handling happens at the right place (before helpers) now.\n\nIf you want an overview without reading the patches too carefully, I\nrecommend reading the documentation added in patches 03 and 09, which\ncontain the API and end-user documentation respectively.\n\nHelper writers may want to look at t0303 added in patch 13; it's an\nadaptation of the test script I posted earlier for testing new external\nhelpers.\n\n [01/13]: test-lib: add test_config_global variant\n [02/13]: t5550: fix typo\n [03/13]: introduce credentials API\n [04/13]: credential: add function for parsing url components\n [05/13]: http: use credential API to get passwords\n [06/13]: credential: apply helper config\n [07/13]: credential: add credential.*.username\n [08/13]: credential: make relevance of http path configurable\n [09/13]: docs: end-user documentation for the credential subsystem\n [10/13]: credentials: add \"cache\" helper\n [11/13]: strbuf: add strbuf_add*_urlencode\n [12/13]: credentials: add \"store\" helper\n [13/13]: t: add test harness for external credential helpers\n\nI've been running with this for a few days, so I think the most horrible\nbugs are shaken out. But I had to rewrite almost all of the code from\nthe first series, so I wouldn't be surprised if there are some new bugs\nlurking in there. If you are reviewing, please read from scratch and\ndon't assume that something that worked in the first series is still\nworking. :)\n\n-Peff\n"},{"id":"179933","messageId":"20111124105851.GA6195@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 01/13] test-lib: add test_config_global variant","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T10:58:51Z","receivedAt":"2011-11-24T10:58:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of test_config is to simultaneously set a config\nvariable and register its cleanup handler, like:\n\n test_config core.foo bar\n\nHowever, it stupidly assumes that $1 contained the name of\nthe variable, which means it won't work for:\n\n test_config --global core.foo bar\n\nWe could try to parse the command-line ourselves and figure\nout which parts need to be fed to test_unconfig. But since\nthis is likely the most common variant, it's much simpler\nand less error-prone to simply add a new function.\n\nSigned-off-by: Jeff King \n---\n t/test-lib.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex bdd9513..160479b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -379,6 +379,11 @@ test_config () {\n \tgit config \"$@\"\n }\n \n+test_config_global () {\n+\ttest_when_finished \"test_unconfig --global '$1'\" &&\n+\tgit config --global \"$@\"\n+}\n+\n # Use test_set_prereq to tell that a particular prerequisite is available.\n # The prerequisite can later be checked for in two ways:\n #\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179934","messageId":"20111124105914.GB6195@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 02/13] t5550: fix typo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T10:59:15Z","receivedAt":"2011-11-24T10:59:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This didn't have an impact, because it was just setting up\nan \"expect\" file that happened to be identical to the one in\nthe test before it.\n\nSigned-off-by: Jeff King \n---\n t/t5550-http-fetch.sh | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex 311a33c..3d6e871 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -66,7 +66,7 @@ test_expect_success 'cloning password-protected repository can fail' '\n \n test_expect_success 'http auth can use user/pass in URL' '\n \t>askpass-query &&\n-\techo wrong >askpass-reponse &&\n+\techo wrong >askpass-response &&\n \tgit clone \"$HTTPD_URL_USER_PASS/auth/repo.git\" clone-auth-none &&\n \ttest_cmp askpass-expect-none askpass-query\n '\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179935","messageId":"20111124110105.GA8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 03/13] introduce credentials API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:01:06Z","receivedAt":"2011-11-24T11:01:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are a few places in git that need to get a username\nand password credential from the user; the most notable one\nis HTTP authentication for smart-http pushing.\n\nRight now the only choices for providing credentials are to\nput them plaintext into your ~/.netrc, or to have git prompt\nyou (either on the terminal or via an askpass program). The\nformer is not very secure, and the latter is not very\nconvenient.\n\nUnfortunately, there is no \"always best\" solution for\npassword management. The details will depend on the tradeoff\nyou want between security and convenience, as well as how\ngit can integrate with other security systems (e.g., many\noperating systems provide a keychain or password wallet for\nsingle sign-on).\n\nThis patch provides an abstract notion of credentials as a\ndata item, and provides three basic operations:\n\n - fill (i.e., acquire from external storage or from the\n user)\n\n - approve (mark a credential as \"working\" for further\n storage)\n\n - reject (mark a credential as \"not working\", so it can\n be removed from storage)\n\nThese operations can be backed by external helper processes\nthat interact with system- or user-specific secure storage.\n\nSigned-off-by: Jeff King \n---\n .gitignore | 1 +\n Documentation/technical/api-credentials.txt | 148 ++++++++++++++++\n Makefile | 3 +\n credential.c | 242 +++++++++++++++++++++++++++\n credential.h | 28 +++\n t/lib-credential.sh | 33 ++++\n t/t0300-credentials.sh | 195 +++++++++++++++++++++\n test-credential.c | 38 ++++\n 8 files changed, 688 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/technical/api-credentials.txt\n create mode 100644 credential.c\n create mode 100644 credential.h\n create mode 100755 t/lib-credential.sh\n create mode 100755 t/t0300-credentials.sh\n create mode 100644 test-credential.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 8572c8c..7d2fefc 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -167,6 +167,7 @@\n /gitweb/static/gitweb.js\n /gitweb/static/gitweb.min.*\n /test-chmtime\n+/test-credential\n /test-ctype\n /test-date\n /test-delta\ndiff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\nnew file mode 100644\nindex 0000000..3061077\n--- /dev/null\n+++ b/Documentation/technical/api-credentials.txt\n@@ -0,0 +1,148 @@\n+credentials API\n+===============\n+\n+The credentials API provides an abstracted way of gathering username and\n+password credentials from the user (even though credentials in the wider\n+world can take many forms, in this document the word \"credential\" always\n+refers to a username and password pair).\n+\n+Data Structures\n+---------------\n+\n+`struct credential`::\n+\n+\tThis struct represents a single username/password combination\n+\talong with any associated context. All string fields should be\n+\theap-allocated (or NULL if they are not known or not applicable).\n+\tThe meaning of the individual context fields is the same as\n+\ttheir counterparts in the helper protocol; see the section below\n+\tfor a description of each field.\n++\n+The `helpers` member of the struct is a `string_list` of helpers. Each\n+string specifies an external helper which will be run, in order, to\n+either acquire or store credentials. See the section on credential\n+storage helpers below.\n++\n+This struct should always be initialized with `CREDENTIAL_INIT` or\n+`credential_init`.\n+\n+\n+Functions\n+---------\n+\n+`credential_init`::\n+\n+\tInitialize a credential structure, setting all fields to empty.\n+\n+`credential_clear`::\n+\n+\tFree any resources associated with the credential structure,\n+\treturning it to a pristine initialized state.\n+\n+`credential_fill`::\n+\n+\tAttempt to fill the username and password fields of the passed\n+\tcredential struct, first consulting storage helpers, then asking\n+\tthe user. Guarantees that the username and password fields will\n+\tbe filled afterwards (or die() will be called).\n+\n+`credential_reject`::\n+\n+\tInform the credential subsystem that the provided credentials\n+\thave been rejected. This will notify any storage helpers of the\n+\trejection (which allows them to, for example, purge the invalid\n+\tcredentials from storage), and then clear the username and\n+\tpassword fields in `struct credential`. It can then be\n+\t`credential_fill`-ed again.\n+\n+`credential_approve`::\n+\n+\tInform the credential subsystem that the provided credentials\n+\twere successfully used for authentication. This will notify any\n+\tstorage helpers of the approval, so that they can store the\n+\tresult to be used again.\n+\n+\n+Credential Storage Helpers\n+--------------------------\n+\n+Credential storage helpers are programs executed by git to fetch or save\n+credentials from and to long-term storage (where \"long-term\" is simply\n+longer than a single git process; e.g., credentials may be stored\n+in-memory for a few minutes, or indefinitely on disk).\n+\n+Helper scripts should generally be found in the PATH, and have names of\n+the form \"git-credential-$HELPER\". When the helper string \"$HELPER\" is\n+passed to credential functions, they will run \"git-credential-$HELPER\"\n+via the shell. If the first word of $HELPER contains non-alphanumeric\n+characters, then $HELPER is executed as a shell command. This makes it\n+possible to specify individual scripts by their full path (e.g.,\n+`/path/to/helper`) or even shell snippets (`f() { do_whatever; }; f`).\n+\n+When a helper is executed, it will have one \"operation\" argument\n+appended to its command line, which is one of:\n+\n+`get`::\n+\n+\tReturn a matching credential, if any exists.\n+\n+`store`::\n+\n+\tStore the credential, if applicable to the helper.\n+\n+`erase`::\n+\n+\tRemove a matching credential, if any, from the helper's storage.\n+\n+The details of the credential will be provided on the helper's stdin\n+stream. The credential is split into a set of named attributes.\n+Attributes are provided to the helper, one per line. Each attribute is\n+specified by a key-value pair, separated by an `=` (equals) sign,\n+followed by a newline. The key may contain any bytes except `=` or\n+newline. The value may contain any bytes except a newline. In both\n+cases, all bytes are treated as-is (i.e., there is no quoting, and one\n+cannot transmit a value with newline in it). The list of attributes is\n+terminated by a blank line or end-of-file.\n+\n+Git will send the following attributes (but may not send all of\n+them for a given credential; for example, a `host` attribute makes no\n+sense when dealing with a non-network protocol):\n+\n+`protocol`::\n+\n+\tThe protocol over which the credential will be used (e.g.,\n+\t`https`).\n+\n+`host`::\n+\n+\tThe remote hostname for a network credential.\n+\n+`path`::\n+\n+\tThe path with which the credential will be used. E.g., for\n+\taccessing a remote https repository, this will be the\n+\trepository's path on the server.\n+\n+`username`::\n+\n+\tThe credential's username, if we already have one (e.g., from a\n+\tURL, from the user, or from a previously run helper).\n+\n+`password`::\n+\n+\tThe credential's password, if we are asking it to be stored.\n+\n+For a `get` operation, the helper should produce a list of attributes\n+on stdout in the same format. A helper is free to produce a subset, or\n+even no values at all if it has nothing useful to provide. Any provided\n+attributes will overwrite those already known about by git.\n+\n+For a `store` or `erase` operation, the helper's output is ignored.\n+If it fails to perform the requested operation, it may complain to\n+stderr to inform the user. If it does not support the requested\n+operation (e.g., a read-only store), it should silently ignore the\n+request.\n+\n+If a helper receives any other operation, it should silently ignore the\n+request. This leaves room for future operations to be added (older\n+helpers will just ignore the new requests).\ndiff --git a/Makefile b/Makefile\nindex b1c80a6..5ca363b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -431,6 +431,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \n TEST_PROGRAMS_NEED_X += test-chmtime\n+TEST_PROGRAMS_NEED_X += test-credential\n TEST_PROGRAMS_NEED_X += test-ctype\n TEST_PROGRAMS_NEED_X += test-date\n TEST_PROGRAMS_NEED_X += test-delta\n@@ -525,6 +526,7 @@ LIB_H += compat/win32/poll.h\n LIB_H += compat/win32/dirent.h\n LIB_H += connected.h\n LIB_H += convert.h\n+LIB_H += credential.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -609,6 +611,7 @@ LIB_OBJS += connect.o\n LIB_OBJS += connected.o\n LIB_OBJS += convert.o\n LIB_OBJS += copy.o\n+LIB_OBJS += credential.o\n LIB_OBJS += csum-file.o\n LIB_OBJS += ctype.o\n LIB_OBJS += date.o\ndiff --git a/credential.c b/credential.c\nnew file mode 100644\nindex 0000000..d50a611\n--- /dev/null\n+++ b/credential.c\n@@ -0,0 +1,242 @@\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"string-list.h\"\n+#include \"run-command.h\"\n+\n+void credential_init(struct credential *c)\n+{\n+\tmemset(c, 0, sizeof(*c));\n+\tc->helpers.strdup_strings = 1;\n+}\n+\n+void credential_clear(struct credential *c)\n+{\n+\tfree(c->protocol);\n+\tfree(c->host);\n+\tfree(c->path);\n+\tfree(c->username);\n+\tfree(c->password);\n+\tstring_list_clear(&c->helpers, 0);\n+\n+\tcredential_init(c);\n+}\n+\n+static void credential_describe(struct credential *c, struct strbuf *out)\n+{\n+\tif (!c->protocol)\n+\t\treturn;\n+\tstrbuf_addf(out, \"%s://\", c->protocol);\n+\tif (c->username && *c->username)\n+\t\tstrbuf_addf(out, \"%s@\", c->username);\n+\tif (c->host)\n+\t\tstrbuf_addstr(out, c->host);\n+\tif (c->path)\n+\t\tstrbuf_addf(out, \"/%s\", c->path);\n+}\n+\n+static char *credential_ask_one(const char *what, struct credential *c)\n+{\n+\tstruct strbuf desc = STRBUF_INIT;\n+\tstruct strbuf prompt = STRBUF_INIT;\n+\tchar *r;\n+\n+\tcredential_describe(c, &desc);\n+\tif (desc.len)\n+\t\tstrbuf_addf(&prompt, \"%s for '%s': \", what, desc.buf);\n+\telse\n+\t\tstrbuf_addf(&prompt, \"%s: \", what);\n+\n+\t/* FIXME: for usernames, we should do something less magical that\n+\t * actually echoes the characters. However, we need to read from\n+\t * /dev/tty and not stdio, which is not portable (but getpass will do\n+\t * it for us). http.c uses the same workaround. */\n+\tr = git_getpass(prompt.buf);\n+\n+\tstrbuf_release(&desc);\n+\tstrbuf_release(&prompt);\n+\treturn xstrdup(r);\n+}\n+\n+static void credential_getpass(struct credential *c)\n+{\n+\tif (!c->username)\n+\t\tc->username = credential_ask_one(\"Username\", c);\n+\tif (!c->password)\n+\t\tc->password = credential_ask_one(\"Password\", c);\n+}\n+\n+int credential_read(struct credential *c, FILE *fp)\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\n+\twhile (strbuf_getline(&line, fp, '\\n') != EOF) {\n+\t\tchar *key = line.buf;\n+\t\tchar *value = strchr(key, '=');\n+\n+\t\tif (!line.len)\n+\t\t\tbreak;\n+\n+\t\tif (!value) {\n+\t\t\twarning(\"invalid credential line: %s\", key);\n+\t\t\tstrbuf_release(&line);\n+\t\t\treturn -1;\n+\t\t}\n+\t\t*value++ = '\\0';\n+\n+\t\tif (!strcmp(key, \"username\")) {\n+\t\t\tfree(c->username);\n+\t\t\tc->username = xstrdup(value);\n+\t\t}\n+\t\telse if (!strcmp(key, \"password\")) {\n+\t\t\tfree(c->password);\n+\t\t\tc->password = xstrdup(value);\n+\t\t}\n+\t\telse if (!strcmp(key, \"protocol\")) {\n+\t\t\tfree(c->protocol);\n+\t\t\tc->protocol = xstrdup(value);\n+\t\t}\n+\t\telse if (!strcmp(key, \"host\")) {\n+\t\t\tfree(c->host);\n+\t\t\tc->host = xstrdup(value);\n+\t\t}\n+\t\telse if (!strcmp(key, \"path\")) {\n+\t\t\tfree(c->path);\n+\t\t\tc->path = xstrdup(value);\n+\t\t}\n+\t\t/* ignore other lines; we don't know what they mean, but\n+\t\t * this future-proofs us when later versions of git do\n+\t\t * learn new lines, and the helpers are updated to match */\n+\t}\n+\n+\tstrbuf_release(&line);\n+\treturn 0;\n+}\n+\n+static void credential_write_item(FILE *fp, const char *key, const char *value)\n+{\n+\tif (!value)\n+\t\treturn;\n+\tfprintf(fp, \"%s=%s\\n\", key, value);\n+}\n+\n+static void credential_write(const struct credential *c, FILE *fp)\n+{\n+\tcredential_write_item(fp, \"protocol\", c->protocol);\n+\tcredential_write_item(fp, \"host\", c->host);\n+\tcredential_write_item(fp, \"path\", c->path);\n+\tcredential_write_item(fp, \"username\", c->username);\n+\tcredential_write_item(fp, \"password\", c->password);\n+}\n+\n+static int first_word_is_alnum(const char *s)\n+{\n+\tfor (; *s && *s != ' '; s++)\n+\t\tif (!isalnum(*s))\n+\t\t\treturn 0;\n+\treturn 1;\n+}\n+\n+static int run_credential_helper(struct credential *c,\n+\t\t\t\t const char *cmd,\n+\t\t\t\t int want_output)\n+{\n+\tstruct child_process helper;\n+\tconst char *argv[] = { NULL, NULL };\n+\tFILE *fp;\n+\n+\tmemset(&helper, 0, sizeof(helper));\n+\targv[0] = cmd;\n+\thelper.argv = argv;\n+\thelper.use_shell = 1;\n+\thelper.in = -1;\n+\tif (want_output)\n+\t\thelper.out = -1;\n+\telse\n+\t\thelper.no_stdout = 1;\n+\n+\tif (start_command(&helper) < 0)\n+\t\treturn -1;\n+\n+\tfp = xfdopen(helper.in, \"w\");\n+\tcredential_write(c, fp);\n+\tfclose(fp);\n+\n+\tif (want_output) {\n+\t\tint r;\n+\t\tfp = xfdopen(helper.out, \"r\");\n+\t\tr = credential_read(c, fp);\n+\t\tfclose(fp);\n+\t\tif (r < 0) {\n+\t\t\tfinish_command(&helper);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\n+\tif (finish_command(&helper))\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static int credential_do(struct credential *c, const char *method,\n+\t\t\t const char *operation)\n+{\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tint r;\n+\n+\tif (first_word_is_alnum(method))\n+\t\tstrbuf_addf(&cmd, \"git credential-%s\", method);\n+\telse\n+\t\tstrbuf_addstr(&cmd, method);\n+\tstrbuf_addf(&cmd, \" %s\", operation);\n+\n+\tr = run_credential_helper(c, cmd.buf, !strcmp(operation, \"get\"));\n+\n+\tstrbuf_release(&cmd);\n+\treturn r;\n+}\n+\n+void credential_fill(struct credential *c)\n+{\n+\tint i;\n+\n+\tif (c->username && c->password)\n+\t\treturn;\n+\n+\tfor (i = 0; i < c->helpers.nr; i++) {\n+\t\tcredential_do(c, c->helpers.items[i].string, \"get\");\n+\t\tif (c->username && c->password)\n+\t\t\treturn;\n+\t}\n+\n+\tcredential_getpass(c);\n+\tif (!c->username && !c->password)\n+\t\tdie(\"unable to get password from user\");\n+}\n+\n+void credential_approve(struct credential *c)\n+{\n+\tint i;\n+\n+\tif (c->approved)\n+\t\treturn;\n+\tif (!c->username || !c->password)\n+\t\treturn;\n+\n+\tfor (i = 0; i < c->helpers.nr; i++)\n+\t\tcredential_do(c, c->helpers.items[i].string, \"store\");\n+\tc->approved = 1;\n+}\n+\n+void credential_reject(struct credential *c)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < c->helpers.nr; i++)\n+\t\tcredential_do(c, c->helpers.items[i].string, \"erase\");\n+\n+\tfree(c->username);\n+\tc->username = NULL;\n+\tfree(c->password);\n+\tc->password = NULL;\n+\tc->approved = 0;\n+}\ndiff --git a/credential.h b/credential.h\nnew file mode 100644\nindex 0000000..2ea7d49\n--- /dev/null\n+++ b/credential.h\n@@ -0,0 +1,28 @@\n+#ifndef CREDENTIAL_H\n+#define CREDENTIAL_H\n+\n+#include \"string-list.h\"\n+\n+struct credential {\n+\tstruct string_list helpers;\n+\tunsigned approved:1;\n+\n+\tchar *username;\n+\tchar *password;\n+\tchar *protocol;\n+\tchar *host;\n+\tchar *path;\n+};\n+\n+#define CREDENTIAL_INIT { STRING_LIST_INIT_DUP }\n+\n+void credential_init(struct credential *);\n+void credential_clear(struct credential *);\n+\n+void credential_fill(struct credential *);\n+void credential_approve(struct credential *);\n+void credential_reject(struct credential *);\n+\n+int credential_read(struct credential *, FILE *);\n+\n+#endif /* CREDENTIAL_H */\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nnew file mode 100755\nindex 0000000..54ae1f4\n--- /dev/null\n+++ b/t/lib-credential.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+# Try a set of credential helpers; the expected stdin,\n+# stdout and stderr should be provided on stdin,\n+# separated by \"--\".\n+check() {\n+\tread_chunk >stdin &&\n+\tread_chunk >expect-stdout &&\n+\tread_chunk >expect-stderr &&\n+\ttest-credential \"$@\" stdout 2>stderr &&\n+\ttest_cmp expect-stdout stdout &&\n+\ttest_cmp expect-stderr stderr\n+}\n+\n+read_chunk() {\n+\twhile read line; do\n+\t\tcase \"$line\" in\n+\t\t--) break ;;\n+\t\t*) echo \"$line\" ;;\n+\t\tesac\n+\tdone\n+}\n+\n+\n+cat >askpass <<\\EOF\n+#!/bin/sh\n+echo >&2 askpass: $*\n+what=`echo $1 | cut -d\" \" -f1 | tr A-Z a-z | tr -cd a-z`\n+echo \"askpass-$what\"\n+EOF\n+chmod +x askpass\n+GIT_ASKPASS=\"$PWD/askpass\"\n+export GIT_ASKPASS\ndiff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh\nnew file mode 100755\nindex 0000000..81a455f\n--- /dev/null\n+++ b/t/t0300-credentials.sh\n@@ -0,0 +1,195 @@\n+#!/bin/sh\n+\n+test_description='basic credential helper tests'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-credential.sh\n+\n+test_expect_success 'setup helper scripts' '\n+\tcat >dump <<-\\EOF &&\n+\twhoami=`echo $0 | sed s/.*git-credential-//`\n+\techo >&2 \"$whoami: $*\"\n+\twhile IFS== read key value; do\n+\t\techo >&2 \"$whoami: $key=$value\"\n+\t\teval \"$key=$value\"\n+\tdone\n+\tEOF\n+\n+\tcat >git-credential-useless <<-\\EOF &&\n+\t#!/bin/sh\n+\t. ./dump\n+\texit 0\n+\tEOF\n+\tchmod +x git-credential-useless &&\n+\n+\tcat >git-credential-verbatim <<-\\EOF &&\n+\t#!/bin/sh\n+\tuser=$1; shift\n+\tpass=$1; shift\n+\t. ./dump\n+\ttest -z \"$user\" || echo username=$user\n+\ttest -z \"$pass\" || echo password=$pass\n+\tEOF\n+\tchmod +x git-credential-verbatim &&\n+\n+\tPATH=\"$PWD:$PATH\"\n+'\n+\n+test_expect_success 'credential_fill invokes helper' '\n+\tcheck fill \"verbatim foo bar\" <<-\\EOF\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tverbatim: get\n+\tEOF\n+'\n+\n+test_expect_success 'credential_fill invokes multiple helpers' '\n+\tcheck fill useless \"verbatim foo bar\" <<-\\EOF\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tuseless: get\n+\tverbatim: get\n+\tEOF\n+'\n+\n+test_expect_success 'credential_fill stops when we get a full response' '\n+\tcheck fill \"verbatim one two\" \"verbatim three four\" <<-\\EOF\n+\t--\n+\tusername=one\n+\tpassword=two\n+\t--\n+\tverbatim: get\n+\tEOF\n+'\n+\n+test_expect_success 'credential_fill continues through partial response' '\n+\tcheck fill \"verbatim one \\\"\\\"\" \"verbatim two three\" <<-\\EOF\n+\t--\n+\tusername=two\n+\tpassword=three\n+\t--\n+\tverbatim: get\n+\tverbatim: get\n+\tverbatim: username=one\n+\tEOF\n+'\n+\n+test_expect_success 'credential_fill passes along metadata' '\n+\tcheck fill \"verbatim one two\" <<-\\EOF\n+\tprotocol=ftp\n+\thost=example.com\n+\tpath=foo.git\n+\t--\n+\tusername=one\n+\tpassword=two\n+\t--\n+\tverbatim: get\n+\tverbatim: protocol=ftp\n+\tverbatim: host=example.com\n+\tverbatim: path=foo.git\n+\tEOF\n+'\n+\n+test_expect_success 'credential_approve calls all helpers' '\n+\tcheck approve useless \"verbatim one two\" <<-\\EOF\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\t--\n+\tuseless: store\n+\tuseless: username=foo\n+\tuseless: password=bar\n+\tverbatim: store\n+\tverbatim: username=foo\n+\tverbatim: password=bar\n+\tEOF\n+'\n+\n+test_expect_success 'do not bother storing password-less credential' '\n+\tcheck approve useless <<-\\EOF\n+\tusername=foo\n+\t--\n+\t--\n+\tEOF\n+'\n+\n+\n+test_expect_success 'credential_reject calls all helpers' '\n+\tcheck reject useless \"verbatim one two\" <<-\\EOF\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\t--\n+\tuseless: erase\n+\tuseless: username=foo\n+\tuseless: password=bar\n+\tverbatim: erase\n+\tverbatim: username=foo\n+\tverbatim: password=bar\n+\tEOF\n+'\n+\n+test_expect_success 'usernames can be preserved' '\n+\tcheck fill \"verbatim \\\"\\\" three\" <<-\\EOF\n+\tusername=one\n+\t--\n+\tusername=one\n+\tpassword=three\n+\t--\n+\tverbatim: get\n+\tverbatim: username=one\n+\tEOF\n+'\n+\n+test_expect_success 'usernames can be overridden' '\n+\tcheck fill \"verbatim two three\" <<-\\EOF\n+\tusername=one\n+\t--\n+\tusername=two\n+\tpassword=three\n+\t--\n+\tverbatim: get\n+\tverbatim: username=one\n+\tEOF\n+'\n+\n+test_expect_success 'do not bother completing already-full credential' '\n+\tcheck fill \"verbatim three four\" <<-\\EOF\n+\tusername=one\n+\tpassword=two\n+\t--\n+\tusername=one\n+\tpassword=two\n+\t--\n+\tEOF\n+'\n+\n+# We can't test the basic terminal password prompt here because\n+# getpass() tries too hard to find the real terminal. But if our\n+# askpass helper is run, we know the internal getpass is working.\n+test_expect_success 'empty helper list falls back to internal getpass' '\n+\tcheck fill <<-\\EOF\n+\t--\n+\tusername=askpass-username\n+\tpassword=askpass-password\n+\t--\n+\taskpass: Username:\n+\taskpass: Password:\n+\tEOF\n+'\n+\n+test_expect_success 'internal getpass does not ask for known username' '\n+\tcheck fill <<-\\EOF\n+\tusername=foo\n+\t--\n+\tusername=foo\n+\tpassword=askpass-password\n+\t--\n+\taskpass: Password:\n+\tEOF\n+'\n+\n+test_done\ndiff --git a/test-credential.c b/test-credential.c\nnew file mode 100644\nindex 0000000..dee200e\n--- /dev/null\n+++ b/test-credential.c\n@@ -0,0 +1,38 @@\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"string-list.h\"\n+\n+static const char usage_msg[] =\n+\"test-credential [helper...]\";\n+\n+int main(int argc, const char **argv)\n+{\n+\tconst char *op;\n+\tstruct credential c = CREDENTIAL_INIT;\n+\tint i;\n+\n+\top = argv[1];\n+\tif (!op)\n+\t\tusage(usage_msg);\n+\tfor (i = 2; i < argc; i++)\n+\t\tstring_list_append(&c.helpers, argv[i]);\n+\n+\tif (credential_read(&c, stdin) < 0)\n+\t\tdie(\"unable to read credential from stdin\");\n+\n+\tif (!strcmp(op, \"fill\")) {\n+\t\tcredential_fill(&c);\n+\t\tif (c.username)\n+\t\t\tprintf(\"username=%s\\n\", c.username);\n+\t\tif (c.password)\n+\t\t\tprintf(\"password=%s\\n\", c.password);\n+\t}\n+\telse if (!strcmp(op, \"approve\"))\n+\t\tcredential_approve(&c);\n+\telse if (!strcmp(op, \"reject\"))\n+\t\tcredential_reject(&c);\n+\telse\n+\t\tusage(usage_msg);\n+\n+\treturn 0;\n+}\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179936","messageId":"20111124110122.GB8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 04/13] credential: add function for parsing url components","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:01:22Z","receivedAt":"2011-11-24T11:01:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"All of the components of a credential struct can be found in\na URL. For example, the URL:\n\n http://foo:bar@example.com/repo.git\n\ncontains:\n\n protocol=http\n host=example.com\n path=repo.git\n username=foo\n password=bar\n\nWe want to be able to turn URLs into broken-down credential\nstructs so that we know two things:\n\n 1. Which parts of the username/password we still need\n\n 2. What the context of the request is (for prompting or\n as a key for storing credentials).\n\nThis code is based on http_auth_init in http.c, but needed a\nfew modifications in order to get all of the components that\nthe credential object is interested in.\n\nOnce the http code is switched over to the credential API,\nthen http_auth_init can just go away.\n\nSigned-off-by: Jeff King \n---\n Documentation/technical/api-credentials.txt | 3 ++\n credential.c | 52 +++++++++++++++++++++++++++\n credential.h | 1 +\n 3 files changed, 56 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\nindex 3061077..0dca0bf 100644\n--- a/Documentation/technical/api-credentials.txt\n+++ b/Documentation/technical/api-credentials.txt\n@@ -62,6 +62,9 @@ Functions\n \tstorage helpers of the approval, so that they can store the\n \tresult to be used again.\n \n+`credential_from_url`::\n+\n+\tParse a URL into broken-down credential fields.\n \n Credential Storage Helpers\n --------------------------\ndiff --git a/credential.c b/credential.c\nindex d50a611..5cc4319 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -2,6 +2,7 @@\n #include \"credential.h\"\n #include \"string-list.h\"\n #include \"run-command.h\"\n+#include \"url.h\"\n \n void credential_init(struct credential *c)\n {\n@@ -240,3 +241,54 @@ void credential_reject(struct credential *c)\n \tc->password = NULL;\n \tc->approved = 0;\n }\n+\n+void credential_from_url(struct credential *c, const char *url)\n+{\n+\tconst char *at, *colon, *cp, *slash, *host, *proto_end;\n+\n+\tcredential_clear(c);\n+\n+\t/*\n+\t * Match one of:\n+\t * (1) proto:///...\n+\t * (2) proto://@/...\n+\t * (3) proto://:@/...\n+\t */\n+\tproto_end = strstr(url, \"://\");\n+\tif (!proto_end)\n+\t\treturn;\n+\tcp = proto_end + 3;\n+\tat = strchr(cp, '@');\n+\tcolon = strchr(cp, ':');\n+\tslash = strchrnul(cp, '/');\n+\n+\tif (!at || slash <= at) {\n+\t\t/* Case (1) */\n+\t\thost = cp;\n+\t}\n+\telse if (!colon || at <= colon) {\n+\t\t/* Case (2) */\n+\t\tc->username = url_decode_mem(cp, at - cp);\n+\t\thost = at + 1;\n+\t} else {\n+\t\t/* Case (3) */\n+\t\tc->username = url_decode_mem(cp, colon - cp);\n+\t\tc->password = url_decode_mem(colon + 1, at - (colon + 1));\n+\t\thost = at + 1;\n+\t}\n+\n+\tif (proto_end - url > 0)\n+\t\tc->protocol = xmemdupz(url, proto_end - url);\n+\tif (slash - host > 0)\n+\t\tc->host = url_decode_mem(host, slash - host);\n+\t/* Trim leading and trailing slashes from path */\n+\twhile (*slash == '/')\n+\t\tslash++;\n+\tif (*slash) {\n+\t\tchar *p;\n+\t\tc->path = url_decode(slash);\n+\t\tp = c->path + strlen(c->path) - 1;\n+\t\twhile (p > c->path && *p == '/')\n+\t\t\t*p-- = '\\0';\n+\t}\n+}\ndiff --git a/credential.h b/credential.h\nindex 2ea7d49..8a6d162 100644\n--- a/credential.h\n+++ b/credential.h\n@@ -24,5 +24,6 @@ struct credential {\n void credential_reject(struct credential *);\n \n int credential_read(struct credential *, FILE *);\n+void credential_from_url(struct credential *, const char *url);\n \n #endif /* CREDENTIAL_H */\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179937","messageId":"20111124110150.GC8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 05/13] http: use credential API to get passwords","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:01:50Z","receivedAt":"2011-11-24T11:01:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This patch converts the http code to use the new credential\nAPI, both for http authentication as well as for getting\ncertificate passwords.\n\nMost of the code change is simply variable naming (the\npasswords are now contained inside the credential struct)\nor deletion of obsolete code (the credential code handles\nURL parsing and prompting for us).\n\nThe behavior should be the same, with one exception: the\ncredential code will prompt with a description based on the\ncredential components. Therefore, the old prompt of:\n\n Username for 'example.com':\n Password for 'example.com':\n\nnow looks like:\n\n Username for 'https://example.com/repo.git':\n Password for 'https://user@example.com/repo.git':\n\nNote that we include more information in each line,\nspecifically:\n\n 1. We now include the protocol. While more noisy, this is\n an important part of knowing what you are accessing\n (especially if you care about http vs https).\n\n 2. We include the username in the password prompt. This is\n not a big deal when you have just been prompted for it,\n but the username may also come from the remote's URL\n (and after future patches, from configuration or\n credential helpers). In that case, it's a nice\n reminder of the user for which you're giving the\n password.\n\n 3. We include the path component of the URL. In many\n cases, the user won't care about this and it's simply\n noise (i.e., they'll use the same credential for a\n whole site). However, that is part of a larger\n question, which is whether path components should be\n part of credential context, both for prompting and for\n lookup by storage helpers. That issue will be addressed\n as a whole in a future patch.\n\nSimilarly, for unlocking certificates, we used to say:\n\n Certificate Password for 'example.com':\n\nand we now say:\n\n Password for 'cert:///path/to/certificate':\n\nShowing the path to the client certificate makes more sense,\nas that is what you are unlocking, not \"example.com\".\n\nSigned-off-by: Jeff King \n---\n http.c | 113 +++++++++++--------------------------------------\n t/t5550-http-fetch.sh | 38 ++++++++++++-----\n 2 files changed, 52 insertions(+), 99 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex e6c7597..917a1ae 100644\n--- a/http.c\n+++ b/http.c\n@@ -3,6 +3,7 @@\n #include \"sideband.h\"\n #include \"run-command.h\"\n #include \"url.h\"\n+#include \"credential.h\"\n \n int data_received;\n int active_requests;\n@@ -42,7 +43,7 @@\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n static const char *curl_cookie_file;\n-static char *user_name, *user_pass, *description;\n+static struct credential http_auth = CREDENTIAL_INIT;\n static const char *user_agent;\n \n #if LIBCURL_VERSION_NUM >= 0x071700\n@@ -53,7 +54,7 @@\n #define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD\n #endif\n \n-static char *ssl_cert_password;\n+static struct credential cert_auth = CREDENTIAL_INIT;\n static int ssl_cert_password_required;\n \n static struct curl_slist *pragma_header;\n@@ -139,27 +140,6 @@ static void process_curl_messages(void)\n }\n #endif\n \n-static char *git_getpass_with_description(const char *what, const char *desc)\n-{\n-\tstruct strbuf prompt = STRBUF_INIT;\n-\tchar *r;\n-\n-\tif (desc)\n-\t\tstrbuf_addf(&prompt, \"%s for '%s': \", what, desc);\n-\telse\n-\t\tstrbuf_addf(&prompt, \"%s: \", what);\n-\t/*\n-\t * NEEDSWORK: for usernames, we should do something less magical that\n-\t * actually echoes the characters. However, we need to read from\n-\t * /dev/tty and not stdio, which is not portable (but getpass will do\n-\t * it for us). http.c uses the same workaround.\n-\t */\n-\tr = git_getpass(prompt.buf);\n-\n-\tstrbuf_release(&prompt);\n-\treturn xstrdup(r);\n-}\n-\n static int http_options(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(\"http.sslverify\", var)) {\n@@ -232,11 +212,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \n static void init_curl_http_auth(CURL *result)\n {\n-\tif (user_name) {\n+\tif (http_auth.username) {\n \t\tstruct strbuf up = STRBUF_INIT;\n-\t\tif (!user_pass)\n-\t\t\tuser_pass = xstrdup(git_getpass_with_description(\"Password\", description));\n-\t\tstrbuf_addf(&up, \"%s:%s\", user_name, user_pass);\n+\t\tcredential_fill(&http_auth);\n+\t\tstrbuf_addf(&up, \"%s:%s\",\n+\t\t\t http_auth.username, http_auth.password);\n \t\tcurl_easy_setopt(result, CURLOPT_USERPWD,\n \t\t\t\t strbuf_detach(&up, NULL));\n \t}\n@@ -244,18 +224,14 @@ static void init_curl_http_auth(CURL *result)\n \n static int has_cert_password(void)\n {\n-\tif (ssl_cert_password != NULL)\n-\t\treturn 1;\n \tif (ssl_cert == NULL || ssl_cert_password_required != 1)\n \t\treturn 0;\n-\t/* Only prompt the user once. */\n-\tssl_cert_password_required = -1;\n-\tssl_cert_password = git_getpass_with_description(\"Certificate Password\", description);\n-\tif (ssl_cert_password != NULL) {\n-\t\tssl_cert_password = xstrdup(ssl_cert_password);\n-\t\treturn 1;\n-\t} else\n-\t\treturn 0;\n+\tif (!cert_auth.password) {\n+\t\tcert_auth.protocol = xstrdup(\"cert\");\n+\t\tcert_auth.path = xstrdup(ssl_cert);\n+\t\tcredential_fill(&cert_auth);\n+\t}\n+\treturn 1;\n }\n \n static CURL *get_curl_handle(void)\n@@ -282,7 +258,7 @@ static int has_cert_password(void)\n \tif (ssl_cert != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n \tif (has_cert_password())\n-\t\tcurl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);\n+\t\tcurl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);\n #if LIBCURL_VERSION_NUM >= 0x070903\n \tif (ssl_key != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n@@ -324,42 +300,6 @@ static int has_cert_password(void)\n \treturn result;\n }\n \n-static void http_auth_init(const char *url)\n-{\n-\tconst char *at, *colon, *cp, *slash, *host;\n-\n-\tcp = strstr(url, \"://\");\n-\tif (!cp)\n-\t\treturn;\n-\n-\t/*\n-\t * Ok, the URL looks like \"proto://something\". Which one?\n-\t * \"proto://:@/...\",\n-\t * \"proto://@/...\", or just\n-\t * \"proto:///...\"?\n-\t */\n-\tcp += 3;\n-\tat = strchr(cp, '@');\n-\tcolon = strchr(cp, ':');\n-\tslash = strchrnul(cp, '/');\n-\tif (!at || slash <= at) {\n-\t\t/* No credentials, but we may have to ask for some later */\n-\t\thost = cp;\n-\t}\n-\telse if (!colon || at <= colon) {\n-\t\t/* Only username */\n-\t\tuser_name = url_decode_mem(cp, at - cp);\n-\t\tuser_pass = NULL;\n-\t\thost = at + 1;\n-\t} else {\n-\t\tuser_name = url_decode_mem(cp, colon - cp);\n-\t\tuser_pass = url_decode_mem(colon + 1, at - (colon + 1));\n-\t\thost = at + 1;\n-\t}\n-\n-\tdescription = url_decode_mem(host, slash - host);\n-}\n-\n static void set_from_env(const char **var, const char *envname)\n {\n \tconst char *val = getenv(envname);\n@@ -432,7 +372,7 @@ void http_init(struct remote *remote, const char *url)\n \t\tcurl_ftp_no_epsv = 1;\n \n \tif (url) {\n-\t\thttp_auth_init(url);\n+\t\tcredential_from_url(&http_auth, url);\n \t\tif (!ssl_cert_password_required &&\n \t\t getenv(\"GIT_SSL_CERT_PASSWORD_PROTECTED\") &&\n \t\t !prefixcmp(url, \"https://\"))\n@@ -481,10 +421,10 @@ void http_cleanup(void)\n \t\tcurl_http_proxy = NULL;\n \t}\n \n-\tif (ssl_cert_password != NULL) {\n-\t\tmemset(ssl_cert_password, 0, strlen(ssl_cert_password));\n-\t\tfree(ssl_cert_password);\n-\t\tssl_cert_password = NULL;\n+\tif (cert_auth.password != NULL) {\n+\t\tmemset(cert_auth.password, 0, strlen(cert_auth.password));\n+\t\tfree(cert_auth.password);\n+\t\tcert_auth.password = NULL;\n \t}\n \tssl_cert_password_required = 0;\n }\n@@ -836,17 +776,11 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\telse if (missing_target(&results))\n \t\t\tret = HTTP_MISSING_TARGET;\n \t\telse if (results.http_code == 401) {\n-\t\t\tif (user_name && user_pass) {\n+\t\t\tif (http_auth.username && http_auth.password) {\n+\t\t\t\tcredential_reject(&http_auth);\n \t\t\t\tret = HTTP_NOAUTH;\n \t\t\t} else {\n-\t\t\t\t/*\n-\t\t\t\t * git_getpass is needed here because its very likely stdin/stdout are\n-\t\t\t\t * pipes to our parent process. So we instead need to use /dev/tty,\n-\t\t\t\t * but that is non-portable. Using git_getpass() can at least be stubbed\n-\t\t\t\t * on other platforms with a different implementation if/when necessary.\n-\t\t\t\t */\n-\t\t\t\tif (!user_name)\n-\t\t\t\t\tuser_name = xstrdup(git_getpass_with_description(\"Username\", description));\n+\t\t\t\tcredential_fill(&http_auth);\n \t\t\t\tinit_curl_http_auth(slot->curl);\n \t\t\t\tret = HTTP_REAUTH;\n \t\t\t}\n@@ -866,6 +800,9 @@ static int http_request(const char *url, void *result, int target, int options)\n \tcurl_slist_free_all(headers);\n \tstrbuf_release(&buf);\n \n+\tif (ret == HTTP_OK)\n+\t\tcredential_approve(&http_auth);\n+\n \treturn ret;\n }\n \ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex 3d6e871..398a2d2 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -49,40 +49,56 @@ test_expect_success 'setup askpass helpers' '\n \tEOF\n \tchmod +x askpass &&\n \tGIT_ASKPASS=\"$PWD/askpass\" &&\n-\texport GIT_ASKPASS &&\n-\t>askpass-expect-none &&\n-\techo \"askpass: Password for '\\''$HTTPD_DEST'\\'': \" >askpass-expect-pass &&\n-\t{ echo \"askpass: Username for '\\''$HTTPD_DEST'\\'': \" &&\n-\t cat askpass-expect-pass\n-\t} >askpass-expect-both\n-'\n+\texport GIT_ASKPASS\n+'\n+\n+expect_askpass() {\n+\tdest=$HTTPD_DEST/auth/repo.git\n+\t{\n+\t\tcase \"$1\" in\n+\t\tnone)\n+\t\t\t;;\n+\t\tpass)\n+\t\t\techo \"askpass: Password for 'http://$2@$dest': \"\n+\t\t\t;;\n+\t\tboth)\n+\t\t\techo \"askpass: Username for 'http://$dest': \"\n+\t\t\techo \"askpass: Password for 'http://$2@$dest': \"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tfalse\n+\t\t\t;;\n+\t\tesac\n+\t} >askpass-expect &&\n+\ttest_cmp askpass-expect askpass-query\n+}\n \n test_expect_success 'cloning password-protected repository can fail' '\n \t>askpass-query &&\n \techo wrong >askpass-response &&\n \ttest_must_fail git clone \"$HTTPD_URL/auth/repo.git\" clone-auth-fail &&\n-\ttest_cmp askpass-expect-both askpass-query\n+\texpect_askpass both wrong\n '\n \n test_expect_success 'http auth can use user/pass in URL' '\n \t>askpass-query &&\n \techo wrong >askpass-response &&\n \tgit clone \"$HTTPD_URL_USER_PASS/auth/repo.git\" clone-auth-none &&\n-\ttest_cmp askpass-expect-none askpass-query\n+\texpect_askpass none\n '\n \n test_expect_success 'http auth can use just user in URL' '\n \t>askpass-query &&\n \techo user@host >askpass-response &&\n \tgit clone \"$HTTPD_URL_USER/auth/repo.git\" clone-auth-pass &&\n-\ttest_cmp askpass-expect-pass askpass-query\n+\texpect_askpass pass user@host\n '\n \n test_expect_success 'http auth can request both user and pass' '\n \t>askpass-query &&\n \techo user@host >askpass-response &&\n \tgit clone \"$HTTPD_URL/auth/repo.git\" clone-auth-both &&\n-\ttest_cmp askpass-expect-both askpass-query\n+\texpect_askpass both user@host\n '\n \n test_expect_success 'fetch changes via http' '\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179938","messageId":"20111124110223.GD8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 06/13] credential: apply helper config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:02:23Z","receivedAt":"2011-11-24T11:02:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The functionality for credential storage helpers is already\nthere; we just need to give the users a way to turn it on.\nThis patch provides a \"credential.helper\" configuration\nvariable which allows the user to provide one or more helper\nstrings.\n\nRather than simply matching credential.helper, we will also\ncompare URLs in subsection headings to the current context.\nThis means you can apply configuration to a subset of\ncredentials. For example:\n\n [credential \"https://example.com\"]\n\thelper = foo\n\nwould match a request for \"https://example.com/foo.git\", but\nnot one for \"https://kernel.org/foo.git\".\n\nThis is overkill for the \"helper\" variable, since users are\nunlikely to want different helpers for different sites (and\nsince helpers run arbitrary code, they could do the matching\nthemselves anyway).\n\nHowever, future patches will add new config variables where\nthis extra feature will be more useful.\n\nSigned-off-by: Jeff King \n---\n credential.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++\n credential.h | 5 +++-\n t/t0300-credentials.sh | 42 +++++++++++++++++++++++++++++++++\n t/t5550-http-fetch.sh | 12 +++++++++\n 4 files changed, 119 insertions(+), 1 deletions(-)\n\ndiff --git a/credential.c b/credential.c\nindex 5cc4319..120ea62 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -22,6 +22,61 @@ void credential_clear(struct credential *c)\n \tcredential_init(c);\n }\n \n+int credential_match(const struct credential *want,\n+\t\t const struct credential *have)\n+{\n+#define CHECK(x) (!want->x || (have->x && !strcmp(want->x, have->x)))\n+\treturn CHECK(protocol) &&\n+\t CHECK(host) &&\n+\t CHECK(path) &&\n+\t CHECK(username);\n+#undef CHECK\n+}\n+\n+static int credential_config_callback(const char *var, const char *value,\n+\t\t\t\t void *data)\n+{\n+\tstruct credential *c = data;\n+\tconst char *key, *dot;\n+\n+\tkey = skip_prefix(var, \"credential.\");\n+\tif (!key)\n+\t\treturn 0;\n+\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\n+\tdot = strrchr(key, '.');\n+\tif (dot) {\n+\t\tstruct credential want = CREDENTIAL_INIT;\n+\t\tchar *url = xmemdupz(key, dot - key);\n+\t\tint matched;\n+\n+\t\tcredential_from_url(&want, url);\n+\t\tmatched = credential_match(&want, c);\n+\n+\t\tcredential_clear(&want);\n+\t\tfree(url);\n+\n+\t\tif (!matched)\n+\t\t\treturn 0;\n+\t\tkey = dot + 1;\n+\t}\n+\n+\tif (!strcmp(key, \"helper\"))\n+\t\tstring_list_append(&c->helpers, value);\n+\n+\treturn 0;\n+}\n+\n+static void credential_apply_config(struct credential *c)\n+{\n+\tif (c->configured)\n+\t\treturn;\n+\tgit_config(credential_config_callback, c);\n+\tc->configured = 1;\n+}\n+\n static void credential_describe(struct credential *c, struct strbuf *out)\n {\n \tif (!c->protocol)\n@@ -203,6 +258,8 @@ void credential_fill(struct credential *c)\n \tif (c->username && c->password)\n \t\treturn;\n \n+\tcredential_apply_config(c);\n+\n \tfor (i = 0; i < c->helpers.nr; i++) {\n \t\tcredential_do(c, c->helpers.items[i].string, \"get\");\n \t\tif (c->username && c->password)\n@@ -223,6 +280,8 @@ void credential_approve(struct credential *c)\n \tif (!c->username || !c->password)\n \t\treturn;\n \n+\tcredential_apply_config(c);\n+\n \tfor (i = 0; i < c->helpers.nr; i++)\n \t\tcredential_do(c, c->helpers.items[i].string, \"store\");\n \tc->approved = 1;\n@@ -232,6 +291,8 @@ void credential_reject(struct credential *c)\n {\n \tint i;\n \n+\tcredential_apply_config(c);\n+\n \tfor (i = 0; i < c->helpers.nr; i++)\n \t\tcredential_do(c, c->helpers.items[i].string, \"erase\");\n \ndiff --git a/credential.h b/credential.h\nindex 8a6d162..e504272 100644\n--- a/credential.h\n+++ b/credential.h\n@@ -5,7 +5,8 @@\n \n struct credential {\n \tstruct string_list helpers;\n-\tunsigned approved:1;\n+\tunsigned approved:1,\n+\t\t configured:1;\n \n \tchar *username;\n \tchar *password;\n@@ -25,5 +26,7 @@ struct credential {\n \n int credential_read(struct credential *, FILE *);\n void credential_from_url(struct credential *, const char *url);\n+int credential_match(const struct credential *have,\n+\t\t const struct credential *want);\n \n #endif /* CREDENTIAL_H */\ndiff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh\nindex 81a455f..e3f61f4 100755\n--- a/t/t0300-credentials.sh\n+++ b/t/t0300-credentials.sh\n@@ -192,4 +192,46 @@ test_expect_success 'internal getpass does not ask for known username' '\n \tEOF\n '\n \n+HELPER=\"f() {\n+\t\tcat >/dev/null\n+\t\techo username=foo\n+\t\techo password=bar\n+\t}; f\"\n+test_expect_success 'respect configured credentials' '\n+\ttest_config credential.helper \"$HELPER\" &&\n+\tcheck fill <<-\\EOF\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tEOF\n+'\n+\n+test_expect_success 'match configured credential' '\n+\ttest_config credential.https://example.com.helper \"$HELPER\" &&\n+\tcheck fill <<-\\EOF\n+\tprotocol=https\n+\thost=example.com\n+\tpath=repo.git\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tEOF\n+'\n+\n+test_expect_success 'do not match configured credential' '\n+\ttest_config credential.https://foo.helper \"$HELPER\" &&\n+\tcheck fill <<-\\EOF\n+\tprotocol=https\n+\thost=bar\n+\t--\n+\tusername=askpass-username\n+\tpassword=askpass-password\n+\t--\n+\taskpass: Username for '\\''https://bar'\\'':\n+\taskpass: Password for '\\''https://askpass-username@bar'\\'':\n+\tEOF\n+'\n+\n test_done\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex 398a2d2..21e2f5d 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -101,6 +101,18 @@ test_expect_success 'http auth can request both user and pass' '\n \texpect_askpass both user@host\n '\n \n+test_expect_success 'http auth respects credential helper config' '\n+\ttest_config_global credential.helper \"f() {\n+\t\tcat >/dev/null\n+\t\techo username=user@host\n+\t\techo password=user@host\n+\t}; f\" &&\n+\t>askpass-query &&\n+\techo wrong >askpass-response &&\n+\tgit clone \"$HTTPD_URL/auth/repo.git\" clone-auth-helper &&\n+\texpect_askpass none\n+'\n+\n test_expect_success 'fetch changes via http' '\n \techo content >>file &&\n \tgit commit -a -m two &&\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179939","messageId":"20111124110247.GE8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 07/13] credential: add credential.*.username","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:02:47Z","receivedAt":"2011-11-24T11:02:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Credential helpers can help users avoid having to type their\nusername and password over and over. However, some users may\nnot want a helper for their password, or they may be running\na helper which caches for a short time. In this case, it is\nconvenient to provide the non-secret username portion of\ntheir credential via config.\n\nSigned-off-by: Jeff King \n---\n credential.c | 4 ++++\n t/t0300-credentials.sh | 13 +++++++++++++\n t/t5550-http-fetch.sh | 16 ++++++++++++++++\n 3 files changed, 33 insertions(+), 0 deletions(-)\n\ndiff --git a/credential.c b/credential.c\nindex 120ea62..06e4e15 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -65,6 +65,10 @@ static int credential_config_callback(const char *var, const char *value,\n \n \tif (!strcmp(key, \"helper\"))\n \t\tstring_list_append(&c->helpers, value);\n+\telse if (!strcmp(key, \"username\")) {\n+\t\tif (!c->username)\n+\t\t\tc->username = xstrdup(value);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh\nindex e3f61f4..57751a1 100755\n--- a/t/t0300-credentials.sh\n+++ b/t/t0300-credentials.sh\n@@ -234,4 +234,17 @@ test_expect_success 'do not match configured credential' '\n \tEOF\n '\n \n+test_expect_success 'pull username from config' '\n+\ttest_config credential.https://example.com.username foo &&\n+\tcheck fill <<-\\EOF\n+\tprotocol=https\n+\thost=example.com\n+\t--\n+\tusername=foo\n+\tpassword=askpass-password\n+\t--\n+\taskpass: Password for '\\''https://foo@example.com'\\'':\n+\tEOF\n+'\n+\n test_done\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex 21e2f5d..18f4d59 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -113,6 +113,22 @@ test_expect_success 'http auth respects credential helper config' '\n \texpect_askpass none\n '\n \n+test_expect_success 'http auth can get username from config' '\n+\ttest_config_global \"credential.$HTTPD_URL.username\" user@host &&\n+\t>askpass-query &&\n+\techo user@host >askpass-response &&\n+\tgit clone \"$HTTPD_URL/auth/repo.git\" clone-auth-user &&\n+\texpect_askpass pass user@host\n+'\n+\n+test_expect_success 'configured username does not override URL' '\n+\ttest_config_global \"credential.$HTTPD_URL.username\" wrong &&\n+\t>askpass-query &&\n+\techo user@host >askpass-response &&\n+\tgit clone \"$HTTPD_URL_USER/auth/repo.git\" clone-auth-user2 &&\n+\texpect_askpass pass user@host\n+'\n+\n test_expect_success 'fetch changes via http' '\n \techo content >>file &&\n \tgit commit -a -m two &&\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179940","messageId":"20111124110356.GF8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 08/13] credential: make relevance of http path configurable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:03:56Z","receivedAt":"2011-11-24T11:03:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When parsing a URL into a credential struct, we carefully\nrecord each part of the URL, including the path on the\nremote host, and use the result as part of the credential\ncontext.\n\nThis had two practical implications:\n\n 1. Credential helpers which store a credential for later\n access are likely to use the \"path\" portion as part of\n the storage key. That means that a request to\n\n https://example.com/foo.git\n\n would not use the same credential that was stored in an\n earlier request for:\n\n https://example.com/bar.git\n\n 2. The prompt shown to the user includes all relevant\n context, including the path.\n\nIn most cases, however, users will have a single password\nper host. The behavior in (1) will be inconvenient, and the\nprompt in (2) will be overly long.\n\nThis patch introduces a config option to toggle the\nrelevance of http paths. When turned on, we use the path as\nbefore. When turned off, we drop the path component from the\ncontext: helpers don't see it, and it does not appear in the\nprompt.\n\nThis is nothing you couldn't do with a clever credential\nhelper at the start of your stack, like:\n\n [credential \"http://\"]\n\thelper = \"f() { grep -v ^path= ; }; f\"\n\thelper = your_real_helper\n\nBut doing this:\n\n [credential]\n\tuseHttpPath = false\n\nis way easier and more readable. Furthermore, since most\nusers will want the \"off\" behavior, that is the new default.\nUsers who want it \"on\" can set the variable (either for all\ncredentials, or just for a subset using\ncredential.*.useHttpPath).\n\nSigned-off-by: Jeff King \n---\n credential.c | 14 ++++++++++++++\n credential.h | 3 ++-\n t/t0300-credentials.sh | 29 +++++++++++++++++++++++++++++\n t/t5550-http-fetch.sh | 2 +-\n 4 files changed, 46 insertions(+), 2 deletions(-)\n\ndiff --git a/credential.c b/credential.c\nindex 06e4e15..36ff0ec 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -69,16 +69,30 @@ static int credential_config_callback(const char *var, const char *value,\n \t\tif (!c->username)\n \t\t\tc->username = xstrdup(value);\n \t}\n+\telse if (!strcmp(key, \"usehttppath\"))\n+\t\tc->use_http_path = git_config_bool(var, value);\n \n \treturn 0;\n }\n \n+static int proto_is_http(const char *s)\n+{\n+\tif (!s)\n+\t\treturn 0;\n+\treturn !strcmp(s, \"https\") || !strcmp(s, \"http\");\n+}\n+\n static void credential_apply_config(struct credential *c)\n {\n \tif (c->configured)\n \t\treturn;\n \tgit_config(credential_config_callback, c);\n \tc->configured = 1;\n+\n+\tif (!c->use_http_path && proto_is_http(c->protocol)) {\n+\t\tfree(c->path);\n+\t\tc->path = NULL;\n+\t}\n }\n \n static void credential_describe(struct credential *c, struct strbuf *out)\ndiff --git a/credential.h b/credential.h\nindex e504272..96ea41b 100644\n--- a/credential.h\n+++ b/credential.h\n@@ -6,7 +6,8 @@\n struct credential {\n \tstruct string_list helpers;\n \tunsigned approved:1,\n-\t\t configured:1;\n+\t\t configured:1,\n+\t\t use_http_path:1;\n \n \tchar *username;\n \tchar *password;\ndiff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh\nindex 57751a1..89a8531 100755\n--- a/t/t0300-credentials.sh\n+++ b/t/t0300-credentials.sh\n@@ -247,4 +247,33 @@ test_expect_success 'pull username from config' '\n \tEOF\n '\n \n+test_expect_success 'http paths can be part of context' '\n+\tcheck fill \"verbatim foo bar\" <<-\\EOF &&\n+\tprotocol=https\n+\thost=example.com\n+\tpath=foo.git\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tverbatim: get\n+\tverbatim: protocol=https\n+\tverbatim: host=example.com\n+\tEOF\n+\ttest_config credential.https://example.com.useHttpPath true &&\n+\tcheck fill \"verbatim foo bar\" <<-\\EOF\n+\tprotocol=https\n+\thost=example.com\n+\tpath=foo.git\n+\t--\n+\tusername=foo\n+\tpassword=bar\n+\t--\n+\tverbatim: get\n+\tverbatim: protocol=https\n+\tverbatim: host=example.com\n+\tverbatim: path=foo.git\n+\tEOF\n+'\n+\n test_done\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex 18f4d59..dd073d1 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -53,7 +53,7 @@ test_expect_success 'setup askpass helpers' '\n '\n \n expect_askpass() {\n-\tdest=$HTTPD_DEST/auth/repo.git\n+\tdest=$HTTPD_DEST\n \t{\n \t\tcase \"$1\" in\n \t\tnone)\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179941","messageId":"20111124110505.GG8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 09/13] docs: end-user documentation for the credential subsystem","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:05:05Z","receivedAt":"2011-11-24T11:05:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The credential API and helper format is already defined in\ntechnical/api-credentials.txt. This presents the end-user\nview.\n\nSigned-off-by: Jeff King \n---\nI'm hopelessly far from being the target audience of this document.\nComments welcome on any spots where I forgot for a moment that the\naudience hasn't been reading the underlying code for the past 8 hours.\n\n Documentation/Makefile | 1 +\n Documentation/config.txt | 23 +++++\n Documentation/gitcredentials.txt | 170 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 194 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/gitcredentials.txt\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 304b31e..116f175 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -7,6 +7,7 @@ MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt githooks.txt \\\n MAN7_TXT=gitcli.txt gittutorial.txt gittutorial-2.txt \\\n \tgitcvs-migration.txt gitcore-tutorial.txt gitglossary.txt \\\n \tgitdiffcore.txt gitnamespaces.txt gitrevisions.txt gitworkflows.txt\n+MAN7_TXT += gitcredentials.txt\n \n MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)\n MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT))\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5a841da..36bcdf2 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -832,6 +832,29 @@ commit.template::\n \t\"{tilde}/\" is expanded to the value of `$HOME` and \"{tilde}user/\" to the\n \tspecified user's home directory.\n \n+credential.helper::\n+\tSpecify an external helper to be called when a username or\n+\tpassword credential is needed; the helper may consult external\n+\tstorage to avoid prompting the user for the credentials. See\n+\tlinkgit:gitcredentials[7] for details.\n+\n+credential.useHttpPath::\n+\tWhen acquiring credentials, consider the \"path\" component of an http\n+\tor https URL to be important. Defaults to false. See\n+\tlinkgit:gitcredentials[7] for more information.\n+\n+credential.username::\n+\tIf no username is set for a network authentication, use this username\n+\tby default. See credential..* below, and\n+\tlinkgit:gitcredentials[7].\n+\n+credential..*::\n+\tAny of the credential.* options above can be applied selectively to\n+\tsome credentials. For example \"credential.https://example.com.username\"\n+\twould set the default username only for https connections to\n+\texample.com. See linkgit:gitcredentials[7] for details on how URLs are\n+\tmatched.\n+\n include::diff-config.txt[]\n \n difftool..path::\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nnew file mode 100644\nindex 0000000..1e54117\n--- /dev/null\n+++ b/Documentation/gitcredentials.txt\n@@ -0,0 +1,170 @@\n+gitcredentials(7)\n+=================\n+\n+NAME\n+----\n+gitcredentials - providing usernames and passwords to git\n+\n+SYNOPSIS\n+--------\n+------------------\n+git config credential.https://example.com.username myusername\n+git config credential.helper \"$helper $options\"\n+------------------\n+\n+DESCRIPTION\n+-----------\n+\n+Git will sometimes need credentials from the user in order to perform\n+operations; for example, it may need to ask for a username and password\n+in order to access a remote repository over HTTP. This manual describes\n+the mechanisms git uses to request these credentials, as well as some\n+features to avoid inputting these credentials repeatedly.\n+\n+REQUESTING CREDENTIALS\n+----------------------\n+\n+Without any credential helpers defined, git will try the following\n+strategies to ask the user for usernames and passwords:\n+\n+1. If the `GIT_ASKPASS` environment variable is set, the program\n+ specified by the variable is invoked. A suitable prompt is provided\n+ to the program on the command line, and the user's input is read\n+ from its standard output.\n+\n+2. Otherwise, if the `core.askpass` configuration variable is set, its\n+ value is used as above.\n+\n+3. Otherwise, if the `SSH_ASKPASS` environment variable is set, its\n+ value is used as above.\n+\n+4. Otherwise, the user is prompted on the terminal.\n+\n+AVOIDING REPETITION\n+-------------------\n+\n+It can be cumbersome to input the same credentials over and over. Git\n+provides two methods to reduce this annoyance:\n+\n+1. Static configuration of usernames for a given authentication context.\n+\n+2. Credential helpers to cache or store passwords, or to interact with\n+ a system password wallet or keychain.\n+\n+The first is simple and appropriate if you do not have secure storage available\n+for a password. It is generally configured by adding this to your config:\n+\n+---------------------------------------\n+[credential \"https://example.com\"]\n+\tusername = me\n+---------------------------------------\n+\n+Credential helpers, on the other hand, are external programs from which git can\n+request both usernames and passwords; they typically interface with secure\n+storage provided by the OS or other programs.\n+\n+To use a helper, you must first select one to use. Git does not yet\n+include any credential helpers, but you may have third-party helpers\n+installed; search for `credential-*` in the output of `git help -a`, and\n+consult the documentation of individual helpers. Once you have selected\n+a helper, you can tell git to use it by putting its name into the\n+credential.helper variable.\n+\n+1. Find a helper.\n++\n+-------------------------------------------\n+$ git help -a | grep credential-\n+credential-foo\n+-------------------------------------------\n+\n+2. Read its description.\n++\n+-------------------------------------------\n+$ git help credential-foo\n+-------------------------------------------\n+\n+3. Tell git to use it.\n++\n+-------------------------------------------\n+$ git config --global credential.helper foo\n+-------------------------------------------\n+\n+If there are multiple instances of the `credential.helper` configuration\n+variable, each helper will be tried in turn, and may provide a username,\n+password, or nothing. Once git has acquired both a username and a\n+password, no more helpers will be tried.\n+\n+\n+CREDENTIAL CONTEXTS\n+-------------------\n+\n+Git considers each credential to have a context defined by a URL. This context\n+is used to look up context-specific configuration, and is passed to any\n+helpers, which may use it as an index into secure storage.\n+\n+For instance, imagine we are accessing `https://example.com/foo.git`. When git\n+looks into a config file to see if a section matches this context, it will\n+consider the two a match if the context is a more-specific subset of the\n+pattern in the config file. For example, if you have this in your config file:\n+\n+--------------------------------------\n+[credential \"https://example.com\"]\n+\tusername = foo\n+--------------------------------------\n+\n+then we will match: both protocols are the same, both hosts are the same, and\n+the \"pattern\" URL does not care about the path component at all. However, this\n+context would not match:\n+\n+--------------------------------------\n+[credential \"https://kernel.org\"]\n+\tusername = foo\n+--------------------------------------\n+\n+because the hostnames differ. Nor would it match `foo.example.com`; git\n+compares hostnames exactly, without considering whether two hosts are part of\n+the same domain. Likewise, a config entry for `http://example.com` would not\n+match: git compares the protocols exactly.\n+\n+\n+CONFIGURATION OPTIONS\n+---------------------\n+\n+Options for a credential context can be configured either in\n+`credential.\\*` (which applies to all credentials), or\n+`credential..\\*`, where matches the context as described\n+above.\n+\n+The following options are available in either location:\n+\n+helper::\n+\n+\tThe name of an external credential helper, and any associated options.\n+\tThe value is executed by the shell. If the first word of the helper\n+\tstring is alphanumeric, then `git-credential-` is prepended (so if you\n+\twant to use the `git-credential-foo` helper, you would typically set\n+\tthis to just `foo`). See the manual for specific helpers for examples.\n+\n+username::\n+\n+\tA default username, if one is not provided in the URL.\n+\n+useHttpPath::\n+\n+\tBy default, git does not consider the \"path\" component of an http URL\n+\tto be worth matching via external helpers. This means that a credential\n+\tstored for `https://example.com/foo.git` will also be used for\n+\t`https://example.com/bar.git`. If you do want to distinguish these\n+\tcases, set this option to `true`.\n+\n+\n+CUSTOM HELPERS\n+--------------\n+\n+You can write your own custom helpers to interface with any system in\n+which you keep credentials. See the documentation for git's\n+link:technical/api-credentials.html[credentials API] for details.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179942","messageId":"20111124110710.GH8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 10/13] credentials: add \"cache\" helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:07:10Z","receivedAt":"2011-11-24T11:07:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you access repositories over smart-http using http\nauthentication, then it can be annoying to have git ask you\nfor your password repeatedly. We cache credentials in\nmemory, of course, but git is composed of many small\nprograms. Having to input your password for each one can be\nfrustrating.\n\nThis patch introduces a credential helper that will cache\npasswords in memory for a short period of time.\n\nSigned-off-by: Jeff King \n---\nThis required some rewriting to handle the new helper protocol. I also\nincorporated suggestions from Ramsay Jones to handle cleanup of the\nsocket better.\n\nI suspect this will all have to get turned off for msysgit, unless\nsomebody feels like writing a unix-sockets abstraction layer.\n\n .gitignore | 2 +\n Documentation/git-credential-cache--daemon.txt | 26 +++\n Documentation/git-credential-cache.txt | 77 +++++++\n Documentation/gitcredentials.txt | 17 +-\n Makefile | 3 +\n credential-cache--daemon.c | 269 ++++++++++++++++++++++++\n credential-cache.c | 120 +++++++++++\n git-compat-util.h | 1 +\n t/lib-credential.sh | 219 +++++++++++++++++++\n t/t0301-credential-cache.sh | 18 ++\n unix-socket.c | 56 +++++\n unix-socket.h | 7 +\n 12 files changed, 810 insertions(+), 5 deletions(-)\n create mode 100644 Documentation/git-credential-cache--daemon.txt\n create mode 100644 Documentation/git-credential-cache.txt\n create mode 100644 credential-cache--daemon.c\n create mode 100644 credential-cache.c\n create mode 100755 t/t0301-credential-cache.sh\n create mode 100644 unix-socket.c\n create mode 100644 unix-socket.h\n\ndiff --git a/.gitignore b/.gitignore\nindex 7d2fefc..a6b0bd4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -30,6 +30,8 @@\n /git-commit-tree\n /git-config\n /git-count-objects\n+/git-credential-cache\n+/git-credential-cache--daemon\n /git-cvsexportcommit\n /git-cvsimport\n /git-cvsserver\ndiff --git a/Documentation/git-credential-cache--daemon.txt b/Documentation/git-credential-cache--daemon.txt\nnew file mode 100644\nindex 0000000..11edc5a\n--- /dev/null\n+++ b/Documentation/git-credential-cache--daemon.txt\n@@ -0,0 +1,26 @@\n+git-credential-cache--daemon(1)\n+===============================\n+\n+NAME\n+----\n+git-credential-cache--daemon - temporarily store user credentials in memory\n+\n+SYNOPSIS\n+--------\n+[verse]\n+git credential-cache--daemon \n+\n+DESCRIPTION\n+-----------\n+\n+NOTE: You probably don't want to invoke this command yourself; it is\n+started automatically when you use linkgit:git-credential-cache[1].\n+\n+This command listens on the Unix domain socket specified by ``\n+for `git-credential-cache` clients. Clients may store and retrieve\n+credentials. Each credential is held for a timeout specified by the\n+client; once no credentials are held, the daemon exits.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Documentation/git-credential-cache.txt b/Documentation/git-credential-cache.txt\nnew file mode 100644\nindex 0000000..f3d09c5\n--- /dev/null\n+++ b/Documentation/git-credential-cache.txt\n@@ -0,0 +1,77 @@\n+git-credential-cache(1)\n+=======================\n+\n+NAME\n+----\n+git-credential-cache - helper to temporarily store passwords in memory\n+\n+SYNOPSIS\n+--------\n+-----------------------------\n+git config credential.helper 'cache [options]'\n+-----------------------------\n+\n+DESCRIPTION\n+-----------\n+\n+This command caches credentials in memory for use by future git\n+programs. The stored credentials never touch the disk, and are forgotten\n+after a configurable timeout. The cache is accessible over a Unix\n+domain socket, restricted to the current user by filesystem permissions.\n+\n+You probably don't want to invoke this command directly; it is meant to\n+be used as a credential helper by other parts of git. See\n+linkgit:gitcredentials[7] or `EXAMPLES` below.\n+\n+OPTIONS\n+-------\n+\n+--timeout ::\n+\n+\tNumber of seconds to cache credentials (default: 900).\n+\n+--socket ::\n+\n+\tUse `` to contact a running cache daemon (or start a new\n+\tcache daemon if one is not started). Defaults to\n+\t`~/.git-credential-cache/socket`. If your home directory is on a\n+\tnetwork-mounted filesystem, you may need to change this to a\n+\tlocal filesystem.\n+\n+CONTROLLING THE DAEMON\n+----------------------\n+\n+If you would like the daemon to exit early, forgetting all cached\n+credentials before their timeout, you can issue an `exit` action:\n+\n+--------------------------------------\n+git credential-cache exit\n+--------------------------------------\n+\n+EXAMPLES\n+--------\n+\n+The point of this helper is to reduce the number of times you must type\n+your username or password. For example:\n+\n+------------------------------------\n+$ git config credential.helper cache\n+$ git push http://example.com/repo.git\n+Username: \n+Password: \n+\n+[work for 5 more minutes]\n+$ git push http://example.com/repo.git\n+[your credentials are used automatically]\n+------------------------------------\n+\n+You can provide options via the credential.helper configuration\n+variable (this example drops the cache time to 5 minutes):\n+\n+-------------------------------------------------------\n+$ git config credential.helper 'cache --timeout=300'\n+-------------------------------------------------------\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 1e54117..a1592ed 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -63,11 +63,18 @@ Credential helpers, on the other hand, are external programs from which git can\n request both usernames and passwords; they typically interface with secure\n storage provided by the OS or other programs.\n \n-To use a helper, you must first select one to use. Git does not yet\n-include any credential helpers, but you may have third-party helpers\n-installed; search for `credential-*` in the output of `git help -a`, and\n-consult the documentation of individual helpers. Once you have selected\n-a helper, you can tell git to use it by putting its name into the\n+To use a helper, you must first select one to use. Git currently\n+includes the following helpers:\n+\n+cache::\n+\n+\tCache credentials in memory for a short period of time. See\n+\tlinkgit:git-credential-cache[1] for details.\n+\n+You may may also have third-party helpers installed; search for\n+`credential-*` in the output of `git help -a`, and consult the\n+documentation of individual helpers. Once you have selected a helper,\n+you can tell git to use it by putting its name into the\n credential.helper variable.\n \n 1. Find a helper.\ndiff --git a/Makefile b/Makefile\nindex 5ca363b..5d41c29 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -427,6 +427,8 @@ PROGRAM_OBJS += show-index.o\n PROGRAM_OBJS += upload-pack.o\n PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n+PROGRAM_OBJS += credential-cache.o\n+PROGRAM_OBJS += credential-cache--daemon.o\n \n PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \n@@ -699,6 +701,7 @@ LIB_OBJS += transport-helper.o\n LIB_OBJS += tree-diff.o\n LIB_OBJS += tree.o\n LIB_OBJS += tree-walk.o\n+LIB_OBJS += unix-socket.o\n LIB_OBJS += unpack-trees.o\n LIB_OBJS += url.o\n LIB_OBJS += usage.o\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nnew file mode 100644\nindex 0000000..390f194\n--- /dev/null\n+++ b/credential-cache--daemon.c\n@@ -0,0 +1,269 @@\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"unix-socket.h\"\n+#include \"sigchain.h\"\n+\n+static const char *socket_path;\n+\n+static void cleanup_socket(void)\n+{\n+\tif (socket_path)\n+\t\tunlink(socket_path);\n+}\n+\n+static void cleanup_socket_on_signal(int sig)\n+{\n+\tcleanup_socket();\n+\tsigchain_pop(sig);\n+\traise(sig);\n+}\n+\n+struct credential_cache_entry {\n+\tstruct credential item;\n+\tunsigned long expiration;\n+};\n+static struct credential_cache_entry *entries;\n+static int entries_nr;\n+static int entries_alloc;\n+\n+static void cache_credential(struct credential *c, int timeout)\n+{\n+\tstruct credential_cache_entry *e;\n+\n+\tALLOC_GROW(entries, entries_nr + 1, entries_alloc);\n+\te = &entries[entries_nr++];\n+\n+\t/* take ownership of pointers */\n+\tmemcpy(&e->item, c, sizeof(*c));\n+\tmemset(c, 0, sizeof(*c));\n+\te->expiration = time(NULL) + timeout;\n+}\n+\n+static struct credential_cache_entry *lookup_credential(const struct credential *c)\n+{\n+\tint i;\n+\tfor (i = 0; i < entries_nr; i++) {\n+\t\tstruct credential *e = &entries[i].item;\n+\t\tif (credential_match(c, e))\n+\t\t\treturn &entries[i];\n+\t}\n+\treturn NULL;\n+}\n+\n+static void remove_credential(const struct credential *c)\n+{\n+\tstruct credential_cache_entry *e;\n+\n+\te = lookup_credential(c);\n+\tif (e)\n+\t\te->expiration = 0;\n+}\n+\n+static int check_expirations(void)\n+{\n+\tstatic unsigned long wait_for_entry_until;\n+\tint i = 0;\n+\tunsigned long now = time(NULL);\n+\tunsigned long next = (unsigned long)-1;\n+\n+\t/*\n+\t * Initially give the client 30 seconds to actually contact us\n+\t * and store a credential before we decide there's no point in\n+\t * keeping the daemon around.\n+\t */\n+\tif (!wait_for_entry_until)\n+\t\twait_for_entry_until = now + 30;\n+\n+\twhile (i < entries_nr) {\n+\t\tif (entries[i].expiration <= now) {\n+\t\t\tentries_nr--;\n+\t\t\tcredential_clear(&entries[i].item);\n+\t\t\tif (i != entries_nr)\n+\t\t\t\tmemcpy(&entries[i], &entries[entries_nr], sizeof(*entries));\n+\t\t\t/*\n+\t\t\t * Stick around 30 seconds in case a new credential\n+\t\t\t * shows up (e.g., because we just removed a failed\n+\t\t\t * one, and we will soon get the correct one).\n+\t\t\t */\n+\t\t\twait_for_entry_until = now + 30;\n+\t\t}\n+\t\telse {\n+\t\t\tif (entries[i].expiration < next)\n+\t\t\t\tnext = entries[i].expiration;\n+\t\t\ti++;\n+\t\t}\n+\t}\n+\n+\tif (!entries_nr) {\n+\t\tif (wait_for_entry_until <= now)\n+\t\t\treturn 0;\n+\t\tnext = wait_for_entry_until;\n+\t}\n+\n+\treturn next - now;\n+}\n+\n+static int read_request(FILE *fh, struct credential *c,\n+\t\t\tstruct strbuf *action, int *timeout) {\n+\tstatic struct strbuf item = STRBUF_INIT;\n+\tconst char *p;\n+\n+\tstrbuf_getline(&item, fh, '\\n');\n+\tp = skip_prefix(item.buf, \"action=\");\n+\tif (!p)\n+\t\treturn error(\"client sent bogus action line: %s\", item.buf);\n+\tstrbuf_addstr(action, p);\n+\n+\tstrbuf_getline(&item, fh, '\\n');\n+\tp = skip_prefix(item.buf, \"timeout=\");\n+\tif (!p)\n+\t\treturn error(\"client sent bogus timeout line: %s\", item.buf);\n+\t*timeout = atoi(p);\n+\n+\tif (credential_read(c, fh) < 0)\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static void serve_one_client(FILE *in, FILE *out)\n+{\n+\tstruct credential c = CREDENTIAL_INIT;\n+\tstruct strbuf action = STRBUF_INIT;\n+\tint timeout = -1;\n+\n+\tif (read_request(in, &c, &action, &timeout) < 0)\n+\t\t/* ignore error */ ;\n+\telse if (!strcmp(action.buf, \"get\")) {\n+\t\tstruct credential_cache_entry *e = lookup_credential(&c);\n+\t\tif (e) {\n+\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n+\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n+\t\t}\n+\t}\n+\telse if (!strcmp(action.buf, \"exit\"))\n+\t\texit(0);\n+\telse if (!strcmp(action.buf, \"erase\"))\n+\t\tremove_credential(&c);\n+\telse if (!strcmp(action.buf, \"store\")) {\n+\t\tif (timeout < 0)\n+\t\t\twarning(\"cache client didn't specify a timeout\");\n+\t\telse if (!c.username || !c.password)\n+\t\t\twarning(\"cache client gave us a partial credential\");\n+\t\telse {\n+\t\t\tremove_credential(&c);\n+\t\t\tcache_credential(&c, timeout);\n+\t\t}\n+\t}\n+\telse\n+\t\twarning(\"cache client sent unknown action: %s\", action.buf);\n+\n+\tcredential_clear(&c);\n+\tstrbuf_release(&action);\n+}\n+\n+static int serve_cache_loop(int fd)\n+{\n+\tstruct pollfd pfd;\n+\tunsigned long wakeup;\n+\n+\twakeup = check_expirations();\n+\tif (!wakeup)\n+\t\treturn 0;\n+\n+\tpfd.fd = fd;\n+\tpfd.events = POLLIN;\n+\tif (poll(&pfd, 1, 1000 * wakeup) < 0) {\n+\t\tif (errno != EINTR)\n+\t\t\tdie_errno(\"poll failed\");\n+\t\treturn 1;\n+\t}\n+\n+\tif (pfd.revents & POLLIN) {\n+\t\tint client, client2;\n+\t\tFILE *in, *out;\n+\n+\t\tclient = accept(fd, NULL, NULL);\n+\t\tif (client < 0) {\n+\t\t\twarning(\"accept failed: %s\", strerror(errno));\n+\t\t\treturn 1;\n+\t\t}\n+\t\tclient2 = dup(client);\n+\t\tif (client2 < 0) {\n+\t\t\twarning(\"dup failed: %s\", strerror(errno));\n+\t\t\tclose(client);\n+\t\t\treturn 1;\n+\t\t}\n+\n+\t\tin = xfdopen(client, \"r\");\n+\t\tout = xfdopen(client2, \"w\");\n+\t\tserve_one_client(in, out);\n+\t\tfclose(in);\n+\t\tfclose(out);\n+\t}\n+\treturn 1;\n+}\n+\n+static void serve_cache(const char *socket_path)\n+{\n+\tint fd;\n+\n+\tfd = unix_stream_listen(socket_path);\n+\tif (fd < 0)\n+\t\tdie_errno(\"unable to bind to '%s'\", socket_path);\n+\n+\tprintf(\"ok\\n\");\n+\tfclose(stdout);\n+\n+\twhile (serve_cache_loop(fd))\n+\t\t; /* nothing */\n+\n+\tclose(fd);\n+\tunlink(socket_path);\n+}\n+\n+static const char permissions_advice[] =\n+\"The permissions on your socket directory are too loose; other\\n\"\n+\"users may be able to read your cached credentials. Consider running:\\n\"\n+\"\\n\"\n+\"\tchmod 0700 %s\";\n+static void check_socket_directory(const char *path)\n+{\n+\tstruct stat st;\n+\tchar *path_copy = xstrdup(path);\n+\tchar *dir = dirname(path_copy);\n+\n+\tif (!stat(dir, &st)) {\n+\t\tif (st.st_mode & 077)\n+\t\t\tdie(permissions_advice, dir);\n+\t\tfree(path_copy);\n+\t\treturn;\n+\t}\n+\n+\t/*\n+\t * We must be sure to create the directory with the correct mode,\n+\t * not just chmod it after the fact; otherwise, there is a race\n+\t * condition in which somebody can chdir to it, sleep, then try to open\n+\t * our protected socket.\n+\t */\n+\tif (safe_create_leading_directories_const(dir) < 0)\n+\t\tdie_errno(\"unable to create directories for '%s'\", dir);\n+\tif (mkdir(dir, 0700) < 0)\n+\t\tdie_errno(\"unable to mkdir '%s'\", dir);\n+\tfree(path_copy);\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tsocket_path = argv[1];\n+\n+\tif (!socket_path)\n+\t\tdie(\"usage: git-credential-cache--daemon \");\n+\tcheck_socket_directory(socket_path);\n+\n+\tatexit(cleanup_socket);\n+\tsigchain_push_common(cleanup_socket_on_signal);\n+\n+\tserve_cache(socket_path);\n+\n+\treturn 0;\n+}\ndiff --git a/credential-cache.c b/credential-cache.c\nnew file mode 100644\nindex 0000000..482c4b1\n--- /dev/null\n+++ b/credential-cache.c\n@@ -0,0 +1,120 @@\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"string-list.h\"\n+#include \"parse-options.h\"\n+#include \"unix-socket.h\"\n+#include \"run-command.h\"\n+\n+#define FLAG_SPAWN 0x1\n+#define FLAG_RELAY 0x2\n+\n+static int send_request(const char *socket, const struct strbuf *out)\n+{\n+\tint got_data = 0;\n+\tint fd = unix_stream_connect(socket);\n+\n+\tif (fd < 0)\n+\t\treturn -1;\n+\n+\tif (write_in_full(fd, out->buf, out->len) < 0)\n+\t\tdie_errno(\"unable to write to cache daemon\");\n+\tshutdown(fd, SHUT_WR);\n+\n+\twhile (1) {\n+\t\tchar in[1024];\n+\t\tint r;\n+\n+\t\tr = read_in_full(fd, in, sizeof(in));\n+\t\tif (r == 0)\n+\t\t\tbreak;\n+\t\tif (r < 0)\n+\t\t\tdie_errno(\"read error from cache daemon\");\n+\t\twrite_or_die(1, in, r);\n+\t\tgot_data = 1;\n+\t}\n+\treturn got_data;\n+}\n+\n+static void spawn_daemon(const char *socket)\n+{\n+\tstruct child_process daemon;\n+\tconst char *argv[] = { NULL, NULL, NULL };\n+\tchar buf[128];\n+\tint r;\n+\n+\tmemset(&daemon, 0, sizeof(daemon));\n+\targv[0] = \"git-credential-cache--daemon\";\n+\targv[1] = socket;\n+\tdaemon.argv = argv;\n+\tdaemon.no_stdin = 1;\n+\tdaemon.out = -1;\n+\n+\tif (start_command(&daemon))\n+\t\tdie_errno(\"unable to start cache daemon\");\n+\tr = read_in_full(daemon.out, buf, sizeof(buf));\n+\tif (r < 0)\n+\t\tdie_errno(\"unable to read result code from cache daemon\");\n+\tif (r != 3 || memcmp(buf, \"ok\\n\", 3))\n+\t\tdie(\"cache daemon did not start: %.*s\", r, buf);\n+\tclose(daemon.out);\n+}\n+\n+static void do_cache(const char *socket, const char *action, int timeout,\n+\t\t int flags)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tstrbuf_addf(&buf, \"action=%s\\n\", action);\n+\tstrbuf_addf(&buf, \"timeout=%d\\n\", timeout);\n+\tif (flags & FLAG_RELAY) {\n+\t\tif (strbuf_read(&buf, 0, 0) < 0)\n+\t\t\tdie_errno(\"unable to relay credential\");\n+\t}\n+\n+\tif (!send_request(socket, &buf))\n+\t\treturn;\n+\tif (flags & FLAG_SPAWN) {\n+\t\tspawn_daemon(socket);\n+\t\tsend_request(socket, &buf);\n+\t}\n+\tstrbuf_release(&buf);\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tchar *socket_path = NULL;\n+\tint timeout = 900;\n+\tconst char *op;\n+\tconst char * const usage[] = {\n+\t\t\"git credential-cache [options] \",\n+\t\tNULL\n+\t};\n+\tstruct option options[] = {\n+\t\tOPT_INTEGER(0, \"timeout\", &timeout,\n+\t\t\t \"number of seconds to cache credentials\"),\n+\t\tOPT_STRING(0, \"socket\", &socket_path, \"path\",\n+\t\t\t \"path of cache-daemon socket\"),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, NULL, options, usage, 0);\n+\tif (!argc)\n+\t\tusage_with_options(usage, options);\n+\top = argv[0];\n+\n+\tif (!socket_path)\n+\t\tsocket_path = expand_user_path(\"~/.git-credential-cache/socket\");\n+\tif (!socket_path)\n+\t\tdie(\"unable to find a suitable socket path; use --socket\");\n+\n+\tif (!strcmp(op, \"exit\"))\n+\t\tdo_cache(socket_path, op, timeout, 0);\n+\telse if (!strcmp(op, \"get\") || !strcmp(op, \"erase\"))\n+\t\tdo_cache(socket_path, op, timeout, FLAG_RELAY);\n+\telse if (!strcmp(op, \"store\"))\n+\t\tdo_cache(socket_path, op, timeout, FLAG_RELAY|FLAG_SPAWN);\n+\telse\n+\t\tdie(\"unknown operation: %s\", op);\n+\n+\treturn 0;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 8b4dd5c..5c238bd 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -130,6 +130,7 @@\n #include \n #include \n #include \n+#include \n #ifndef NO_INTTYPES_H\n #include \n #else\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 54ae1f4..ac54b38 100755\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -21,6 +21,225 @@ read_chunk() {\n \tdone\n }\n \n+# Clear any residual data from previous tests. We only\n+# need this when testing third-party helpers which read and\n+# write outside of our trash-directory sandbox.\n+#\n+# Don't bother checking for success here, as it is\n+# outside the scope of tests and represents a best effort to\n+# clean up after ourselves.\n+helper_test_clean() {\n+\treject $1 https example.com store-user\n+\treject $1 https example.com user1\n+\treject $1 https example.com user2\n+\treject $1 ftp other.tld user\n+\treject $1 https timeout.tld user\n+}\n+\n+reject() {\n+\t(\n+\t\techo protocol=$2\n+\t\techo host=$3\n+\t\techo username=$4\n+\t) | test-credential reject $1\n+}\n+\n+helper_test() {\n+\tHELPER=$1\n+\n+\ttest_expect_success \"helper ($HELPER) has no existing data\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@example.com'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) stores password\" '\n+\t\tcheck approve $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=store-user\n+\t\tpassword=store-pass\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) can retrieve password\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\t--\n+\t\tusername=store-user\n+\t\tpassword=store-pass\n+\t\t--\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) requires matching protocol\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=http\n+\t\thost=example.com\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''http://example.com'\\'':\n+\t\taskpass: Password for '\\''http://askpass-username@example.com'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) requires matching host\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=other.tld\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://other.tld'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@other.tld'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) requires matching username\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=other\n+\t\t--\n+\t\tusername=other\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Password for '\\''https://other@example.com'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) requires matching path\" '\n+\t\tcheck approve $HELPER <<-\\EOF &&\n+\t\tprotocol=ftp\n+\t\thost=other.tld\n+\t\tpath=foo.git\n+\t\tusername=user\n+\t\tpassword=pass\n+\t\tEOF\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=ftp\n+\t\thost=other.tld\n+\t\tpath=bar.git\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''ftp://other.tld/bar.git'\\'':\n+\t\taskpass: Password for '\\''ftp://askpass-username@other.tld/bar.git'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) can forget host\" '\n+\t\tcheck reject $HELPER <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tEOF\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@example.com'\\'':\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) can store multiple users\" '\n+\t\tcheck approve $HELPER <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user1\n+\t\tpassword=pass1\n+\t\tEOF\n+\t\tcheck approve $HELPER <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user2\n+\t\tpassword=pass2\n+\t\tEOF\n+\t\tcheck fill $HELPER <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user1\n+\t\t--\n+\t\tusername=user1\n+\t\tpassword=pass1\n+\t\tEOF\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user2\n+\t\t--\n+\t\tusername=user2\n+\t\tpassword=pass2\n+\t\tEOF\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) can forget user\" '\n+\t\tcheck reject $HELPER <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user1\n+\t\tEOF\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user1\n+\t\t--\n+\t\tusername=user1\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Password for '\\''https://user1@example.com'\\'':\n+\t'\n+\n+\ttest_expect_success \"helper ($HELPER) remembers other user\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=example.com\n+\t\tusername=user2\n+\t\t--\n+\t\tusername=user2\n+\t\tpassword=pass2\n+\t\tEOF\n+\t'\n+}\n+\n+helper_test_timeout() {\n+\tHELPER=\"$*\"\n+\n+\ttest_expect_success \"helper ($HELPER) times out\" '\n+\t\tcheck approve \"$HELPER\" <<-\\EOF &&\n+\t\tprotocol=https\n+\t\thost=timeout.tld\n+\t\tusername=user\n+\t\tpassword=pass\n+\t\tEOF\n+\t\tsleep 2 &&\n+\t\tcheck fill \"$HELPER\" <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=timeout.tld\n+\t\t--\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://timeout.tld'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@timeout.tld'\\'':\n+\t\tEOF\n+\t'\n+}\n \n cat >askpass <<\\EOF\n #!/bin/sh\ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nnew file mode 100755\nindex 0000000..3a65f99\n--- /dev/null\n+++ b/t/t0301-credential-cache.sh\n@@ -0,0 +1,18 @@\n+#!/bin/sh\n+\n+test_description='credential-cache tests'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-credential.sh\n+\n+# don't leave a stale daemon running\n+trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT\n+\n+helper_test cache\n+helper_test_timeout cache --timeout=1\n+\n+# we can't rely on our \"trap\" above working after test_done,\n+# as test_done will delete the trash directory containing\n+# our socket, leaving us with no way to access the daemon.\n+git credential-cache exit\n+\n+test_done\ndiff --git a/unix-socket.c b/unix-socket.c\nnew file mode 100644\nindex 0000000..84b1509\n--- /dev/null\n+++ b/unix-socket.c\n@@ -0,0 +1,56 @@\n+#include \"cache.h\"\n+#include \"unix-socket.h\"\n+\n+static int unix_stream_socket(void)\n+{\n+\tint fd = socket(AF_UNIX, SOCK_STREAM, 0);\n+\tif (fd < 0)\n+\t\tdie_errno(\"unable to create socket\");\n+\treturn fd;\n+}\n+\n+static void unix_sockaddr_init(struct sockaddr_un *sa, const char *path)\n+{\n+\tint size = strlen(path) + 1;\n+\tif (size > sizeof(sa->sun_path))\n+\t\tdie(\"socket path is too long to fit in sockaddr\");\n+\tmemset(sa, 0, sizeof(*sa));\n+\tsa->sun_family = AF_UNIX;\n+\tmemcpy(sa->sun_path, path, size);\n+}\n+\n+int unix_stream_connect(const char *path)\n+{\n+\tint fd;\n+\tstruct sockaddr_un sa;\n+\n+\tunix_sockaddr_init(&sa, path);\n+\tfd = unix_stream_socket();\n+\tif (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {\n+\t\tclose(fd);\n+\t\treturn -1;\n+\t}\n+\treturn fd;\n+}\n+\n+int unix_stream_listen(const char *path)\n+{\n+\tint fd;\n+\tstruct sockaddr_un sa;\n+\n+\tunix_sockaddr_init(&sa, path);\n+\tfd = unix_stream_socket();\n+\n+\tunlink(path);\n+\tif (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {\n+\t\tclose(fd);\n+\t\treturn -1;\n+\t}\n+\n+\tif (listen(fd, 5) < 0) {\n+\t\tclose(fd);\n+\t\treturn -1;\n+\t}\n+\n+\treturn fd;\n+}\ndiff --git a/unix-socket.h b/unix-socket.h\nnew file mode 100644\nindex 0000000..e271aee\n--- /dev/null\n+++ b/unix-socket.h\n@@ -0,0 +1,7 @@\n+#ifndef UNIX_SOCKET_H\n+#define UNIX_SOCKET_H\n+\n+int unix_stream_connect(const char *path);\n+int unix_stream_listen(const char *path);\n+\n+#endif /* UNIX_SOCKET_H */\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179943","messageId":"20111124110728.GI8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:07:28Z","receivedAt":"2011-11-24T11:07:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This just follows the rfc3986 rules for percent-encoding\nurl data into a strbuf.\n\nSigned-off-by: Jeff King \n---\n strbuf.c | 37 +++++++++++++++++++++++++++++++++++++\n strbuf.h | 5 +++++\n 2 files changed, 42 insertions(+), 0 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 3ad2cc0..60e5e59 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -397,3 +397,40 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n \n \treturn len;\n }\n+\n+static int is_rfc3986_reserved(char ch)\n+{\n+\tswitch (ch) {\n+\t\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n+\t\tcase ':': case '@': case '&': case '=': case '+': case '$':\n+\t\tcase ',': case '/': case '?': case '#': case '[': case ']':\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int is_rfc3986_unreserved(char ch)\n+{\n+\treturn isalnum(ch) ||\n+\t\tch == '-' || ch == '_' || ch == '.' || ch == '~';\n+}\n+\n+void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,\n+\t\t\t int reserved)\n+{\n+\tstrbuf_grow(sb, len);\n+\twhile (len--) {\n+\t\tchar ch = *s++;\n+\t\tif (is_rfc3986_unreserved(ch) ||\n+\t\t (!reserved && is_rfc3986_reserved(ch)))\n+\t\t\tstrbuf_addch(sb, ch);\n+\t\telse\n+\t\t\tstrbuf_addf(sb, \"%%%02x\", ch);\n+\t}\n+}\n+\n+void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,\n+\t\t\t int reserved)\n+{\n+\tstrbuf_add_urlencode(sb, s, strlen(s), reserved);\n+}\ndiff --git a/strbuf.h b/strbuf.h\nindex 46a33f8..cecd48c 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -115,4 +115,9 @@ struct strbuf_expand_dict_entry {\n extern int strbuf_branchname(struct strbuf *sb, const char *name);\n extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n \n+extern void strbuf_add_urlencode(struct strbuf *, const char *, size_t,\n+\t\t\t\t int reserved);\n+extern void strbuf_addstr_urlencode(struct strbuf *, const char *,\n+\t\t\t\t int reserved);\n+\n #endif /* STRBUF_H */\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179944","messageId":"20111124110756.GJ8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 12/13] credentials: add \"store\" helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:07:56Z","receivedAt":"2011-11-24T11:07:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is like \"cache\", except that we actually put the\ncredentials on disk. This can be terribly insecure, of\ncourse, but we do what we can to protect them by filesystem\npermissions, and we warn the user in the documentation.\n\nThis is not unlike using .netrc to store entries, but it's a\nlittle more user-friendly. Instead of putting credentials in\nplace ahead of time, we transparently store them after\nprompting the user for them once.\n\nSigned-off-by: Jeff King \n---\n .gitignore | 1 +\n Documentation/git-credential-store.txt | 75 +++++++++++++++++\n Documentation/gitcredentials.txt | 5 +\n Makefile | 1 +\n credential-store.c | 144 ++++++++++++++++++++++++++++++++\n t/t0302-credential-store.sh | 9 ++\n 6 files changed, 235 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/git-credential-store.txt\n create mode 100644 credential-store.c\n create mode 100755 t/t0302-credential-store.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex a6b0bd4..2b7a3f9 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -32,6 +32,7 @@\n /git-count-objects\n /git-credential-cache\n /git-credential-cache--daemon\n+/git-credential-store\n /git-cvsexportcommit\n /git-cvsimport\n /git-cvsserver\ndiff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt\nnew file mode 100644\nindex 0000000..3dcffbd\n--- /dev/null\n+++ b/Documentation/git-credential-store.txt\n@@ -0,0 +1,75 @@\n+git-credential-store(1)\n+=======================\n+\n+NAME\n+----\n+git-credential-store - helper to store credentials on disk\n+\n+SYNOPSIS\n+--------\n+-------------------\n+git config credential.helper 'store [options]'\n+-------------------\n+\n+DESCRIPTION\n+-----------\n+\n+NOTE: Using this helper will store your passwords unencrypted on disk,\n+protected only by filesystem permissions. If this is not an acceptable\n+security tradeoff, try linkgit:git-credential-cache[1], or find a helper\n+that integrates with secure storage provided by your operating system.\n+\n+This command stores credentials indefinitely on disk for use by future\n+git programs.\n+\n+You probably don't want to invoke this command directly; it is meant to\n+be used as a credential helper by other parts of git. See\n+linkgit:gitcredentials[7] or `EXAMPLES` below.\n+\n+OPTIONS\n+-------\n+\n+--store=::\n+\n+\tUse `` to store credentials. The file will have its\n+\tfilesystem permissions set to prevent other users on the system\n+\tfrom reading it, but will not be encrypted or otherwise\n+\tprotected. Defaults to `~/.git-credentials`.\n+\n+EXAMPLES\n+--------\n+\n+The point of this helper is to reduce the number of times you must type\n+your username or password. For example:\n+\n+------------------------------------------\n+$ git config credential.helper store\n+$ git push http://example.com/repo.git\n+Username: \n+Password: \n+\n+[several days later]\n+$ git push http://example.com/repo.git\n+[your credentials are used automatically]\n+------------------------------------------\n+\n+STORAGE FORMAT\n+--------------\n+\n+The `.git-credentials` file is stored in plaintext. Each credential is\n+stored on its own line as a URL like:\n+\n+------------------------------\n+https://user:pass@example.com\n+------------------------------\n+\n+When git needs authentication for a particular context URL context,\n+credential-store will consider that context a pattern to match against\n+each entry in the credentials file. If the protocol, hostname, and\n+username (if we already have one) match, then the password is returned\n+to git. See the discussion of configuration in linkgit:gitcredentials[7]\n+for more information.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex a1592ed..519d2bb 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -71,6 +71,11 @@ cache::\n \tCache credentials in memory for a short period of time. See\n \tlinkgit:git-credential-cache[1] for details.\n \n+store::\n+\n+\tStore credentials indefinitely on disk. See\n+\tlinkgit:git-credential-store[1] for details.\n+\n You may may also have third-party helpers installed; search for\n `credential-*` in the output of `git help -a`, and consult the\n documentation of individual helpers. Once you have selected a helper,\ndiff --git a/Makefile b/Makefile\nindex 5d41c29..2537128 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -429,6 +429,7 @@ PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += credential-cache.o\n PROGRAM_OBJS += credential-cache--daemon.o\n+PROGRAM_OBJS += credential-store.o\n \n PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \ndiff --git a/credential-store.c b/credential-store.c\nnew file mode 100644\nindex 0000000..a6ddb0c\n--- /dev/null\n+++ b/credential-store.c\n@@ -0,0 +1,144 @@\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"string-list.h\"\n+#include \"parse-options.h\"\n+\n+static struct lock_file credential_lock;\n+\n+static void parse_credential_file(const char *fn,\n+\t\t\t\t struct credential *c,\n+\t\t\t\t void (*match_cb)(struct credential *),\n+\t\t\t\t void (*other_cb)(struct strbuf *))\n+{\n+\tFILE *fh;\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct credential entry = CREDENTIAL_INIT;\n+\n+\tfh = fopen(fn, \"r\");\n+\tif (!fh) {\n+\t\tif (errno != ENOENT)\n+\t\t\tdie_errno(\"unable to open %s\", fn);\n+\t\treturn;\n+\t}\n+\n+\twhile (strbuf_getline(&line, fh, '\\n') != EOF) {\n+\t\tcredential_from_url(&entry, line.buf);\n+\t\tif (entry.username && entry.password &&\n+\t\t credential_match(c, &entry)) {\n+\t\t\tif (match_cb) {\n+\t\t\t\tmatch_cb(&entry);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\telse if (other_cb)\n+\t\t\tother_cb(&line);\n+\t}\n+\n+\tcredential_clear(&entry);\n+\tstrbuf_release(&line);\n+\tfclose(fh);\n+}\n+\n+static void print_entry(struct credential *c)\n+{\n+\tprintf(\"username=%s\\n\", c->username);\n+\tprintf(\"password=%s\\n\", c->password);\n+}\n+\n+static void print_line(struct strbuf *buf)\n+{\n+\tstrbuf_addch(buf, '\\n');\n+\twrite_or_die(credential_lock.fd, buf->buf, buf->len);\n+}\n+\n+static void rewrite_credential_file(const char *fn, struct credential *c,\n+\t\t\t\t struct strbuf *extra)\n+{\n+\tumask(077);\n+\tif (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)\n+\t\tdie_errno(\"unable to get credential storage lock\");\n+\tparse_credential_file(fn, c, NULL, print_line);\n+\tif (extra)\n+\t\tprint_line(extra);\n+\tif (commit_lock_file(&credential_lock) < 0)\n+\t\tdie_errno(\"unable to commit credential store\");\n+}\n+\n+static void store_credential(const char *fn, struct credential *c)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (!c->protocol || !(c->host || c->path) ||\n+\t !c->username || !c->password)\n+\t\treturn;\n+\n+\tstrbuf_addf(&buf, \"%s://\", c->protocol);\n+\tstrbuf_addstr_urlencode(&buf, c->username, 1);\n+\tstrbuf_addch(&buf, ':');\n+\tstrbuf_addstr_urlencode(&buf, c->password, 1);\n+\tstrbuf_addch(&buf, '@');\n+\tif (c->host)\n+\t\tstrbuf_addstr_urlencode(&buf, c->host, 1);\n+\tif (c->path) {\n+\t\tstrbuf_addch(&buf, '/');\n+\t\tstrbuf_addstr_urlencode(&buf, c->path, 0);\n+\t}\n+\n+\trewrite_credential_file(fn, c, &buf);\n+\tstrbuf_release(&buf);\n+}\n+\n+static void remove_credential(const char *fn, struct credential *c)\n+{\n+\tif (!c->protocol || !(c->host || c->path))\n+\t\treturn;\n+\trewrite_credential_file(fn, c, NULL);\n+}\n+\n+static int lookup_credential(const char *fn, struct credential *c)\n+{\n+\tif (!c->protocol || !(c->host || c->path))\n+\t\treturn 0;\n+\tparse_credential_file(fn, c, print_entry, NULL);\n+\treturn c->username && c->password;\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tconst char * const usage[] = {\n+\t\t\"git credential-store [options] \",\n+\t\tNULL\n+\t};\n+\tconst char *op;\n+\tstruct credential c = CREDENTIAL_INIT;\n+\tchar *store = NULL;\n+\tstruct option options[] = {\n+\t\tOPT_STRING_LIST(0, \"store\", &store, \"file\",\n+\t\t\t\t\"fetch and store credentials in \"),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, NULL, options, usage, 0);\n+\tif (argc != 1)\n+\t\tusage_with_options(usage, options);\n+\top = argv[0];\n+\n+\tif (!store)\n+\t\tstore = expand_user_path(\"~/.git-credentials\");\n+\tif (!store)\n+\t\tdie(\"unable to set up default store; use --store\");\n+\n+\tif (credential_read(&c, stdin) < 0)\n+\t\tdie(\"unable to read credential\");\n+\n+\tif (!strcmp(op, \"get\"))\n+\t\tlookup_credential(store, &c);\n+\telse if (!strcmp(op, \"erase\"))\n+\t\tremove_credential(store, &c);\n+\telse if (!strcmp(op, \"store\"))\n+\t\tstore_credential(store, &c);\n+\telse\n+\t\tdie(\"unknown operation: %s\", op);\n+\n+\treturn 0;\n+}\ndiff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh\nnew file mode 100755\nindex 0000000..f61b40c\n--- /dev/null\n+++ b/t/t0302-credential-store.sh\n@@ -0,0 +1,9 @@\n+#!/bin/sh\n+\n+test_description='credential-store tests'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-credential.sh\n+\n+helper_test store\n+\n+test_done\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179945","messageId":"20111124110904.GK8417@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 13/13] t: add test harness for external credential helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:09:04Z","receivedAt":"2011-11-24T11:09:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have tests for the internal helpers, but it's\nnice to give authors of external tools an easy way to\nsanity-check their helpers.\n\nIf you have written the \"git-credential-foo\" helper, you can\ndo so with:\n\n GIT_TEST_CREDENTIAL_HELPER=foo \\\n make t0303-credential-external.sh\n\nThis assumes that your helper is capable of both storing and\nretrieving credentials (some helpers may be read-only, and\nthey will fail these tests).\n\nIf your helper supports time-based expiration with a\nconfigurable timeout, you can test that feature like this:\n\n GIT_TEST_CREDENTIAL_HELPER_TIMEOUT=\"foo --timeout=1\" \\\n make t0303-credential-external.sh\n\nSigned-off-by: Jeff King \n---\nI haven't tried porting or testing any of the older helpers to the new\ninterface. So this script has only been lightly tested with:\n\n GIT_TEST_CREDENTIAL_HELPER=cache\n\n t/t0303-credential-external.sh | 19 +++++++++++++++++++\n 1 files changed, 19 insertions(+), 0 deletions(-)\n create mode 100755 t/t0303-credential-external.sh\n\ndiff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\nnew file mode 100755\nindex 0000000..f3953d4\n--- /dev/null\n+++ b/t/t0303-credential-external.sh\n@@ -0,0 +1,19 @@\n+#!/bin/sh\n+\n+test_description='external credential helper tests'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-credential.sh\n+\n+if test -z \"$GIT_TEST_CREDENTIAL_HELPER\"; then\n+\tsay \"# skipping external helper tests (set GIT_TEST_CREDENTIAL_HELPER)\"\n+else\n+\thelper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n+fi\n+\n+if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n+\tsay \"# skipping external helper timeout tests\"\n+else\n+\thelper_test_timeout \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"\n+fi\n+\n+test_done\n-- \n1.7.7.4.5.gb32a5\n"},{"id":"179946","messageId":"CABPQNSaT+4F=EW_agAh_FY0h_ZRgMCzpukdKuvZTdfmz5_Nueg@mail.gmail.com","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"Re: [PATCH 0/13] credential helpers, take two","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-24T11:45:39Z","receivedAt":"2011-11-24T11:45:39Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Nov 24, 2011 at 11:58 AM, Jeff King wrote:\n> Here's a revised version of the http-auth / credential-helper series.\n>\n\nNice. Do you have the branch somewhere public I can pull it from?\n"},{"id":"179948","messageId":"20111124115351.GA29465@sigill.intra.peff.net","threadId":"29017","inReplyTo":"CABPQNSaT+4F=EW_agAh_FY0h_ZRgMCzpukdKuvZTdfmz5_Nueg@mail.gmail.com","subject":"Re: [PATCH 0/13] credential helpers, take two","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T11:53:51Z","receivedAt":"2011-11-24T11:53:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 24, 2011 at 12:45:39PM +0100, Erik Faye-Lund wrote:\n\n> On Thu, Nov 24, 2011 at 11:58 AM, Jeff King wrote:\n> > Here's a revised version of the http-auth / credential-helper series.\n> >\n> \n> Nice. Do you have the branch somewhere public I can pull it from?\n\n git://github.com/peff/git.git jk/http-auth-simple\n\nBut be aware that I do rebase my topic branches frequently.\n\n-Peff\n"},{"id":"179949","messageId":"CABPQNSbb2xjoWSZ+UC8LtpAi2DZ6=zFtavMrVvAFy6kPPjOCdg@mail.gmail.com","threadId":"29017","inReplyTo":"20111124115351.GA29465@sigill.intra.peff.net","subject":"Re: [PATCH 0/13] credential helpers, take two","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-24T12:08:39Z","receivedAt":"2011-11-24T12:08:39Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Nov 24, 2011 at 12:53 PM, Jeff King wrote:\n> On Thu, Nov 24, 2011 at 12:45:39PM +0100, Erik Faye-Lund wrote:\n>\n>> On Thu, Nov 24, 2011 at 11:58 AM, Jeff King wrote:\n>> > Here's a revised version of the http-auth / credential-helper series.\n>> >\n>>\n>> Nice. Do you have the branch somewhere public I can pull it from?\n>\n>  git://github.com/peff/git.git jk/http-auth-simple\n>\n\nThanks.\n\n> But be aware that I do rebase my topic branches frequently.\n>\n\nThat's fine by me, I just didn't want to have to download each patch\nas a raw e-mail and apply it manually ;)\n"},{"id":"179951","messageId":"CAPig+cQ_eP0wG=AFB0rt-SueW-XxrFXKYAXr+4PneYC-dciyuw@mail.gmail.com","threadId":"29017","inReplyTo":"20111124110756.GJ8417@sigill.intra.peff.net","subject":"Re: [PATCH 12/13] credentials: add \"store\" helper","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2011-11-24T14:29:36Z","receivedAt":"2011-11-24T14:29:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 24, 2011 at 6:07 AM, Jeff King wrote:\n> diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt\n> new file mode 100644\n> index 0000000..3dcffbd\n> --- /dev/null\n> +++ b/Documentation/git-credential-store.txt\n> @@ -0,0 +1,75 @@\n> [snip]\n> +The `.git-credentials` file is stored in plaintext. Each credential is\n> +stored on its own line as a URL like:\n> +\n> +------------------------------\n> +https://user:pass@example.com\n> +------------------------------\n> +\n> +When git needs authentication for a particular context URL context,\n\n\"context URL context\"?\n\n-- ES\n"},{"id":"179952","messageId":"CAPig+cShsteRy4=hEtJdmUk5TDPr1_vjCk5DiLfv-a2NpeVYRA@mail.gmail.com","threadId":"29017","inReplyTo":"20111124110710.GH8417@sigill.intra.peff.net","subject":"Re: [PATCH 10/13] credentials: add \"cache\" helper","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2011-11-24T14:36:43Z","receivedAt":"2011-11-24T14:36:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 24, 2011 at 6:07 AM, Jeff King wrote:\n> diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\n> index 1e54117..a1592ed 100644\n> --- a/Documentation/gitcredentials.txt\n> +++ b/Documentation/gitcredentials.txt\n> @@ -63,11 +63,18 @@ Credential helpers, on the other hand, are external programs from which git can\n>  [snip]\n> +cache::\n> +\n> +       Cache credentials in memory for a short period of time. See\n> +       linkgit:git-credential-cache[1] for details.\n> +\n> +You may may also have third-party helpers installed; search for\n\ns/may may/may/\n\n-- ES\n"},{"id":"179954","messageId":"20111124200926.GA30585@sigill.intra.peff.net","threadId":"29017","inReplyTo":"CAPig+cQ_eP0wG=AFB0rt-SueW-XxrFXKYAXr+4PneYC-dciyuw@mail.gmail.com","subject":"Re: [PATCH 12/13] credentials: add \"store\" helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-24T20:09:26Z","receivedAt":"2011-11-24T20:09:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 24, 2011 at 09:29:36AM -0500, Eric Sunshine wrote:\n\n> > +When git needs authentication for a particular context URL context,\n> \n> \"context URL context\"?\n\nThanks. That is what I get for proofreading right before bed. :)\n\nI squashed this and your other typo fix into the appropriate patches.\n\n-Peff\n"},{"id":"180003","messageId":"20111127082744.GA32068@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111124105801.GA6168@sigill.intra.peff.net","subject":"[PATCH 0/6] echo usernames as they are typed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:27:45Z","receivedAt":"2011-11-27T08:27:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:\n\n> Here's a revised version of the http-auth / credential-helper series.\n\nAnd here's something I've been meaning to do on top: actually echo\ncharacters at the username prompt. We can't do this portably, but we can\nat least stub out a compatibility layer and let each system do something\nsensible.\n\n [1/6]: move git_getpass to its own source file\n [2/6]: refactor git_getpass into generic prompt function\n [3/6]: stub out getpass_echo function\n [4/6]: prompt: add PROMPT_ECHO flag\n [5/6]: credential: use git_prompt instead of git_getpass\n [6/6]: compat/getpass: add a /dev/tty implementation\n\n-Peff\n"},{"id":"180004","messageId":"20111127082816.GA1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 1/6] move git_getpass to its own source file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:28:16Z","receivedAt":"2011-11-27T08:28:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is currently in connect.c, but really has nothing to\ndo with the git protocol itself. Let's make a new source\nfile all about prompting the user, which will make it\ncleaner to refactor.\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 ++\n cache.h | 1 -\n connect.c | 44 --------------------------------------------\n credential.c | 1 +\n imap-send.c | 1 +\n prompt.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n prompt.h | 6 ++++++\n 7 files changed, 58 insertions(+), 45 deletions(-)\n create mode 100644 prompt.c\n create mode 100644 prompt.h\n\ndiff --git a/Makefile b/Makefile\nindex 2537128..14c6480 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -563,6 +563,7 @@ LIB_H += parse-options.h\n LIB_H += patch-ids.h\n LIB_H += pkt-line.h\n LIB_H += progress.h\n+LIB_H += prompt.h\n LIB_H += quote.h\n LIB_H += reflog-walk.h\n LIB_H += refs.h\n@@ -669,6 +670,7 @@ LIB_OBJS += pkt-line.o\n LIB_OBJS += preload-index.o\n LIB_OBJS += pretty.o\n LIB_OBJS += progress.o\n+LIB_OBJS += prompt.o\n LIB_OBJS += quote.o\n LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\ndiff --git a/cache.h b/cache.h\nindex 2e6ad36..f320c98 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1024,7 +1024,6 @@ struct ref {\n extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n \n #define CONNECT_VERBOSE (1u << 0)\n-extern char *git_getpass(const char *prompt);\n extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n extern int finish_connect(struct child_process *conn);\n extern int git_connection_is_socket(struct child_process *conn);\ndiff --git a/connect.c b/connect.c\nindex 51990fa..519e527 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -619,47 +619,3 @@ int finish_connect(struct child_process *conn)\n \tfree(conn);\n \treturn code;\n }\n-\n-char *git_getpass(const char *prompt)\n-{\n-\tconst char *askpass;\n-\tstruct child_process pass;\n-\tconst char *args[3];\n-\tstatic struct strbuf buffer = STRBUF_INIT;\n-\n-\taskpass = getenv(\"GIT_ASKPASS\");\n-\tif (!askpass)\n-\t\taskpass = askpass_program;\n-\tif (!askpass)\n-\t\taskpass = getenv(\"SSH_ASKPASS\");\n-\tif (!askpass || !(*askpass)) {\n-\t\tchar *result = getpass(prompt);\n-\t\tif (!result)\n-\t\t\tdie_errno(\"Could not read password\");\n-\t\treturn result;\n-\t}\n-\n-\targs[0] = askpass;\n-\targs[1]\t= prompt;\n-\targs[2] = NULL;\n-\n-\tmemset(&pass, 0, sizeof(pass));\n-\tpass.argv = args;\n-\tpass.out = -1;\n-\n-\tif (start_command(&pass))\n-\t\texit(1);\n-\n-\tstrbuf_reset(&buffer);\n-\tif (strbuf_read(&buffer, pass.out, 20) < 0)\n-\t\tdie(\"failed to read password from %s\\n\", askpass);\n-\n-\tclose(pass.out);\n-\n-\tif (finish_command(&pass))\n-\t\texit(1);\n-\n-\tstrbuf_setlen(&buffer, strcspn(buffer.buf, \"\\r\\n\"));\n-\n-\treturn buffer.buf;\n-}\ndiff --git a/credential.c b/credential.c\nindex 36ff0ec..d918ba5 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -3,6 +3,7 @@\n #include \"string-list.h\"\n #include \"run-command.h\"\n #include \"url.h\"\n+#include \"prompt.h\"\n \n void credential_init(struct credential *c)\n {\ndiff --git a/imap-send.c b/imap-send.c\nindex e1ad1a4..8fc2e2d 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -25,6 +25,7 @@\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n #include \"run-command.h\"\n+#include \"prompt.h\"\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #else\ndiff --git a/prompt.c b/prompt.c\nnew file mode 100644\nindex 0000000..42a1c9f\n--- /dev/null\n+++ b/prompt.c\n@@ -0,0 +1,48 @@\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n+#include \"prompt.h\"\n+\n+char *git_getpass(const char *prompt)\n+{\n+\tconst char *askpass;\n+\tstruct child_process pass;\n+\tconst char *args[3];\n+\tstatic struct strbuf buffer = STRBUF_INIT;\n+\n+\taskpass = getenv(\"GIT_ASKPASS\");\n+\tif (!askpass)\n+\t\taskpass = askpass_program;\n+\tif (!askpass)\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\tif (!askpass || !(*askpass)) {\n+\t\tchar *result = getpass(prompt);\n+\t\tif (!result)\n+\t\t\tdie_errno(\"Could not read password\");\n+\t\treturn result;\n+\t}\n+\n+\targs[0] = askpass;\n+\targs[1]\t= prompt;\n+\targs[2] = NULL;\n+\n+\tmemset(&pass, 0, sizeof(pass));\n+\tpass.argv = args;\n+\tpass.out = -1;\n+\n+\tif (start_command(&pass))\n+\t\texit(1);\n+\n+\tstrbuf_reset(&buffer);\n+\tif (strbuf_read(&buffer, pass.out, 20) < 0)\n+\t\tdie(\"failed to read password from %s\\n\", askpass);\n+\n+\tclose(pass.out);\n+\n+\tif (finish_command(&pass))\n+\t\texit(1);\n+\n+\tstrbuf_setlen(&buffer, strcspn(buffer.buf, \"\\r\\n\"));\n+\n+\treturn buffer.buf;\n+}\ndiff --git a/prompt.h b/prompt.h\nnew file mode 100644\nindex 0000000..0fd7bd9\n--- /dev/null\n+++ b/prompt.h\n@@ -0,0 +1,6 @@\n+#ifndef PROMPT_H\n+#define PROMPT_H\n+\n+char *git_getpass(const char *prompt);\n+\n+#endif /* PROMPT_H */\n-- \n1.7.7.4.7.g24824\n"},{"id":"180005","messageId":"20111127082932.GB1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 2/6] refactor git_getpass into generic prompt function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:29:32Z","receivedAt":"2011-11-27T08:29:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This will allow callers to specify more options (e.g.,\nleaving echo on). The original git_getpass becomes a slim\nwrapper around the new function.\n\nSigned-off-by: Jeff King \n---\nMaking askpass optional isn't necessary for this series, but splitting\nit actually makes the code a little cleaner, imho, and some callers\nmight eventually want to turn it off.\n\n prompt.c | 41 +++++++++++++++++++++++++----------------\n prompt.h | 3 +++\n 2 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/prompt.c b/prompt.c\nindex 42a1c9f..7c8f9aa 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -3,26 +3,13 @@\n #include \"strbuf.h\"\n #include \"prompt.h\"\n \n-char *git_getpass(const char *prompt)\n+static char *do_askpass(const char *cmd, const char *prompt, const char *name)\n {\n-\tconst char *askpass;\n \tstruct child_process pass;\n \tconst char *args[3];\n \tstatic struct strbuf buffer = STRBUF_INIT;\n \n-\taskpass = getenv(\"GIT_ASKPASS\");\n-\tif (!askpass)\n-\t\taskpass = askpass_program;\n-\tif (!askpass)\n-\t\taskpass = getenv(\"SSH_ASKPASS\");\n-\tif (!askpass || !(*askpass)) {\n-\t\tchar *result = getpass(prompt);\n-\t\tif (!result)\n-\t\t\tdie_errno(\"Could not read password\");\n-\t\treturn result;\n-\t}\n-\n-\targs[0] = askpass;\n+\targs[0] = cmd;\n \targs[1]\t= prompt;\n \targs[2] = NULL;\n \n@@ -35,7 +22,7 @@\n \n \tstrbuf_reset(&buffer);\n \tif (strbuf_read(&buffer, pass.out, 20) < 0)\n-\t\tdie(\"failed to read password from %s\\n\", askpass);\n+\t\tdie(\"failed to read %s from %s\\n\", name, cmd);\n \n \tclose(pass.out);\n \n@@ -46,3 +33,25 @@\n \n \treturn buffer.buf;\n }\n+\n+char *git_prompt(const char *prompt, const char *name, int flags)\n+{\n+\tif (flags & PROMPT_ASKPASS) {\n+\t\tconst char *askpass;\n+\n+\t\taskpass = getenv(\"GIT_ASKPASS\");\n+\t\tif (!askpass)\n+\t\t\taskpass = askpass_program;\n+\t\tif (!askpass)\n+\t\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\t\tif (askpass && *askpass)\n+\t\t\treturn do_askpass(askpass, prompt, name);\n+\t}\n+\n+\treturn getpass(prompt);\n+}\n+\n+char *git_getpass(const char *prompt)\n+{\n+\treturn git_prompt(prompt, \"password\", PROMPT_ASKPASS);\n+}\ndiff --git a/prompt.h b/prompt.h\nindex 0fd7bd9..18868c2 100644\n--- a/prompt.h\n+++ b/prompt.h\n@@ -1,6 +1,9 @@\n #ifndef PROMPT_H\n #define PROMPT_H\n \n+#define PROMPT_ASKPASS (1<<0)\n+\n+char *git_prompt(const char *prompt, const char *name, int flags);\n char *git_getpass(const char *prompt);\n \n #endif /* PROMPT_H */\n-- \n1.7.7.4.7.g24824\n"},{"id":"180006","messageId":"20111127083016.GC1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 3/6] stub out getpass_echo function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:30:16Z","receivedAt":"2011-11-27T08:30:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can't implement getpass_echo portably, but we can at\nleast put in the infrastructure so that builds can provide a\nsystem-specific way of accomplishing this.\n\nRight now we just fall back on calling getpass (which\ndoesn't echo, but is available almost everywhere).\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 ++\n compat/getpass.c | 6 ++++++\n compat/getpass.h | 6 ++++++\n 3 files changed, 14 insertions(+), 0 deletions(-)\n create mode 100644 compat/getpass.c\n create mode 100644 compat/getpass.h\n\ndiff --git a/Makefile b/Makefile\nindex 14c6480..d133e2b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -521,6 +521,7 @@ LIB_H += color.h\n LIB_H += commit.h\n LIB_H += compat/bswap.h\n LIB_H += compat/cygwin.h\n+LIB_H += compat/getpass.h\n LIB_H += compat/mingw.h\n LIB_H += compat/obstack.h\n LIB_H += compat/win32/pthread.h\n@@ -609,6 +610,7 @@ LIB_OBJS += cache-tree.o\n LIB_OBJS += color.o\n LIB_OBJS += combine-diff.o\n LIB_OBJS += commit.o\n+LIB_OBJS += compat/getpass.o\n LIB_OBJS += compat/obstack.o\n LIB_OBJS += config.o\n LIB_OBJS += connect.o\ndiff --git a/compat/getpass.c b/compat/getpass.c\nnew file mode 100644\nindex 0000000..8ae82f0\n--- /dev/null\n+++ b/compat/getpass.c\n@@ -0,0 +1,6 @@\n+#include \"../git-compat-util.h\"\n+\n+char *getpass_echo(const char *prompt)\n+{\n+\treturn getpass(prompt);\n+}\ndiff --git a/compat/getpass.h b/compat/getpass.h\nnew file mode 100644\nindex 0000000..5f1986b\n--- /dev/null\n+++ b/compat/getpass.h\n@@ -0,0 +1,6 @@\n+#ifndef GETPASS_H\n+#define GETPASS_H\n+\n+char *getpass_echo(const char *prompt);\n+\n+#endif /* GETPASS_H */\n-- \n1.7.7.4.7.g24824\n"},{"id":"180007","messageId":"20111127083025.GD1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 4/6] prompt: add PROMPT_ECHO flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:30:26Z","receivedAt":"2011-11-27T08:30:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This will use getpass_echo when set.\n\nSigned-off-by: Jeff King \n---\n prompt.c | 5 ++++-\n prompt.h | 1 +\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/prompt.c b/prompt.c\nindex 7c8f9aa..c46227f 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -2,6 +2,7 @@\n #include \"run-command.h\"\n #include \"strbuf.h\"\n #include \"prompt.h\"\n+#include \"compat/getpass.h\"\n \n static char *do_askpass(const char *cmd, const char *prompt, const char *name)\n {\n@@ -48,7 +49,9 @@\n \t\t\treturn do_askpass(askpass, prompt, name);\n \t}\n \n-\treturn getpass(prompt);\n+\treturn flags & PROMPT_ECHO ?\n+\t\tgetpass_echo(prompt) :\n+\t\tgetpass(prompt);\n }\n \n char *git_getpass(const char *prompt)\ndiff --git a/prompt.h b/prompt.h\nindex 18868c2..7201cae 100644\n--- a/prompt.h\n+++ b/prompt.h\n@@ -2,6 +2,7 @@\n #define PROMPT_H\n \n #define PROMPT_ASKPASS (1<<0)\n+#define PROMPT_ECHO (1<<1)\n \n char *git_prompt(const char *prompt, const char *name, int flags);\n char *git_getpass(const char *prompt);\n-- \n1.7.7.4.7.g24824\n"},{"id":"180008","messageId":"20111127083113.GE1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 5/6] credential: use git_prompt instead of git_getpass","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:31:13Z","receivedAt":"2011-11-27T08:31:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we request a username and password from the user on the\nterminal, we can't use stdin/stdout, because they may be\nconnected to pipes to other git processes.\n\nInstead, we use getpass() (via git_getpass), which will\ngenerally open /dev/tty and read and write from that. The\nonly problem is that getpass is meant to take passwords, and\ntherefore will not echo characters from the username, which\nis annoying.\n\nNow that git_prompt understand the \"echo\" flag, we can use\nthat to let the user see their username as they type it.\n\nSigned-off-by: Jeff King \n---\n credential.c | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/credential.c b/credential.c\nindex d918ba5..9ad580d 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -109,7 +109,8 @@ static void credential_describe(struct credential *c, struct strbuf *out)\n \t\tstrbuf_addf(out, \"/%s\", c->path);\n }\n \n-static char *credential_ask_one(const char *what, struct credential *c)\n+static char *credential_ask_one(const char *what, struct credential *c,\n+\t\t\t\tint flags)\n {\n \tstruct strbuf desc = STRBUF_INIT;\n \tstruct strbuf prompt = STRBUF_INIT;\n@@ -121,11 +122,7 @@ static void credential_describe(struct credential *c, struct strbuf *out)\n \telse\n \t\tstrbuf_addf(&prompt, \"%s: \", what);\n \n-\t/* FIXME: for usernames, we should do something less magical that\n-\t * actually echoes the characters. However, we need to read from\n-\t * /dev/tty and not stdio, which is not portable (but getpass will do\n-\t * it for us). http.c uses the same workaround. */\n-\tr = git_getpass(prompt.buf);\n+\tr = git_prompt(prompt.buf, what, flags);\n \n \tstrbuf_release(&desc);\n \tstrbuf_release(&prompt);\n@@ -135,9 +132,11 @@ static void credential_describe(struct credential *c, struct strbuf *out)\n static void credential_getpass(struct credential *c)\n {\n \tif (!c->username)\n-\t\tc->username = credential_ask_one(\"Username\", c);\n+\t\tc->username = credential_ask_one(\"Username\", c,\n+\t\t\t\t\t\t PROMPT_ASKPASS|PROMPT_ECHO);\n \tif (!c->password)\n-\t\tc->password = credential_ask_one(\"Password\", c);\n+\t\tc->password = credential_ask_one(\"Password\", c,\n+\t\t\t\t\t\t PROMPT_ASKPASS);\n }\n \n int credential_read(struct credential *c, FILE *fp)\n-- \n1.7.7.4.7.g24824\n"},{"id":"180009","messageId":"20111127083151.GF1702@sigill.intra.peff.net","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"[PATCH 6/6] compat/getpass: add a /dev/tty implementation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-27T08:31:51Z","receivedAt":"2011-11-27T08:31:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is more or less what regular getpass() does, but\nwithout turning off character echoing. You have to set\nHAVE_DEV_TTY to enable it.\n\nFor now, only Linux enables this by default. People on other\n/dev/tty-enabled systems can submit patches to turn it on\nonce they have tested it.\n\nSigned-off-by: Jeff King \n---\n Makefile | 8 ++++++++\n compat/getpass.c | 35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d133e2b..a2afe31 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -227,6 +227,9 @@ all::\n #\n # Define NO_REGEX if you have no or inferior regex support in your C library.\n #\n+# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the\n+# user.\n+#\n # Define GETTEXT_POISON if you are debugging the choice of strings marked\n # for translation. In a GETTEXT_POISON build, you can turn all strings marked\n # for translation into gibberish by setting the GIT_GETTEXT_POISON variable\n@@ -835,6 +838,7 @@ ifeq ($(uname_S),Linux)\n \tNO_STRLCPY = YesPlease\n \tNO_MKSTEMPS = YesPlease\n \tHAVE_PATHS_H = YesPlease\n+\tHAVE_DEV_TTY = YesPlease\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tNO_STRLCPY = YesPlease\n@@ -1641,6 +1645,10 @@ ifdef HAVE_PATHS_H\n \tBASIC_CFLAGS += -DHAVE_PATHS_H\n endif\n \n+ifdef HAVE_DEV_TTY\n+\tBASIC_CFLAGS += -DHAVE_DEV_TTY\n+endif\n+\n ifdef DIR_HAS_BSD_GROUP_SEMANTICS\n \tCOMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS\n endif\ndiff --git a/compat/getpass.c b/compat/getpass.c\nindex 8ae82f0..b5bd1dd 100644\n--- a/compat/getpass.c\n+++ b/compat/getpass.c\n@@ -1,6 +1,41 @@\n #include \"../git-compat-util.h\"\n \n+#ifdef HAVE_DEV_TTY\n+\n+char *getpass_echo(const char *prompt)\n+{\n+\tstatic char buf[1024];\n+\tFILE *fh;\n+\tint i;\n+\n+\tfh = fopen(\"/dev/tty\", \"w+\");\n+\tif (!fh)\n+\t\treturn NULL;\n+\n+\tfputs(prompt, fh);\n+\tfflush(fh);\n+\tfor (i = 0; i < sizeof(buf) - 1; i++) {\n+\t\tint ch = getc(fh);\n+\t\tif (ch == EOF || ch == '\\n')\n+\t\t\tbreak;\n+\t\tbuf[i] = ch;\n+\t}\n+\tbuf[i] = '\\0';\n+\n+\tif (ferror(fh)) {\n+\t\tfclose(fh);\n+\t\treturn NULL;\n+\t}\n+\n+\tfclose(fh);\n+\treturn buf;\n+}\n+\n+#else\n+\n char *getpass_echo(const char *prompt)\n {\n \treturn getpass(prompt);\n }\n+\n+#endif\n-- \n1.7.7.4.7.g24824\n"},{"id":"180011","messageId":"7v4nxqvt7r.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-27T08:56:56Z","receivedAt":"2011-11-27T08:56:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:\n>\n>> Here's a revised version of the http-auth / credential-helper series.\n>\n> And here's something I've been meaning to do on top: actually echo\n> characters at the username prompt. We can't do this portably, but we can\n> at least stub out a compatibility layer and let each system do something\n> sensible.\n\nYay ;-)\n"},{"id":"180012","messageId":"CABPQNSb09kxjvdvz2P-WqU2VtMspaaA8ujTNLJ4+MuCrV=2zsw@mail.gmail.com","threadId":"29017","inReplyTo":"20111127082744.GA32068@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-27T09:17:26Z","receivedAt":"2011-11-27T09:17:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Sun, Nov 27, 2011 at 9:27 AM, Jeff King wrote:\n> On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:\n>\n>> Here's a revised version of the http-auth / credential-helper series.\n>\n> And here's something I've been meaning to do on top: actually echo\n> characters at the username prompt. We can't do this portably, but we can\n> at least stub out a compatibility layer and let each system do something\n> sensible.\n>\n>  [1/6]: move git_getpass to its own source file\n>  [2/6]: refactor git_getpass into generic prompt function\n>  [3/6]: stub out getpass_echo function\n>  [4/6]: prompt: add PROMPT_ECHO flag\n>  [5/6]: credential: use git_prompt instead of git_getpass\n>  [6/6]: compat/getpass: add a /dev/tty implementation\n>\n> -Peff\n\nInteresting, I've been working on something pretty similar: getting\nrid of getpass usage all together:\n\nhttps://github.com/kusma/git/tree/work/askpass\n\nMy reason to write a getpass replacement was to avoid capping input to\nPASS_MAX, which can be as low as 8 characters (and AFAIK is just that\non Solaris)...\n"},{"id":"180030","messageId":"20111128035321.GA15640@sigill.intra.peff.net","threadId":"29017","inReplyTo":"CABPQNSb09kxjvdvz2P-WqU2VtMspaaA8ujTNLJ4+MuCrV=2zsw@mail.gmail.com","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-28T03:53:21Z","receivedAt":"2011-11-28T03:53:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 27, 2011 at 10:17:26AM +0100, Erik Faye-Lund wrote:\n\n> > And here's something I've been meaning to do on top: actually echo\n> > characters at the username prompt. We can't do this portably, but we can\n> > at least stub out a compatibility layer and let each system do something\n> > sensible.\n>\n> Interesting, I've been working on something pretty similar: getting\n> rid of getpass usage all together:\n> \n> https://github.com/kusma/git/tree/work/askpass\n> \n> My reason to write a getpass replacement was to avoid capping input to\n> PASS_MAX, which can be as low as 8 characters (and AFAIK is just that\n> on Solaris)...\n\nYeah, if there are really bad getpass implementations, we would want to\nwork around them. If we are going to do so, it might make sense to\ncombine the effort with my getpass_echo wrapper, as they are really the\nsame function, modulo tweaking the echo settings.\n\nIt would also be nice to make getpass a little more predictable. If\n/dev/tty can't be opened, glibc's getpass will fall back to writing the\nprompt to stderr and reading the password from stdin. But we definitely\ndon't want to do that in git-remote-curl, where stdin is already talking\na special protocol with the parent fetch process.\n\nSo I think it might be best to just write our own getpass. However,\nyour implementation looks wrong to me:\n\n> diff --git a/password.c b/password.c\n> new file mode 100644\n> index 0000000..727ed84\n> --- /dev/null\n> +++ b/password.c\n> @@ -0,0 +1,94 @@\n> +#include \"cache.h\"\n> +#include \"git-compat-util.h\"\n> +#include \"strbuf.h\"\n> +#include \"run-command.h\"\n> +\n> +#ifndef WIN32\n> +\n> +static void ask_password(struct strbuf *sb, const char *prompt)\n> +{\n> +\tstruct termios attr;\n> +\ttcflag_t c_lflag;\n> +\n> +\tif (tcgetattr(1, &attr) < 0)\n> +\t\tdie(\"tcgetattr failed!\");\n> +\tc_lflag = attr.c_lflag;\n\nThis is getting the terminal attributes for stdout. But in many\ncases, stdout will not be connected to the terminal (in particular,\nremote-curl, as I mentioned above, will have its stdio connected to the\nparent fetch process). Stderr is a better guess, as you do here:\n\n> +\tfputs(prompt, stderr);\n\nbut even that is not foolproof. With getpass(), this should work:\n\n git clone ... 2>errors\n\nwith the prompt going to the terminal. But it doesn't with your patch.\n\nYou really want to open \"/dev/tty\" on most Unix systems (which is what\ngetpass() does). I have no idea what would be appropriate on Windows.\n\n> +\tfor (;;) {\n> +\t\tint c = getchar();\n> +\t\tif (c == EOF || c == '\\n')\n> +\t\t\tbreak;\n> +\t\tstrbuf_addch(sb, c);\n> +\t}\n\nAnd this is even worse. You're reading from stdin, which will get\nwhatever cruft is in the pipe coming from the parent process (or may\neven cause a hang, as the parent is probably blocking waiting to read\nfrom the child helper).\n\n-Peff\n"},{"id":"180038","messageId":"CABPQNSbLvWh-ivaqBk-Du+kwZvV3t+ajEJhHATRzyGZbHYyM=Q@mail.gmail.com","threadId":"29017","inReplyTo":"20111128035321.GA15640@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-28T09:36:21Z","receivedAt":"2011-11-28T09:36:21Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Nov 28, 2011 at 4:53 AM, Jeff King wrote:\n> On Sun, Nov 27, 2011 at 10:17:26AM +0100, Erik Faye-Lund wrote:\n>\n>> > And here's something I've been meaning to do on top: actually echo\n>> > characters at the username prompt. We can't do this portably, but we can\n>> > at least stub out a compatibility layer and let each system do something\n>> > sensible.\n>>\n>> Interesting, I've been working on something pretty similar: getting\n>> rid of getpass usage all together:\n>>\n>> https://github.com/kusma/git/tree/work/askpass\n>>\n>> My reason to write a getpass replacement was to avoid capping input to\n>> PASS_MAX, which can be as low as 8 characters (and AFAIK is just that\n>> on Solaris)...\n>\n> Yeah, if there are really bad getpass implementations, we would want to\n> work around them. If we are going to do so, it might make sense to\n> combine the effort with my getpass_echo wrapper, as they are really the\n> same function, modulo tweaking the echo settings.\n>\n\nMy thinking exactly ;)\n\n> It would also be nice to make getpass a little more predictable. If\n> /dev/tty can't be opened, glibc's getpass will fall back to writing the\n> prompt to stderr and reading the password from stdin. But we definitely\n> don't want to do that in git-remote-curl, where stdin is already talking\n> a special protocol with the parent fetch process.\n>\n> So I think it might be best to just write our own getpass. However,\n> your implementation looks wrong to me:\n>\n\nIt probably is, yes. It was a very naive attempt ;)\n\n>> diff --git a/password.c b/password.c\n>> new file mode 100644\n>> index 0000000..727ed84\n>> --- /dev/null\n>> +++ b/password.c\n>> @@ -0,0 +1,94 @@\n>> +#include \"cache.h\"\n>> +#include \"git-compat-util.h\"\n>> +#include \"strbuf.h\"\n>> +#include \"run-command.h\"\n>> +\n>> +#ifndef WIN32\n>> +\n>> +static void ask_password(struct strbuf *sb, const char *prompt)\n>> +{\n>> +     struct termios attr;\n>> +     tcflag_t c_lflag;\n>> +\n>> +     if (tcgetattr(1, &attr) < 0)\n>> +             die(\"tcgetattr failed!\");\n>> +     c_lflag = attr.c_lflag;\n>\n> This is getting the terminal attributes for stdout. But in many\n> cases, stdout will not be connected to the terminal (in particular,\n> remote-curl, as I mentioned above, will have its stdio connected to the\n> parent fetch process).  Stderr is a better guess, as you do here:\n>\n>> +     fputs(prompt, stderr);\n>\n> but even that is not foolproof. With getpass(), this should work:\n>\n>  git clone ... 2>errors\n>\n> with the prompt going to the terminal. But it doesn't with your patch.\n>\n> You really want to open \"/dev/tty\" on most Unix systems (which is what\n> getpass() does).\n\nYes, you're right. Opening \"/dev/tty\" is much better. But what happens\nfor processes started by GUI applications (with no easily observable\ntty, if any)? Does open simply fail? If so, is it desirable for us to\nfail in that case?\n\n> I have no idea what would be appropriate on Windows.\n>\n\nIt's pretty similar, but not exactly: CreateFile(\"CONIN$\", ...) or\nCreateFile(\"CONOUT$\", ...), depending on if you want the read-handle\nor the write-handle... I can probably cook up something a bit more\nconcrete, though.\n\nBut _getch() that we already use always reads from the console\n(according to MSDN, I haven't actually tested this myself:\nhttp://msdn.microsoft.com/en-us/library/078sfkak%28v=VS.80%29.aspx).\nBut I don't think this allows us to fail when no console is attached.\nQuestion is, should we fail in such cases? Windows does have an API to\nprompt for passwords in a GUI window... Perhaps fallbacking to those\nare the way to go? Something like:\n\nif (GetConsoleWindow()) {\n\t/* normal console-stuff */\n} else {\n\t/* call CredUIPromptForWindowsCredentials(...) instead */\n}\n\nThis might be nicer towards GUI tools, but it requires us to\nexplicitly ask for a password (and possibly a username at the same\ntime)...\n\n>> +     for (;;) {\n>> +             int c = getchar();\n>> +             if (c == EOF || c == '\\n')\n>> +                     break;\n>> +             strbuf_addch(sb, c);\n>> +     }\n>\n> And this is even worse. You're reading from stdin, which will get\n> whatever cruft is in the pipe coming from the parent process (or may\n> even cause a hang, as the parent is probably blocking waiting to read\n> from the child helper).\n>\n\nYeah, thanks for pointing these glitches out ;)\n"},{"id":"180043","messageId":"20111128113127.GA23408@sigill.intra.peff.net","threadId":"29017","inReplyTo":"CABPQNSbLvWh-ivaqBk-Du+kwZvV3t+ajEJhHATRzyGZbHYyM=Q@mail.gmail.com","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-28T11:31:27Z","receivedAt":"2011-11-28T11:31:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 28, 2011 at 10:36:21AM +0100, Erik Faye-Lund wrote:\n\n> > You really want to open \"/dev/tty\" on most Unix systems (which is what\n> > getpass() does).\n> \n> Yes, you're right. Opening \"/dev/tty\" is much better. But what happens\n> for processes started by GUI applications (with no easily observable\n> tty, if any)? Does open simply fail? If so, is it desirable for us to\n> fail in that case?\n\nYes, the open will fail (on Linux, I get ENXIO).\n\nAnd yes, we should fail in that case. getpass() will generally return\nNULL in that instance, and the current implementation of git_getpass()\nwill die(), explaining that we could not get the password.\n\n> > I have no idea what would be appropriate on Windows.\n> \n> It's pretty similar, but not exactly: CreateFile(\"CONIN$\", ...) or\n> CreateFile(\"CONOUT$\", ...), depending on if you want the read-handle\n> or the write-handle... I can probably cook up something a bit more\n> concrete, though.\n\nOK, that maps to the /dev/tty concept quite well. Though I suspect the\ncode for turning off character echo-ing is going to also be different.\n\n> But _getch() that we already use always reads from the console\n> (according to MSDN, I haven't actually tested this myself:\n> http://msdn.microsoft.com/en-us/library/078sfkak%28v=VS.80%29.aspx).\n> But I don't think this allows us to fail when no console is attached.\n> Question is, should we fail in such cases? Windows does have an API to\n> prompt for passwords in a GUI window... Perhaps fallbacking to those\n> are the way to go? Something like:\n> \n> if (GetConsoleWindow()) {\n> \t/* normal console-stuff */\n> } else {\n> \t/* call CredUIPromptForWindowsCredentials(...) instead */\n> }\n\nCertainly on non-Windows something like that would not be welcome. The\nuser can already have specified GIT_ASKPASS if they don't have a\nterminal. And once the credential-helper code is in, they can use a\nplatform-specific helper that provides a nice dialog if they want it.\n\nSo I would say trying to do something graphical would be surprising and\nunwelcome. But then, I am a very Unix-y kind of guy. Maybe on Windows\nsomething like that would be more favorable. I'll leave that decision to\npeople who know more.\n\n-Peff\n"},{"id":"180044","messageId":"CAH6sp9MvS1naHpLXwkCpPyU0-Z+CHqrVwJvjBbWs=hvin0ZMvQ@mail.gmail.com","threadId":"29017","inReplyTo":"20111128113127.GA23408@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2011-11-28T11:49:10Z","receivedAt":"2011-11-28T11:49:10Z","isPatch":true,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"On Mon, Nov 28, 2011 at 12:31 PM, Jeff King wrote:\n\n> Certainly on non-Windows something like that would not be welcome. The\n> user can already have specified GIT_ASKPASS if they don't have a\n> terminal. And once the credential-helper code is in, they can use a\n> platform-specific helper that provides a nice dialog if they want it.\n>\n> So I would say trying to do something graphical would be surprising and\n> unwelcome. But then, I am a very Unix-y kind of guy. Maybe on Windows\n> something like that would be more favorable. I'll leave that decision to\n> people who know more.\n\nI would say that also on windows it would be surprising if you are\nworking on the command line and suddenly a pop-up appears asking for\ninput. So even on windows you should probably keep away from gui stuff\nin a cli tool, although there are tools that don't.\n\nFrans\n"},{"id":"180045","messageId":"CABPQNSYd8PFsoRi6NtjQYNQzM+NHv_aRCLRWQ=XsFuw2gGWAng@mail.gmail.com","threadId":"29017","inReplyTo":"20111128113127.GA23408@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-28T12:59:34Z","receivedAt":"2011-11-28T12:59:34Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Nov 28, 2011 at 12:31 PM, Jeff King wrote:\n> On Mon, Nov 28, 2011 at 10:36:21AM +0100, Erik Faye-Lund wrote:\n>\n>> > You really want to open \"/dev/tty\" on most Unix systems (which is what\n>> > getpass() does).\n>>\n>> Yes, you're right. Opening \"/dev/tty\" is much better. But what happens\n>> for processes started by GUI applications (with no easily observable\n>> tty, if any)? Does open simply fail? If so, is it desirable for us to\n>> fail in that case?\n>\n> Yes, the open will fail (on Linux, I get ENXIO).\n>\n> And yes, we should fail in that case. getpass() will generally return\n> NULL in that instance, and the current implementation of git_getpass()\n> will die(), explaining that we could not get the password.\n>\n>> > I have no idea what would be appropriate on Windows.\n>>\n>> It's pretty similar, but not exactly: CreateFile(\"CONIN$\", ...) or\n>> CreateFile(\"CONOUT$\", ...), depending on if you want the read-handle\n>> or the write-handle... I can probably cook up something a bit more\n>> concrete, though.\n>\n> OK, that maps to the /dev/tty concept quite well. Though I suspect the\n> code for turning off character echo-ing is going to also be different.\n>\n>> But _getch() that we already use always reads from the console\n>> (according to MSDN, I haven't actually tested this myself:\n>> http://msdn.microsoft.com/en-us/library/078sfkak%28v=VS.80%29.aspx).\n>> But I don't think this allows us to fail when no console is attached.\n>> Question is, should we fail in such cases? Windows does have an API to\n>> prompt for passwords in a GUI window... Perhaps fallbacking to those\n>> are the way to go? Something like:\n>>\n>> if (GetConsoleWindow()) {\n>>       /*  normal console-stuff */\n>> } else {\n>>       /* call CredUIPromptForWindowsCredentials(...) instead */\n>> }\n>\n> Certainly on non-Windows something like that would not be welcome. The\n> user can already have specified GIT_ASKPASS if they don't have a\n> terminal. And once the credential-helper code is in, they can use a\n> platform-specific helper that provides a nice dialog if they want it.\n>\n\nYes, that's certainly cleaner implementation-wise. But didn't you\nchange it to only do the storage-part in the last round, or did I\nmisunderstand the updated series?\n\n> So I would say trying to do something graphical would be surprising and\n> unwelcome. But then, I am a very Unix-y kind of guy. Maybe on Windows\n> something like that would be more favorable. I'll leave that decision to\n> people who know more.\n\nWindows doesn't really have that strict norms when it comes to console\napplications, but it'd be nice if it didn't do anything obviously\nwrong when the GUI isn't available (non-interactive sessions,\nPowerShell remote commands, CopSSH, etc). So I guess this is yet\nanother argument to stay with the credential-helper instead, if\npossible...\n"},{"id":"180057","messageId":"20111128185956.GA28191@sigill.intra.peff.net","threadId":"29017","inReplyTo":"CABPQNSYd8PFsoRi6NtjQYNQzM+NHv_aRCLRWQ=XsFuw2gGWAng@mail.gmail.com","subject":"Re: [PATCH 0/6] echo usernames as they are typed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-28T18:59:56Z","receivedAt":"2011-11-28T18:59:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 28, 2011 at 01:59:34PM +0100, Erik Faye-Lund wrote:\n\n> > Certainly on non-Windows something like that would not be welcome. The\n> > user can already have specified GIT_ASKPASS if they don't have a\n> > terminal. And once the credential-helper code is in, they can use a\n> > platform-specific helper that provides a nice dialog if they want it.\n> >\n> \n> Yes, that's certainly cleaner implementation-wise. But didn't you\n> change it to only do the storage-part in the last round, or did I\n> misunderstand the updated series?\n\nYeah, sorry, I'm getting ahead of myself. I left room in the spec for an\n\"ask\" operation on helpers, but I haven't implemented it yet.\n\n-Peff\n"},{"id":"180066","messageId":"7v4nxnvs1w.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111124110105.GA8417@sigill.intra.peff.net","subject":"Re: [PATCH 03/13] introduce credentials API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-28T21:46:35Z","receivedAt":"2011-11-28T21:46:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\n> new file mode 100644\n> index 0000000..3061077\n> --- /dev/null\n> +++ b/Documentation/technical/api-credentials.txt\n> @@ -0,0 +1,148 @@\n> + ...\n> +`credential_fill`::\n> +\n> +\tAttempt to fill the username and password fields of the passed\n> +\tcredential struct, first consulting storage helpers, then asking\n> +\tthe user. Guarantees that the username and password fields will\n> +\tbe filled afterwards (or die() will be called).\n> +\n> +`credential_reject`::\n> +\n> +\tInform the credential subsystem that the provided credentials\n> +\thave been rejected. This will notify any storage helpers of the\n> +\trejection (which allows them to, for example, purge the invalid\n> +\tcredentials from storage), and then clear the username and\n> +\tpassword fields in `struct credential`. It can then be\n> +\t`credential_fill`-ed again.\n> +\n> +`credential_approve`::\n> +\n> +\tInform the credential subsystem that the provided credentials\n> +\twere successfully used for authentication. This will notify any\n> +\tstorage helpers of the approval, so that they can store the\n> +\tresult to be used again.\n\nIt's a bit hard to read and understand which part of the system calls\nthese and which other part of the system is responsible for implementing\nthem, and how \"helper\" fits into the picture (perhaps calling some of\nthese interfaces will result in \"helper\" getting called?).\n\n> +Credential Storage Helpers\n> +--------------------------\n> +\n> +Credential storage helpers are programs executed by git to fetch or save\n> +credentials from and to long-term storage (where \"long-term\" is simply\n> +longer than a single git process; e.g., credentials may be stored\n> +in-memory for a few minutes, or indefinitely on disk).\n> +\n> +Helper scripts should generally be found in the PATH, and have names of\n> +the form \"git-credential-$HELPER\".\n\nIs this normal PATH or can a helper be moved away into $GIT_EXEC_PATH?\n\nI briefly wondered if they want to be git-credential--$HELPER; I do not\ndeeply care either way, though.\n\n> When the helper string \"$HELPER\" is\n> +passed to credential functions, they will run \"git-credential-$HELPER\"\n> +via the shell. If the first word of $HELPER contains non-alphanumeric\n> +characters, then $HELPER is executed as a shell command. This makes it\n> +possible to specify individual scripts by their full path (e.g.,\n> +`/path/to/helper`) or even shell snippets (`f() { do_whatever; }; f`).\n\nThe definition of \"the first word\" above is not specified but it seems to\nbe \"space separated\". In other words, 'f() { do_whatever; }; f' would be\nOK but 'f () { do_whatever; }; f' would not be. Am I reading and guessing\nyour intention correctly?\n\nFunnily enough, 'f() { do_whatever; }; f' would qualify as the first\nword having a non alphanumeric.\n\n> +The details of the credential will be provided on the helper's stdin\n> +stream. The credential is split into a set of named attributes.\n> +Attributes are provided to the helper, one per line. Each attribute is\n> +specified by a key-value pair, separated by an `=` (equals) sign,\n> +followed by a newline. The key may contain any bytes except `=` or\n> +newline. The value may contain any bytes except a newline. In both\n> +cases, all bytes are treated as-is (i.e., there is no quoting, and one\n> +cannot transmit a value with newline in it).\n\nCan k or v contain a NUL? The literal reading of the above implies they\ncould, but I do not think you meant to.\n\n> +int credential_read(struct credential *c, FILE *fp)\n> +{\n> ...\n> +\t\t\tc->host = xstrdup(value);\n> +\t\t}\n> +\t\telse if (!strcmp(key, \"path\")) {\n> ...\n> +\t\t/* ignore other lines; we don't know what they mean, but\n> +\t\t * this future-proofs us when later versions of git do\n> +\t\t * learn new lines, and the helpers are updated to match */\n\nTwo style nits.\n"},{"id":"180071","messageId":"7vr50ru5d3.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111124110710.GH8417@sigill.intra.peff.net","subject":"Re: [PATCH 10/13] credentials: add \"cache\" helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-29T00:42:00Z","receivedAt":"2011-11-29T00:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/t/lib-credential.sh b/t/lib-credential.sh\n> index 54ae1f4..ac54b38 100755\n> --- a/t/lib-credential.sh\n> +++ b/t/lib-credential.sh\n> @@ -21,6 +21,225 @@ read_chunk() {\n> ...\n> +\ttest_expect_success \"helper ($HELPER) can forget user\" '\n> +\t\tcheck reject $HELPER <<-\\EOF &&\n> +\t\tprotocol=https\n> +\t\thost=example.com\n> +\t\tusername=user1\n> +\t\tEOF\n> +\t\tcheck fill $HELPER <<-\\EOF\n> +\t\tprotocol=https\n> +\t\thost=example.com\n> +\t\tusername=user1\n> +\t\t--\n> +\t\tusername=user1\n> +\t\tpassword=askpass-password\n> +\t\t--\n> +\t\taskpass: Password for '\\''https://user1@example.com'\\'':\n> +\t'\n\nMissing EOF for here document; I fixed this already in 'pu', but please\nsquash it in when rerolling after 1.7.8 final.\n"},{"id":"180074","messageId":"20111129050425.GA32022@sigill.intra.peff.net","threadId":"29017","inReplyTo":"7v4nxnvs1w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 03/13] introduce credentials API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-29T05:04:25Z","receivedAt":"2011-11-29T05:04:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 28, 2011 at 01:46:35PM -0800, Junio C Hamano wrote:\n\n> > diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\n> > new file mode 100644\n> > index 0000000..3061077\n> > --- /dev/null\n> > +++ b/Documentation/technical/api-credentials.txt\n> > @@ -0,0 +1,148 @@\n> > + ...\n> > +`credential_fill`::\n> > +\n> > +\tAttempt to fill the username and password fields of the passed\n> > +\tcredential struct, first consulting storage helpers, then asking\n> > +\tthe user. Guarantees that the username and password fields will\n> > +\tbe filled afterwards (or die() will be called).\n> > +\n> > +`credential_reject`::\n> > +\n> > +\tInform the credential subsystem that the provided credentials\n> > +\thave been rejected. This will notify any storage helpers of the\n> > +\trejection (which allows them to, for example, purge the invalid\n> > +\tcredentials from storage), and then clear the username and\n> > +\tpassword fields in `struct credential`. It can then be\n> > +\t`credential_fill`-ed again.\n> > +\n> > +`credential_approve`::\n> > +\n> > +\tInform the credential subsystem that the provided credentials\n> > +\twere successfully used for authentication. This will notify any\n> > +\tstorage helpers of the approval, so that they can store the\n> > +\tresult to be used again.\n> \n> It's a bit hard to read and understand which part of the system calls\n> these and which other part of the system is responsible for implementing\n> them, and how \"helper\" fits into the picture (perhaps calling some of\n> these interfaces will result in \"helper\" getting called?).\n\nHow about following it with a rough example of how they would be used,\nlike:\n\n /*\n * Create a credential with some context; we don't yet know the\n * username or password.\n */\n struct credential c = CREDENTIAL_INIT;\n c.protocol = xstrdup(\"https\");\n c.host = xstrdup(\"example.com\");\n c.path = xstrdup(\"foo.git\");\n\n /*\n * Fill in the username and password fields by contacting helpers\n * and/or asking the user. The function will die if it fails.\n */\n credential_fill(&c);\n\n /*\n * And then finally make our request, reporting back to the credential\n * system on whether it succeeded or failed.\n */\n if (make_http_request(c.host, c.path, c.username, c.password) < 0)\n credential_reject(&c);\n else\n credential_accept(&c);\n\nDoes that make it more clear?\n\n> > +Credential Storage Helpers\n> > +--------------------------\n> > +\n> > +Credential storage helpers are programs executed by git to fetch or save\n> > +credentials from and to long-term storage (where \"long-term\" is simply\n> > +longer than a single git process; e.g., credentials may be stored\n> > +in-memory for a few minutes, or indefinitely on disk).\n> > +\n> > +Helper scripts should generally be found in the PATH, and have names of\n> > +the form \"git-credential-$HELPER\".\n> \n> Is this normal PATH or can a helper be moved away into $GIT_EXEC_PATH?\n\nThey are executed as normal git commands, so $GIT_EXEC_PATH. Actually,\nthey could even be aliases or builtins, for that matter. I'll update the\ndocumentation to reflect that.\n\n> I briefly wondered if they want to be git-credential--$HELPER; I do not\n> deeply care either way, though.\n\nYeah, I thought about that. In some ways they are implementation details\nthat you don't care about, and that makes the \"--\" separator make sense.\nBut some of them may want to expose other actions to the user. For\nexample, git-credential-cache has an \"exit\" action that you can call to\nask the cache daemon to exit prematurely, forgetting all credentials.\nGit will never use it; it's purely an end-user interface.\n\nI think it might also make sense to ask helpers to provide a \"clear\"\noperation which will delete any stored items. Git wouldn't use it, but\nit would be helpful for testing.\n\n> > When the helper string \"$HELPER\" is\n> > +passed to credential functions, they will run \"git-credential-$HELPER\"\n> > +via the shell. If the first word of $HELPER contains non-alphanumeric\n> > +characters, then $HELPER is executed as a shell command. This makes it\n> > +possible to specify individual scripts by their full path (e.g.,\n> > +`/path/to/helper`) or even shell snippets (`f() { do_whatever; }; f`).\n> \n> The definition of \"the first word\" above is not specified but it seems to\n> be \"space separated\". In other words, 'f() { do_whatever; }; f' would be\n> OK but 'f () { do_whatever; }; f' would not be. Am I reading and guessing\n> your intention correctly?\n\nYes, that is my intent, though I am not altogether happy with it, as\nthere are too many confusing corner cases (e.g., your \"f ()\" example\nabove). My thinking was guided by these requirements:\n\n 1. Users who are using one of the included helpers or a drop-in\n third-party helper should be able to just do:\n\n git config credential.helper cache\n\n which is extremely readable and intuitive. Thus, I want to prepend\n \"git credential-\" at least for simple cases.\n\n 2. You should be able to add arguments to a helper, if it takes them.\n For example, with the current code you can do:\n\n git config credential.helper 'cache --timeout=300'\n\n or\n\n git config credential.helper 'store --store=/some/path'\n\n and that's all interpreted by the shell, and the helpers receive\n your configuration (the \"--store\" name is a holdover from version 1\n of the series, which had many more options. Probably \"--path\" or\n \"--file\" would be better name).\n\n So that means that the rule for when to prepend \"git credential-\"\n is not simply \"when the helper is a single token\". It can be\n arbitrary arguments, and should be interpreted by the shell (to\n handle things like quoting).\n\n 3. You should be able to use a full path, like:\n\n git config credential.helper '/path/to/git-credential-foo'\n\n This works with the isalnum thing. But you could also use\n is_absolute_path.\n\n 4. You should be able to insert full shell snippets if you really want\n to. This is handy for testing, and I could even see somebody doing\n something like:\n\n git config credential.helper '\n f() {\n case \"`uname -s`\" in\n Linux) git credential-gnome \"$@\" ;;\n Darwin) git credential-osx \"$@\" ;;\n esac\n }\n f'\n\n if they have a multi-platform config file (or they could check\n $DISPLAY, or `hostname`, or any of a zillion other things). You\n could put it in a shell script, of course, but sometimes it is\n nice to carry little snippets like that in the config file.\n\n You could do something like \"prepend ! to execute via the shell\",\n like we do for aliases. But that is perhaps a little confusing, in\n that we are _always_ executing via the shell. It is just that\n sometimes we prepend some magic to the command name. I dunno.\n\nSo one possible rule would be:\n\n 1. If it starts with \"!\", clip off the \"!\" and hand it to the shell.\n\n 2. Otherwise, if is_absolute_path(), hand it to the shell directly.\n\n 3. Otherwise, prepend \"git credential-\" and hand it to the shell.\n\nI think that is slightly less confusing than the \"first word is alnum\"\nthing.\n\n> Funnily enough, 'f() { do_whatever; }; f' would qualify as the first\n> word having a non alphanumeric.\n\nYeah, arguably a bug in my code. ;)\n\n> > +The details of the credential will be provided on the helper's stdin\n> > +stream. The credential is split into a set of named attributes.\n> > +Attributes are provided to the helper, one per line. Each attribute is\n> > +specified by a key-value pair, separated by an `=` (equals) sign,\n> > +followed by a newline. The key may contain any bytes except `=` or\n> > +newline. The value may contain any bytes except a newline. In both\n> > +cases, all bytes are treated as-is (i.e., there is no quoting, and one\n> > +cannot transmit a value with newline in it).\n> \n> Can k or v contain a NUL? The literal reading of the above implies they\n> could, but I do not think you meant to.\n\nNo, they cannot. I'll make that more explicit in the next version.\n\nHow do you feel about the \"values cannot contain a newline\" requirement?\nIt feels wrong to me to design a protocol with no escape hatch for\nquoting. But I really didn't want helper implementations to have to deal\nwith unquoting or other parsing headaches. And terminating with NUL\ninstead of newline makes things annoying in some languages (e.g.,\nshell). So I considered it \"good enough\", but I'd be curious to hear\nother opinions.\n\n> > +int credential_read(struct credential *c, FILE *fp)\n> > +{\n> > ...\n> > +\t\t\tc->host = xstrdup(value);\n> > +\t\t}\n> > +\t\telse if (!strcmp(key, \"path\")) {\n> > ...\n> > +\t\t/* ignore other lines; we don't know what they mean, but\n> > +\t\t * this future-proofs us when later versions of git do\n> > +\t\t * learn new lines, and the helpers are updated to match */\n> \n> Two style nits.\n\nI'm supposed to guess? ;P\n\nI know you like blank lines at the top and bottom of multi-line\ncomments; I often forget that, as they are not part of my usual style (I\nfind them no easier to read).\n\nI'm guessing the other is an uncuddled else? I can never remember which\nwe prefer, as we are horribly inconsistent in the current code.\n\n-Peff\n"},{"id":"180075","messageId":"20111129050453.GB32022@sigill.intra.peff.net","threadId":"29017","inReplyTo":"7vr50ru5d3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 10/13] credentials: add \"cache\" helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-29T05:04:53Z","receivedAt":"2011-11-29T05:04:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 28, 2011 at 04:42:00PM -0800, Junio C Hamano wrote:\n\n> > +\t\taskpass: Password for '\\''https://user1@example.com'\\'':\n> > +\t'\n> \n> Missing EOF for here document; I fixed this already in 'pu', but please\n> squash it in when rerolling after 1.7.8 final.\n\nThanks, will do.\n\n-Peff\n"},{"id":"180094","messageId":"7vmxbeu91d.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111129050425.GA32022@sigill.intra.peff.net","subject":"Re: [PATCH 03/13] introduce credentials API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-29T17:34:54Z","receivedAt":"2011-11-29T17:34:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Nov 28, 2011 at 01:46:35PM -0800, Junio C Hamano wrote:\n>\n>> > diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\n>> > new file mode 100644\n>> > index 0000000..3061077\n>> > --- /dev/null\n>> > +++ b/Documentation/technical/api-credentials.txt\n>> > @@ -0,0 +1,148 @@\n>> > + ...\n>> > +`credential_fill`::\n>> > +\n>> > +\tAttempt to fill the username and password fields of the passed\n>> > +\tcredential struct, first consulting storage helpers, then asking\n>> > +\tthe user. Guarantees that the username and password fields will\n>> > +\tbe filled afterwards (or die() will be called).\n>> > +\n>> > +`credential_reject`::\n>> > +\n>> > +\tInform the credential subsystem that the provided credentials\n>> > +\thave been rejected. This will notify any storage helpers of the\n>> > +\trejection (which allows them to, for example, purge the invalid\n>> > +\tcredentials from storage), and then clear the username and\n>> > +\tpassword fields in `struct credential`. It can then be\n>> > +\t`credential_fill`-ed again.\n>> > +\n>> > +`credential_approve`::\n>> > +\n>> > +\tInform the credential subsystem that the provided credentials\n>> > +\twere successfully used for authentication. This will notify any\n>> > +\tstorage helpers of the approval, so that they can store the\n>> > +\tresult to be used again.\n>> \n>> It's a bit hard to read and understand which part of the system calls\n>> these and which other part of the system is responsible for implementing\n>> them, and how \"helper\" fits into the picture (perhaps calling some of\n>> these interfaces will result in \"helper\" getting called?).\n>\n> How about following it with a rough example of how they would be used,\n> like:\n>\n> /*\n> * Create a credential with some context; we don't yet know the\n> * username or password.\n> */\n> struct credential c = CREDENTIAL_INIT;\n> c.protocol = xstrdup(\"https\");\n> c.host = xstrdup(\"example.com\");\n> c.path = xstrdup(\"foo.git\");\n>\n> /*\n> * Fill in the username and password fields by contacting helpers\n> * and/or asking the user. The function will die if it fails.\n> */\n> credential_fill(&c);\n>\n> /*\n> * And then finally make our request, reporting back to the credential\n> * system on whether it succeeded or failed.\n> */\n> if (make_http_request(c.host, c.path, c.username, c.password) < 0)\n> credential_reject(&c);\n> else\n> credential_accept(&c);\n>\n> Does that make it more clear?\n\nImmensely, at least to me. From the perspective of a user of the API, a\ncall to credential_fill() is to \"fill in the credential\" in the sense that\n\"call the function to fill in the credential\" but I find it easier to\nunderstand if it were explained to me as \"ask the API to fill in the\ncredential, which may involve helpers to interact with the user--the point\nof the API is that the caller does not care how it is done\". Same for the\nreject/accept calls---the example makes it clear that they are to tell the\ndecision to reject/accept made by the application to the credential API,\nand it is up to the API layer what it does using that decision (like\nremoving the cached and now stale password).\n\nThe above example is a bit too simplistic and misleading, though. You\nwould call reject only on authentication failure (do not trash stored and\ngood password upon network being unreachable temporarily or the server\nbeing overloaded).\n\n> So one possible rule would be:\n>\n> 1. If it starts with \"!\", clip off the \"!\" and hand it to the shell.\n>\n> 2. Otherwise, if is_absolute_path(), hand it to the shell directly.\n>\n> 3. Otherwise, prepend \"git credential-\" and hand it to the shell.\n>\n> I think that is slightly less confusing than the \"first word is alnum\"\n> thing.\n\nSimpler and easier to explain. Good ;-)\n\n> How do you feel about the \"values cannot contain a newline\" requirement?\n\nIn the context of asking username, password, or passphrase, I think \"LF is\nthe end of the line and you cannot have that byte in your response\" is\nperfectly reasonable. I've yet to find a way to use LF in a passphrase to\nunlock my Gnome keychain ;-).\n\n>> > +int credential_read(struct credential *c, FILE *fp)\n>> > +{\n>> > ...\n>> > +\t\t\tc->host = xstrdup(value);\n>> > +\t\t}\n>> > +\t\telse if (!strcmp(key, \"path\")) {\n>> > ...\n>> > +\t\t/* ignore other lines; we don't know what they mean, but\n>> > +\t\t * this future-proofs us when later versions of git do\n>> > +\t\t * learn new lines, and the helpers are updated to match */\n>> \n>> Two style nits.\n>\n> I'm supposed to guess? ;P\n\nSorry, but you guessed right.\n"},{"id":"180098","messageId":"7vzkfessff.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111124110728.GI8417@sigill.intra.peff.net","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-29T18:19:00Z","receivedAt":"2011-11-29T18:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This just follows the rfc3986 rules for percent-encoding\n> url data into a strbuf.\n>\n> Signed-off-by: Jeff King \n> ---\n> strbuf.c | 37 +++++++++++++++++++++++++++++++++++++\n> strbuf.h | 5 +++++\n> 2 files changed, 42 insertions(+), 0 deletions(-)\n>\n> diff --git a/strbuf.c b/strbuf.c\n> index 3ad2cc0..60e5e59 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -397,3 +397,40 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n> \n> \treturn len;\n> }\n> +\n> +static int is_rfc3986_reserved(char ch)\n> +{\n> +\tswitch (ch) {\n> +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n> +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n> +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n\nPart of me wonders if we still have extra bits in sane_ctype[] array but\nthat one is cumbersome to update, and the above should be easier to read\nand maintain.\n\n> +void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,\n> +\t\t\t int reserved)\n\nDoes \"reserved\" parameter mean \"must-encode-reserved\", or\n\"may-encode-reserved\" (the latter would be more like \"if set to 0,\nper-cent encoding the result would be an error\")?\n"},{"id":"180099","messageId":"7vsjl6ssf9.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111124110756.GJ8417@sigill.intra.peff.net","subject":"Re: [PATCH 12/13] credentials: add \"store\" helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-29T18:19:06Z","receivedAt":"2011-11-29T18:19:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> +static void parse_credential_file(const char *fn,\n> +\t\t\t\t struct credential *c,\n> +\t\t\t\t void (*match_cb)(struct credential *),\n> +\t\t\t\t void (*other_cb)(struct strbuf *))\n> +{\n> +\tFILE *fh;\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tstruct credential entry = CREDENTIAL_INIT;\n> +\n> +\tfh = fopen(fn, \"r\");\n> +\tif (!fh) {\n> +\t\tif (errno != ENOENT)\n> +\t\t\tdie_errno(\"unable to open %s\", fn);\n> +\t\treturn;\n> +\t}\n> +\n> +\twhile (strbuf_getline(&line, fh, '\\n') != EOF) {\n> +\t\tcredential_from_url(&entry, line.buf);\n> +\t\tif (entry.username && entry.password &&\n> +\t\t credential_match(c, &entry)) {\n\nThis looks curious; isn't checking .username and .password part of the\nresponsibility of credential_match()? And even if entry lacks password\n(which won't happen in the context of this program, given the\nimplementation of store_credential() below) shouldn't it still be\nconsidered a match?\n\n> +\t\t\tif (match_cb) {\n> +\t\t\t\tmatch_cb(&entry);\n> +\t\t\t\tbreak;\n\nMultiple matches not allowed, which sounds fine.\n\n> +\t\t\t}\n> +\t\t}\n> +\t\telse if (other_cb)\n> +\t\t\tother_cb(&line);\n> +\t}\n> +\n> +\tcredential_clear(&entry);\n> +\tstrbuf_release(&line);\n> +\tfclose(fh);\n> +}\n> +\n> +static void print_entry(struct credential *c)\n> +{\n> +\tprintf(\"username=%s\\n\", c->username);\n> +\tprintf(\"password=%s\\n\", c->password);\n> +}\n> +\n> +static void print_line(struct strbuf *buf)\n> +{\n> +\tstrbuf_addch(buf, '\\n');\n> +\twrite_or_die(credential_lock.fd, buf->buf, buf->len);\n> +}\n> +\n> +static void rewrite_credential_file(const char *fn, struct credential *c,\n> +\t\t\t\t struct strbuf *extra)\n> +{\n> +\tumask(077);\n\nCurious placement of umask(). I would expect a function that has its own\ncall to umask() restore it before it returns, and a stand-alone program\nwhose sole purpose is to work with a private file, setting a tight umask\nupfront at the beginning of main() may be easier to understand.\n\n> +\tif (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)\n> +\t\tdie_errno(\"unable to get credential storage lock\");\n> +\tparse_credential_file(fn, c, NULL, print_line);\n> +\tif (extra)\n> +\t\tprint_line(extra);\n\nAn entry for a newly updated password comes at the end of the file,\ninstead of replacing an entry already in the file in-place? Given that\nparse_credential_file() when processing a look-up request (which is the\nmajority of the case) stops upon finding a match, it might make more sense\nto have the new one (which may be expected to be used often) at the\nbeginning instead, no?\n\n> +\tif (commit_lock_file(&credential_lock) < 0)\n> +\t\tdie_errno(\"unable to commit credential store\");\n> +}\n> +\n> +static void store_credential(const char *fn, struct credential *c)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\tif (!c->protocol || !(c->host || c->path) ||\n> +\t !c->username || !c->password)\n> +\t\treturn;\n> +\n> +\tstrbuf_addf(&buf, \"%s://\", c->protocol);\n> +\tstrbuf_addstr_urlencode(&buf, c->username, 1);\n> +\tstrbuf_addch(&buf, ':');\n> +\tstrbuf_addstr_urlencode(&buf, c->password, 1);\n> +\tstrbuf_addch(&buf, '@');\n> +\tif (c->host)\n> +\t\tstrbuf_addstr_urlencode(&buf, c->host, 1);\n> +\tif (c->path) {\n> +\t\tstrbuf_addch(&buf, '/');\n> +\t\tstrbuf_addstr_urlencode(&buf, c->path, 0);\n> +\t}\n> +\n> +\trewrite_credential_file(fn, c, &buf);\n> +\tstrbuf_release(&buf);\n> +}\n> +\n> +static void remove_credential(const char *fn, struct credential *c)\n> +{\n> +\tif (!c->protocol || !(c->host || c->path))\n> +\t\treturn;\n\nThe choice of the fields looks rather arbitrary. I cannot say \"remove all\nthe credentials whose username is 'gitster' at 'github.com' no matter what\nprotocol is used\", but I can say \"remove all credentials under any name\nfor any host as long as the transfer goes over 'https' and accesses a\nrepository at 'if/xyzzy' path\", it seems.\n\nThis filtering matches what lookup_credential() does but shouldn't it be\nimplemented at a single place in any case?\n\n> +\trewrite_credential_file(fn, c, NULL);\n> +}\n> +\n> +static int lookup_credential(const char *fn, struct credential *c)\n> +{\n> +\tif (!c->protocol || !(c->host || c->path))\n> +\t\treturn 0;\n> +\tparse_credential_file(fn, c, print_entry, NULL);\n> +\treturn c->username && c->password;\n> +}\n"},{"id":"180112","messageId":"20111129211411.GC1793@sigill.intra.peff.net","threadId":"29017","inReplyTo":"7vmxbeu91d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 03/13] introduce credentials API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-29T21:14:11Z","receivedAt":"2011-11-29T21:14:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 29, 2011 at 09:34:54AM -0800, Junio C Hamano wrote:\n\n> >> > +`credential_fill`::\n> >> > +\n> >> > +\tAttempt to fill the username and password fields of the passed\n> >> > +\tcredential struct, first consulting storage helpers, then asking\n> >> > +\tthe user. Guarantees that the username and password fields will\n> >> > +\tbe filled afterwards (or die() will be called).\n\n> Immensely, at least to me. From the perspective of a user of the API, a\n> call to credential_fill() is to \"fill in the credential\" in the sense that\n> \"call the function to fill in the credential\" but I find it easier to\n> understand if it were explained to me as \"ask the API to fill in the\n> credential, which may involve helpers to interact with the user--the point\n> of the API is that the caller does not care how it is done\". Same for the\n> reject/accept calls---the example makes it clear that they are to tell the\n> decision to reject/accept made by the application to the credential API,\n> and it is up to the API layer what it does using that decision (like\n> removing the cached and now stale password).\n\nAhh, I see your confusion now. It is not that the description is\nnecessarily lacking any content, but that the tense of the sentences is\nmisleading. I'll fix that.\n\n> The above example is a bit too simplistic and misleading, though. You\n> would call reject only on authentication failure (do not trash stored and\n> good password upon network being unreachable temporarily or the server\n> being overloaded).\n\nGood point. I'll fix that and add the example to the documentation.\n\n> > So one possible rule would be:\n> >\n> > 1. If it starts with \"!\", clip off the \"!\" and hand it to the shell.\n> >\n> > 2. Otherwise, if is_absolute_path(), hand it to the shell directly.\n> >\n> > 3. Otherwise, prepend \"git credential-\" and hand it to the shell.\n> >\n> > I think that is slightly less confusing than the \"first word is alnum\"\n> > thing.\n> \n> Simpler and easier to explain. Good ;-)\n\nOK, I'll implement that, then.\n\n> > How do you feel about the \"values cannot contain a newline\" requirement?\n> \n> In the context of asking username, password, or passphrase, I think \"LF is\n> the end of the line and you cannot have that byte in your response\" is\n> perfectly reasonable. I've yet to find a way to use LF in a passphrase to\n> unlock my Gnome keychain ;-).\n\nThe potential issue is that other values get that, too. So if you have a\nURL with \"\\n\" in the path, it cannot be transmitted verbatim. We can\nurl-encode, of course, but I didn't want the helpers to have to deal\nwith quoting issues.\n\n> >> Two style nits.\n> >\n> > I'm supposed to guess? ;P\n> \n> Sorry, but you guessed right.\n\nOK, will fix.\n\n-Peff\n"},{"id":"180113","messageId":"20111129211950.GD1793@sigill.intra.peff.net","threadId":"29017","inReplyTo":"7vzkfessff.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-29T21:19:50Z","receivedAt":"2011-11-29T21:19:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 29, 2011 at 10:19:00AM -0800, Junio C Hamano wrote:\n\n> > +static int is_rfc3986_reserved(char ch)\n> > +{\n> > +\tswitch (ch) {\n> > +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n> > +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n> > +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n> > +\t\treturn 1;\n> > +\t}\n> > +\treturn 0;\n> > +}\n> \n> Part of me wonders if we still have extra bits in sane_ctype[] array but\n> that one is cumbersome to update, and the above should be easier to read\n> and maintain.\n\nWe have 2 bits left. I did consider it, but it just seemed excessively\ncumbersome for something that really doesn't need to be that fast (if it\nis indeed any faster than this case statement).\n\n> > +void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,\n> > +\t\t\t int reserved)\n> \n> Does \"reserved\" parameter mean \"must-encode-reserved\", or\n> \"may-encode-reserved\" (the latter would be more like \"if set to 0,\n> per-cent encoding the result would be an error\")?\n\nIt is \"must-encode-reserved\". The difference, from my reading of the\nrfc, is that we can relax our encoding in the path-name portion of the\nURI. For example, in:\n\n https://user@host/path/to/repo.git\n\nYou definitely want to quote \"/\" in the user or hostname, but doing so\nin path/to/repo.git is just annoying.\n\n-Peff\n"},{"id":"180115","messageId":"20111129213846.GE1793@sigill.intra.peff.net","threadId":"29017","inReplyTo":"7vsjl6ssf9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 12/13] credentials: add \"store\" helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-29T21:38:46Z","receivedAt":"2011-11-29T21:38:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 29, 2011 at 10:19:06AM -0800, Junio C Hamano wrote:\n\n> > +\twhile (strbuf_getline(&line, fh, '\\n') != EOF) {\n> > +\t\tcredential_from_url(&entry, line.buf);\n> > +\t\tif (entry.username && entry.password &&\n> > +\t\t credential_match(c, &entry)) {\n> \n> This looks curious; isn't checking .username and .password part of the\n> responsibility of credential_match()? And even if entry lacks password\n> (which won't happen in the context of this program, given the\n> implementation of store_credential() below) shouldn't it still be\n> considered a match?\n\ncredential_match will check .username, if the pattern mentions it. It\nwill never check .password. My intent here was to enforce well-formed\nentries in the credential file. So you could add:\n\n http://example.com/\n\nto the credential file, but it's just meaningless noise. It\ndoesn't actually tell us a username or password.\n\nThe helper won't add such an entry itself, but given the simplicity of\nthe format, I wanted to leave the door open for curious hackers to\npopulate it manually if they choose.\n\nI think you're right that:\n\n http://user@example.com/\n\nis potentially meaningful, and this would skip that. OTOH, you would be\nmuch better served to just do:\n\n git config credential.http://example.com.username user\n\nSo I consider it a slight abuse of this helper in the first place.\n\n> > +static void rewrite_credential_file(const char *fn, struct credential *c,\n> > +\t\t\t\t struct strbuf *extra)\n> > +{\n> > +\tumask(077);\n> \n> Curious placement of umask(). I would expect a function that has its own\n> call to umask() restore it before it returns, and a stand-alone program\n> whose sole purpose is to work with a private file, setting a tight umask\n> upfront at the beginning of main() may be easier to understand.\n\nI think that is largely a holdover from the original implementation,\nwhich set the umask and did other black magic before calling\ngit_config_set. I agree it would make more sense at the beginning of the\nprogram. Will change.\n\n> > +\tif (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)\n> > +\t\tdie_errno(\"unable to get credential storage lock\");\n> > +\tparse_credential_file(fn, c, NULL, print_line);\n> > +\tif (extra)\n> > +\t\tprint_line(extra);\n> \n> An entry for a newly updated password comes at the end of the file,\n> instead of replacing an entry already in the file in-place? Given that\n> parse_credential_file() when processing a look-up request (which is the\n> majority of the case) stops upon finding a match, it might make more sense\n> to have the new one (which may be expected to be used often) at the\n> beginning instead, no?\n\nYeah. It's a linear search. Your worst-case is always going to be O(n),\nbut I just assumed n would remain relatively small and we wouldn't care\n(if it isn't, the right solution is probably a smarter data structure).\n\nBut your optimization is trivial to implement, so it's probably worth\ndoing.\n\n> > +\tif (commit_lock_file(&credential_lock) < 0)\n> > +\t\tdie_errno(\"unable to commit credential store\");\n> > +}\n> > +\n> > +static void store_credential(const char *fn, struct credential *c)\n> > +{\n> > +\tstruct strbuf buf = STRBUF_INIT;\n> > +\n> > +\tif (!c->protocol || !(c->host || c->path) ||\n> > +\t !c->username || !c->password)\n> > +\t\treturn;\n> [...]\n> > +static void remove_credential(const char *fn, struct credential *c)\n> > +{\n> > +\tif (!c->protocol || !(c->host || c->path))\n> > +\t\treturn;\n> \n> The choice of the fields looks rather arbitrary. I cannot say \"remove all\n> the credentials whose username is 'gitster' at 'github.com' no matter what\n> protocol is used\", but I can say \"remove all credentials under any name\n> for any host as long as the transfer goes over 'https' and accesses a\n> repository at 'if/xyzzy' path\", it seems.\n\nIt is kind of arbitrary. The storage format is URLs, which is why\nstore_credential is a little pedantic. We can't store something that\ndoesn't have a protocol part, as that is a required part of the URL\n(actually, in URL-speak this is the \"scheme\"; I wonder if we should use\nthe same term here).\n\nI was thinking we need a protocol for the same reason in\nremove_credential, but I think you are right. We never actually convert\nit to a URL, so in theory you could do:\n\n git credential-store erase <<\\EOF\n username=gitster\n host=github.com\n EOF\n\nAgain, not an operation that git will ever perform, but I guess\nsomething that people might want to do (I had always assumed the\n\"$EDITOR ~/.git-credentials\" was going to be the preferred way of doing\nsuch operations :) ).\n\nI don't think there's any harm in loosening that condition.\n\n-Peff\n"},{"id":"180124","messageId":"4ED56A1C.9050800@lsrfire.ath.cx","threadId":"29017","inReplyTo":"20111129211950.GD1793@sigill.intra.peff.net","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-11-29T23:26:20Z","receivedAt":"2011-11-29T23:26:20Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.11.2011 22:19, schrieb Jeff King:\n> On Tue, Nov 29, 2011 at 10:19:00AM -0800, Junio C Hamano wrote:\n> \n>>> +static int is_rfc3986_reserved(char ch)\n>>> +{\n>>> +\tswitch (ch) {\n>>> +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n>>> +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n>>> +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n>>> +\t\treturn 1;\n>>> +\t}\n>>> +\treturn 0;\n>>> +}\n>>\n>> Part of me wonders if we still have extra bits in sane_ctype[] array but\n>> that one is cumbersome to update, and the above should be easier to read\n>> and maintain.\n> \n> We have 2 bits left. I did consider it, but it just seemed excessively\n> cumbersome for something that really doesn't need to be that fast (if it\n> is indeed any faster than this case statement).\n\nSorry for my bikeshedding, but I'd paint it like this:\n\n\treturn !!strchr(\"!*'();:@&=+$,/?#[]\", ch);\n\nRené\n"},{"id":"180127","messageId":"20111130032028.GA24704@sigill.intra.peff.net","threadId":"29017","inReplyTo":"4ED56A1C.9050800@lsrfire.ath.cx","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-30T03:20:28Z","receivedAt":"2011-11-30T03:20:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 30, 2011 at 12:26:20AM +0100, René Scharfe wrote:\n\n> >>> +static int is_rfc3986_reserved(char ch)\n> >>> +{\n> >>> +\tswitch (ch) {\n> >>> +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n> >>> +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n> >>> +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n> >>> +\t\treturn 1;\n> >>> +\t}\n> [...]\n> Sorry for my bikeshedding, but I'd paint it like this:\n> \n> \treturn !!strchr(\"!*'();:@&=+$,/?#[]\", ch);\n\nI was always under the impression that computed jumps via \"switch\" would\nout-perform even an optimized strchr. Of course, I never tested. And I\ndoubt performance is even relevant here, and I admit I don't care overly\nmuch. I find them both equally readable.\n\nI'm going to leave it as-is unless somebody else wants to say \"I\nstrongly prefer version X\".\n\n-Peff\n"},{"id":"180129","messageId":"7v4nxmrwvq.fsf@alter.siamese.dyndns.org","threadId":"29017","inReplyTo":"20111130032028.GA24704@sigill.intra.peff.net","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-30T05:40:25Z","receivedAt":"2011-11-30T05:40:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 30, 2011 at 12:26:20AM +0100, René Scharfe wrote:\n>\n>> >>> +static int is_rfc3986_reserved(char ch)\n>> >>> +{\n>> >>> +\tswitch (ch) {\n>> >>> +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n>> >>> +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n>> >>> +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n>> >>> +\t\treturn 1;\n>> >>> +\t}\n>> [...]\n>> Sorry for my bikeshedding, but I'd paint it like this:\n>> \n>> \treturn !!strchr(\"!*'();:@&=+$,/?#[]\", ch);\n>\n> I was always under the impression that computed jumps via \"switch\" would\n> out-perform even an optimized strchr. Of course, I never tested. And I\n> doubt performance is even relevant here, and I admit I don't care overly\n> much. I find them both equally readable.\n>\n> I'm going to leave it as-is unless somebody else wants to say \"I\n> strongly prefer version X\".\n\nI find the switch/case one much easier to read and count, especially since\nall the choices are essentially line-noise characters.\n\nJust make sure you indent it correctly ;-)\n"},{"id":"180130","messageId":"4ED5C211.6040401@lsrfire.ath.cx","threadId":"29017","inReplyTo":"20111130032028.GA24704@sigill.intra.peff.net","subject":"Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-11-30T05:41:37Z","receivedAt":"2011-11-30T05:41:37Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.11.2011 04:20, schrieb Jeff King:\n> On Wed, Nov 30, 2011 at 12:26:20AM +0100, René Scharfe wrote:\n> \n>>>>> +static int is_rfc3986_reserved(char ch)\n>>>>> +{\n>>>>> +\tswitch (ch) {\n>>>>> +\tcase '!': case '*': case '\\'': case '(': case ')': case ';':\n>>>>> +\tcase ':': case '@': case '&': case '=': case '+': case '$':\n>>>>> +\tcase ',': case '/': case '?': case '#': case '[': case ']':\n>>>>> +\t\treturn 1;\n>>>>> +\t}\n>> [...]\n>> Sorry for my bikeshedding, but I'd paint it like this:\n>>\n>> \treturn !!strchr(\"!*'();:@&=+$,/?#[]\", ch);\n> \n> I was always under the impression that computed jumps via \"switch\" would\n> out-perform even an optimized strchr. Of course, I never tested. And I\n> doubt performance is even relevant here, and I admit I don't care overly\n> much. I find them both equally readable.\n> \n> I'm going to leave it as-is unless somebody else wants to say \"I\n> strongly prefer version X\".\n\nSure, the second one is significantly slower than the first one. I just\nprefer it based one its looks in case performance doesn't matter, but\nthat's probably just me being (sometimes too) fond of terseness. :)\n\nRené\n"}]}