{"thread":{"id":"30000","subject":"[PATCH/RFC] contrib: add win32 credential-helper","startedAt":"2012-03-19T23:06:54Z","lastAt":"2012-04-04T17:44:22Z","messageCount":8,"participants":["Erik Faye-Lund","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"187294","messageId":"1332198414-5884-1-git-send-email-kusmabite@gmail.com","threadId":"30000","inReplyTo":null,"subject":"[PATCH/RFC] contrib: add win32 credential-helper","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-03-19T23:06:54Z","receivedAt":"2012-03-19T23:06:54Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"This one pretty much sucks. Mem-leaks and a sketchy deletion-filter.\n\nCurrently uses \"::\" as an attribute-separator, but this is not robust\nwithout encoding if the attribute values themselves contains \"::\".\n\nSince the Windows port of Git expects binary pipes, we need to make\nsure the helper-end also sets up binary pipes.\n\nSide-step CRLF-issue in test to make it pass.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n\nHere's a hacky and not-quite-as-cool-as-I'd-hope credential-helper\nfor Windows. The good news is that it Works For Me(tm), but the bad\nnews is that it sucks.\n\nThe code is provided for discussion only so far.\n\nI'm not really sure how to make it less sucky in some regards, part\nof this I blame on lacking documentation of the credential-helper\nprococol :P\n\nHere's some things I can't find documented:\n\n 1) Encoding of usernames. I'm assuming this is supposed to be\n UTF-8, because SecKeychainFindInternetPassword which is used by\n the OSX-helper is documented to take accountName as UTF-8.\n\n 2) Encoding of passwords. I'm assuming UTF-8, as mixing encodings\n here would be insane :P\n\n 3) How to match credentials for look-up when not supplying all\n attributes. I've implemented a kind-of dodgy matching; I use a\n string that packs all supplied attributes. This seems to work,\n but I'm a bit worried about what happens when people try to mix\n credentials saved with credential.useHttpPath=true with those\n saved with credential.useHttpPath=false.\n\n 4) How to match credentials for deletion when not supplying\n all attributes. It seems from the test-suite that a credential is\n expected to be deleted even if the username is not given. But what\n happens in such a case when there's two different credentials for\n one domain, with different username? The OSX-helper seems to only\n delete one of them (but I'm not entirely sure which one, due to\n lacking documentation from Apple's side).\n \n The OSX-helper also have a comment saying \"Require at least a\n protocol and host for removal, which is what git will give us; if\n you want to do  something more fancy, use the Keychain manager\".\n This comment puzzles me a bit; won't git also give us path in case\n of credential.useHttpPath=true?\n\n 5) How overwriting credentials work. If there's a credential where\n all attributes except for the password match, will \"store\" just\n overwrite it? Should it complain?\n\nI'm guessing that a lot of these questions fall in the \"who cares\"\ncategory. For instance, git probably never tries to create a\ncredential it already has stored. But at least saying \"This cannot\nhappen\" in the documentation makes it a lot easier to write a\ncredential-helper.\n\nThoughts?\n\n contrib/credential/wincred/Makefile                |    8 +\n .../credential/wincred/git-credential-wincred.c    |  320 ++++++++++++++++++++\n t/lib-credential.sh                                |    4 +\n 3 files changed, 332 insertions(+), 0 deletions(-)\n create mode 100644 contrib/credential/wincred/Makefile\n create mode 100644 contrib/credential/wincred/git-credential-wincred.c\n\ndiff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\nnew file mode 100644\nindex 0000000..b4f098f\n--- /dev/null\n+++ b/contrib/credential/wincred/Makefile\n@@ -0,0 +1,8 @@\n+all: git-credential-wincred.exe\n+\n+CC = gcc\n+RM = rm -f\n+CFLAGS = -O2 -Wall\n+\n+git-credential-wincred.exe : git-credential-wincred.c\n+\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nnew file mode 100644\nindex 0000000..760a3dc\n--- /dev/null\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -0,0 +1,320 @@\n+/*\n+ * A git credential helper that interface with Windows' Credential Manager\n+ *\n+ */\n+#include <windows.h>\n+#include <stdio.h>\n+#include <io.h>\n+#include <fcntl.h>\n+\n+/* common helpers */\n+\n+static void die(const char *err, ...)\n+{\n+\tchar msg[4096];\n+\tva_list params;\n+\tva_start(params, err);\n+\tvsnprintf(msg, sizeof(msg), err, params);\n+\tfprintf(stderr, \"%s\\n\", msg);\n+\tva_end(params);\n+\texit(1);\n+}\n+\n+static void *xmalloc(size_t size)\n+{\n+\tvoid *ret = malloc(size);\n+\tif (!ret && !size)\n+\t\tret = malloc(1);\n+\tif (!ret)\n+\t\t die(\"Out of memory\");\n+\treturn ret;\n+}\n+\n+static char *xstrdup(const char *str)\n+{\n+\tchar *ret = strdup(str);\n+\tif (!ret)\n+\t\tdie(\"Out of memory\");\n+\treturn ret;\n+}\n+\n+/* MinGW doesn't have wincred.h, so we need to define stuff */\n+\n+typedef struct _CREDENTIAL_ATTRIBUTE {\n+\tLPWSTR Keyword;\n+\tDWORD  Flags;\n+\tDWORD  ValueSize;\n+\tLPBYTE Value;\n+} CREDENTIAL_ATTRIBUTE, *PCREDENTIAL_ATTRIBUTE;\n+\n+typedef struct _CREDENTIALW {\n+\tDWORD                 Flags;\n+\tDWORD                 Type;\n+\tLPWSTR                TargetName;\n+\tLPWSTR                Comment;\n+\tFILETIME              LastWritten;\n+\tDWORD                 CredentialBlobSize;\n+\tLPBYTE                CredentialBlob;\n+\tDWORD                 Persist;\n+\tDWORD                 AttributeCount;\n+\tPCREDENTIAL_ATTRIBUTE Attributes;\n+\tLPWSTR                TargetAlias;\n+\tLPWSTR                UserName;\n+} CREDENTIALW, *PCREDENTIALW;\n+\n+#define CRED_TYPE_GENERIC 1\n+#define CRED_PERSIST_LOCAL_MACHINE 2\n+#define CRED_PACK_GENERIC_CREDENTIALS 4\n+\n+typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);\n+typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,\n+    LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);\n+typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,\n+    PCREDENTIALW **);\n+typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,\n+    PBYTE, DWORD *);\n+typedef VOID (WINAPI *CredFreeT)(PVOID);\n+typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);\n+\n+static HMODULE advapi, credui;\n+static CredWriteWT CredWriteW;\n+static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;\n+static CredEnumerateWT CredEnumerateW;\n+static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;\n+static CredFreeT CredFree;\n+static CredDeleteWT CredDeleteW;\n+\n+static void load_cred_funcs(void)\n+{\n+\t/* load DLLs */\n+\tadvapi = LoadLibrary(\"advapi32.dll\");\n+\tcredui = LoadLibrary(\"credui.dll\");\n+\tif (!advapi || !credui)\n+\t\tdie(\"failed to load DLLs\");\n+\n+\t/* get function pointers */\n+\tCredWriteW = (CredWriteWT)GetProcAddress(advapi, \"CredWriteW\");\n+\tCredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT)\n+\t    GetProcAddress(credui, \"CredUnPackAuthenticationBufferW\");\n+\tCredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi,\n+\t    \"CredEnumerateW\");\n+\tCredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT)\n+\t    GetProcAddress(credui, \"CredPackAuthenticationBufferW\");\n+\tCredFree = (CredFreeT)GetProcAddress(advapi, \"CredFree\");\n+\tCredDeleteW = (CredDeleteWT)GetProcAddress(advapi, \"CredDeleteW\");\n+\tif (!CredWriteW || !CredUnPackAuthenticationBufferW ||\n+\t    !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||\n+\t    !CredDeleteW)\n+\t\tdie(\"failed to load functions\");\n+}\n+\n+static char target_buf[1024];\n+static char *protocol, *host, *path, *username;\n+static WCHAR *wusername, *password, *target;\n+\n+static void write_item(const char *what, WCHAR *wbuf)\n+{\n+\tchar *buf;\n+\tint len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,\n+\t    FALSE);\n+\tbuf = xmalloc(len);\n+\n+\tif (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))\n+\t\tdie(\"WideCharToMultiByte failed!\");\n+\n+\tprintf(\"%s=\", what);\n+\tfwrite(buf, 1, len - 1, stdout);\n+\tputchar('\\n');\n+\tfree(buf);\n+}\n+\n+static void get_credential(void)\n+{\n+\tWCHAR *user_buf, *pass_buf;\n+\tDWORD user_buf_size = 0, pass_buf_size = 0;\n+\tCREDENTIALW **creds, *cred;\n+\tDWORD num_creds;\n+\tWCHAR temp[4096];\n+\n+\twcscpy(temp, target);\n+\twcscat(temp, L\"*\");\n+\n+\tif (!CredEnumerateW(temp, 0, &num_creds, &creds))\n+\t\treturn;\n+\n+\tif (!wusername) {\n+\t\t/* no username was specified, just pick the first one */\n+\t\tcred = creds[0];\n+\t} else {\n+\t\t/* search for the first credential that matches username */\n+\t\tint i;\n+\t\tcred = NULL;\n+\t\tfor (i = 0; i < num_creds; ++i)\n+\t\t\tif (!wcscmp(wusername, creds[i]->UserName)) {\n+\t\t\t\tcred = creds[i];\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\tif (!cred)\n+\t\t\treturn;\n+\t}\n+\n+\tCredUnPackAuthenticationBufferW(0, cred->CredentialBlob,\n+\t    cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,\n+\t    NULL, &pass_buf_size);\n+\n+\tuser_buf = xmalloc(user_buf_size * sizeof(WCHAR));\n+\tpass_buf = xmalloc(pass_buf_size * sizeof(WCHAR));\n+\n+\tif (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,\n+\t    cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,\n+\t    pass_buf, &pass_buf_size))\n+\t\tdie(\"CredUnPackAuthenticationBuffer failed\");\n+\n+\tCredFree(creds);\n+\n+\t/* zero-terminate (sizes include zero-termination) */\n+\tuser_buf[user_buf_size - 1] = L'\\0';\n+\tpass_buf[pass_buf_size - 1] = L'\\0';\n+\n+\twrite_item(\"username\", user_buf);\n+\twrite_item(\"password\", pass_buf);\n+\n+\tfree(user_buf);\n+\tfree(pass_buf);\n+}\n+\n+static void store_credential(void)\n+{\n+\tCREDENTIALW cred;\n+\tBYTE *auth_buf;\n+\tDWORD auth_buf_size = 0;\n+\n+\tif (!wusername || !password)\n+\t\treturn;\n+\n+\t/* query buffer size */\n+\tCredPackAuthenticationBufferW(0, wusername, password,\n+\t    NULL, &auth_buf_size);\n+\n+\tauth_buf = xmalloc(auth_buf_size);\n+\n+\tif (!CredPackAuthenticationBufferW(0, wusername, password,\n+\t    auth_buf, &auth_buf_size))\n+\t\tdie(\"CredPackAuthenticationBuffer failed\");\n+\n+\tcred.Flags = 0;\n+\tcred.Type = CRED_TYPE_GENERIC;\n+\tcred.TargetName = target;\n+\tcred.Comment = L\"saved by git-credential-wincred\";\n+\tcred.CredentialBlobSize = auth_buf_size;\n+\tcred.CredentialBlob = auth_buf;\n+\tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n+\tcred.AttributeCount = 0;\n+\tcred.Attributes = NULL;\n+\tcred.TargetAlias = NULL;\n+\tcred.UserName = wusername;\n+\tif (!CredWriteW(&cred, 0))\n+\t\tdie(\"CredWrite failed\");\n+}\n+\n+static void erase_credential(void)\n+{\n+\tWCHAR temp[4096];\n+\tCREDENTIALW **creds;\n+\tDWORD num_creds;\n+\tint i;\n+\n+\twcscpy(temp, target);\n+\twcscat(temp, L\"*\");\n+\n+\tif (!CredEnumerateW(temp, 0, &num_creds, &creds))\n+\t\treturn;\n+\n+\tfor (i = 0; i < num_creds; ++i)\n+\t\tCredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);\n+\n+\tCredFree(creds);\n+}\n+\n+static WCHAR *utf8_to_utf16_dup(const char *str)\n+{\n+\tint wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);\n+\tWCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);\n+\tMultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen);\n+\treturn wstr;\n+}\n+\n+static void read_credential(void)\n+{\n+\tchar buf[1024];\n+\n+\twhile (fgets(buf, sizeof(buf), stdin)) {\n+\t\tchar *v;\n+\n+\t\tif (!strcmp(buf, \"\\n\"))\n+\t\t\tbreak;\n+\t\tbuf[strlen(buf)-1] = '\\0';\n+\n+\t\tv = strchr(buf, '=');\n+\t\tif (!v)\n+\t\t\tdie(\"bad input: %s\", buf);\n+\t\t*v++ = '\\0';\n+\n+\t\tif (!strcmp(buf, \"protocol\"))\n+\t\t\tprotocol = xstrdup(v);\n+\t\telse if (!strcmp(buf, \"host\"))\n+\t\t\thost = xstrdup(v);\n+\t\telse if (!strcmp(buf, \"path\"))\n+\t\t\tpath = xstrdup(v);\n+\t\telse if (!strcmp(buf, \"username\")) {\n+\t\t\tusername = xstrdup(v);\n+\t\t\twusername = utf8_to_utf16_dup(v);\n+\t\t} else if (!strcmp(buf, \"password\"))\n+\t\t\tpassword = utf8_to_utf16_dup(v);\n+\t}\n+}\n+\n+int main(int argc, char *argv[])\n+{\n+\tconst char *usage =\n+\t    \"Usage: git credential-wincred <get|store|erase>\\n\";\n+\n+\tif (!argv[1])\n+\t\tdie(usage);\n+\n+\t/* git use binary pipes to avoid CRLF-issues */\n+\t_setmode(_fileno(stdin), _O_BINARY);\n+\t_setmode(_fileno(stdout), _O_BINARY);\n+\n+\tread_credential();\n+\n+\tload_cred_funcs();\n+\n+\t/* prepare 'target', the unique key for the credential */\n+\tif (!protocol || !host)\n+\t\treturn 0;\n+\n+\tstrncpy(target_buf, \"git::protocol=\", sizeof(target_buf));\n+\tstrncat(target_buf, protocol, sizeof(target_buf));\n+\tstrncat(target_buf, \"::host=\", sizeof(target_buf));\n+\tstrncat(target_buf, host, sizeof(target_buf));\n+\tif (username) {\n+\t\tstrncat(target_buf, \"::user=\", sizeof(target_buf));\n+\t\tstrncat(target_buf, username, sizeof(target_buf));\n+\t}\n+\tif (path) {\n+\t\tstrncat(target_buf, \"::path=\", sizeof(target_buf));\n+\t\tstrncat(target_buf, host, sizeof(target_buf));\n+\t}\n+\n+\ttarget = utf8_to_utf16_dup(target_buf);\n+\n+\tif (!strcmp(argv[1], \"get\"))\n+\t\tget_credential();\n+\telse if (!strcmp(argv[1], \"store\"))\n+\t\tstore_credential();\n+\telse if (!strcmp(argv[1], \"erase\"))\n+\t\terase_credential();\n+\t/* otherwise, ignore unknown action */\n+\treturn 0;\n+}\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 4a37cd7..d30ae8a 100755\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -8,6 +8,10 @@ check() {\n \tread_chunk >expect-stdout &&\n \tread_chunk >expect-stderr &&\n \ttest-credential \"$@\" <stdin >stdout 2>stderr &&\n+\tif test_have_prereq MINGW\n+\tthen\n+\t\tdos2unix -q stderr\n+\tfi &&\n \ttest_cmp expect-stdout stdout &&\n \ttest_cmp expect-stderr stderr\n }\n-- \n1.7.9\n"},{"id":"187611","messageId":"20120323211001.GA18198@sigill.intra.peff.net","threadId":"30000","inReplyTo":"1332198414-5884-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-23T21:10:01Z","receivedAt":"2012-03-23T21:10:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 20, 2012 at 12:06:54AM +0100, Erik Faye-Lund wrote:\n\n> This one pretty much sucks. Mem-leaks and a sketchy deletion-filter.\n\nThanks for moving forward on this. I'm sorry I can't be much help on the\nWindows-specific knowledge, but I'll answer what I can.\n\n> Currently uses \"::\" as an attribute-separator, but this is not robust\n> without encoding if the attribute values themselves contains \"::\".\n\nYeah. Can you store arbitrary bytes? If so, NUL would be a good\nterminator. Otherwise, newline is a reasonable choice, as the protocol\nalready can't communicate usernames/passwords with newlines (a\nlimitation that I accepted to make the protocol much simpler for\nscripting use).\n\nThat being said, I realized when writing credential-store that you can\nencode all of the components in a URL, like:\n\n  proto://user:password@host/path\n\nYou do have to get the URL encoding/decoding right, of course, but since\nit is a standard format, you may have library or OS support (for the\nstock helpers, I was able to cheat and just link against git's code).\n\n> I'm not really sure how to make it less sucky in some regards, part\n> of this I blame on lacking documentation of the credential-helper\n> prococol :P\n\nHeh. OK, I'll take the blame. :)\n\n>  1) Encoding of usernames. I'm assuming this is supposed to be\n>  UTF-8, because SecKeychainFindInternetPassword which is used by\n>  the OSX-helper is documented to take accountName as UTF-8.\n\nLike many other parts of git, we treat the data as binary goo as much as\npossible. So git hands the helper whatever bytes the user provided, and\nships off whatever bytes are provided by the helper over http without\nany further processing. The only two exceptions are:\n\n  1. Fields cannot contain NUL (which means wide encodings like utf-16\n     are pretty much out).\n\n  2. Fields cannot contain newline (which effectively means that the\n     encoding needs to be some ascii superset like utf8 or latin1).\n\nIn practice, I would expect most usernames and passwords only contain\nascii bytes, if only because charset issues between the client and\nserver would lead to insanity.\n\n>  2) Encoding of passwords. I'm assuming UTF-8, as mixing encodings\n>  here would be insane :P\n\nSame as above.\n\n>  3) How to match credentials for look-up when not supplying all\n>  attributes. I've implemented a kind-of dodgy matching; I use a\n>  string that packs all supplied attributes. This seems to work,\n>  but I'm a bit worried about what happens when people try to mix\n>  credentials saved with credential.useHttpPath=true with those\n>  saved with credential.useHttpPath=false.\n\nSee how credential_match in credential.c does it. For a get, the input\nto the helper is basically a template saying \"this is what I want to\nmatch\". If you have more information in your database than is in the\ntemplate, that's OK; anything the template doesn't specify should not be\nconsidered a restriction.\n\nSo if you get:\n\n  protocol=http\n  host=example.com\n\nyou should match an entry previously stored for:\n\n  protocol=http\n  host=example.com\n  path=foo.git\n\nbecause what is being asked is a more generic form (if you had two\nentries, one for foo.git and one for bar.git, it's unclear which should\nbe chosen; my implementations just pick the first match they encounter).\n\nThe same thing happens for usernames. You might get asked for a\ncredential for a specific username, or you might get asked for a\ncredential of any username at that host. It's nice in the latter case to\nbe able to respond with a previously-stored credential from the former.\n\nBut note that none of this advanced matching is strictly necessary. It's\npurely a quality-of-implementation issue in the helper, and that's why I\nleft the details out of the documentation (also, I hoped many helpers\ncould just rely on OS-provided libraries to do the heavy lifting of the\nmatch; that's how it is for OS X, but it seems Windows doesn't have\nanything equivalent).\n\nSo I'm just describing here what git does for credential-store,\ncredential-cache, and for the internal config matching. You could do a\nlot less in the helper. For example, I keep my passwords in a password\nwallet that is backed by GPG-encrypted storage, and my config looks\nlike:\n\n  [credential \"https://github.com\"]\n    username = peff\n    helper = \"!f() { test $1 = get && echo password=`pass -n github.web.password`; }; f\"\n  [credential]\n    helper = cache\n\nSo note that the helper is totally dumb and relies on the internal\nconfig-matching to restrict when it gets used.\n\nLikewise, you could be a lot smarter. You could conflate http and https\nprotocol entries according to some internal policy. You could\nprefix-match paths so that a stored credential for\n\"http://example.com/foo\" would be applied to a request for\n\"http://example.com/foo/bar.git\". In my helpers, I drew the line where I\nthought it was reasonable, but there's no reason other helpers have to\nfollow the exact same rules.\n\n>  4) How to match credentials for deletion when not supplying\n>  all attributes. It seems from the test-suite that a credential is\n>  expected to be deleted even if the username is not given.\n\nYeah, that's what I would expect. The input is a template, so delete\nanything that matches the template.\n\n> But what happens in such a case when there's two different credentials\n> for one domain, with different username? The OSX-helper seems to only\n> delete one of them (but I'm not entirely sure which one, due to\n> lacking documentation from Apple's side).\n\nI'd expect them both to be deleted if no username is given (and I'm\npretty sure that's what the stock helpers will do). It may be that the\nOS X helper does not (I don't recall testing it explicitly). You could\nconsider it a bug, or merely a different design decision.\n\nWhen git deletes a credential, it should provide the username (because\nit only does so after trying to use the credential and getting a\nrejection). Again, I intentionally left corner cases like this\nunspecified, because I didn't want to force helpers to behave one way or\nthe other (especially when git itself doesn't care either way). I hoped\nthat helper writers would then be free to do whatever behavior made\nsense to them and to their implementation.  But from the writer's\nperspective, I guess it looks like the behavior is simply\nunder-specified. :)\n\n>  The OSX-helper also have a comment saying \"Require at least a\n>  protocol and host for removal, which is what git will give us; if\n>  you want to do  something more fancy, use the Keychain manager\".\n>  This comment puzzles me a bit; won't git also give us path in case\n>  of credential.useHttpPath=true?\n\nYes; git will provide _at least_ the protocol and host. That code and\ncomment came out of discussion with Junio, where we realized that an\n\"empty\" template would cause everything to be deleted. So doing this:\n\n  git credential-foo erase </dev/null\n\nwould delete everything! Which maybe is something you would want to do,\nbut it seemed too easy to accidentally trigger this destructive\nbehavior. So I require some input as a sanity-check, and if you want to\ndo something like \"delete all entries\", you will have to use the\nseparate keychain manager program.\n\nDoes that make more sense?\n\n>  5) How overwriting credentials work. If there's a credential where\n>  all attributes except for the password match, will \"store\" just\n>  overwrite it? Should it complain?\n\nIn the stock implementations, we will overwrite. I don't think it\nmatters to git, though. If the helper gives us a credential that works,\nwe will not bother parroting it back[1]. If we get one that does not\nwork, we will erase it.\n\n[1] That was my intent, anyway. But I think it may parrot it back,\n    anyway. We should probably set the \"approved\" flag on a credential\n    given to us by a helper, as there is not much point in asking them\n    to store it again afterwards.\n\n> I'm guessing that a lot of these questions fall in the \"who cares\"\n> category. For instance, git probably never tries to create a\n> credential it already has stored. But at least saying \"This cannot\n> happen\" in the documentation makes it a lot easier to write a\n> credential-helper.\n\nThe problem is that it is not \"this cannot happen\", but rather \"git does\nnot currently do this\". I wanted to specify as little as possible so\nthat future versions could change those corner cases if they wanted to\n(or somebody could write a more advanced tool to interact with the\nhelpers).\n\nBut maybe we should be more explicit about that in the api-credential\ndocument.\n\n-Peff\n"},{"id":"188315","messageId":"CABPQNSac0opcTDVBwr4VeuA7pxqbtsqU7mdmYWms9tu3MjcpeQ@mail.gmail.com","threadId":"30000","inReplyTo":"20120323211001.GA18198@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-04-02T15:53:47Z","receivedAt":"2012-04-02T15:53:47Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Mar 23, 2012 at 10:10 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Mar 20, 2012 at 12:06:54AM +0100, Erik Faye-Lund wrote:\n>\n>> This one pretty much sucks. Mem-leaks and a sketchy deletion-filter.\n>\n> Thanks for moving forward on this. I'm sorry I can't be much help on the\n> Windows-specific knowledge, but I'll answer what I can.\n>\n\nThanks for following up :)\n\n>> Currently uses \"::\" as an attribute-separator, but this is not robust\n>> without encoding if the attribute values themselves contains \"::\".\n>\n> Yeah. Can you store arbitrary bytes? If so, NUL would be a good\n> terminator.\n\nNo. TargetName is an LPWSTR, which is supposed to be zero-terminated.\nThere's no way of specifying it's length directly.\n\n> Otherwise, newline is a reasonable choice, as the protocol\n> already can't communicate usernames/passwords with newlines (a\n> limitation that I accepted to make the protocol much simpler for\n> scripting use).\n>\n\nThis works, but it causes Windows 7's credential manager to glitch in\nrendering the credential (adding all the newlines to the end of the\nline, and stretching an icon - yuck), which is also a bit unfortunate.\n\nSo I'm thinking that escaping the string needs to be done. It can't be\nthat big of a deal ;)\n\n> That being said, I realized when writing credential-store that you can\n> encode all of the components in a URL, like:\n>\n>  proto://user:password@host/path\n>\n> You do have to get the URL encoding/decoding right, of course, but since\n> it is a standard format, you may have library or OS support (for the\n> stock helpers, I was able to cheat and just link against git's code).\n>\n\nYeah, I guess UrlCanonicalize from WinAPI or something along those\nlines can be used.\n\nSo far I've simply tried to put the stuff in an order that made simple\nstring-compare sufficient to pass the test. It seems I need to\nimplement proper matching instead.\n\n>> I'm not really sure how to make it less sucky in some regards, part\n>> of this I blame on lacking documentation of the credential-helper\n>> prococol :P\n>\n> Heh. OK, I'll take the blame. :)\n>\n>>  1) Encoding of usernames. I'm assuming this is supposed to be\n>>  UTF-8, because SecKeychainFindInternetPassword which is used by\n>>  the OSX-helper is documented to take accountName as UTF-8.\n>\n> Like many other parts of git, we treat the data as binary goo as much as\n> possible. So git hands the helper whatever bytes the user provided, and\n> ships off whatever bytes are provided by the helper over http without\n> any further processing. The only two exceptions are:\n>\n>  1. Fields cannot contain NUL (which means wide encodings like utf-16\n>     are pretty much out).\n>\n>  2. Fields cannot contain newline (which effectively means that the\n>     encoding needs to be some ascii superset like utf8 or latin1).\n>\n> In practice, I would expect most usernames and passwords only contain\n> ascii bytes, if only because charset issues between the client and\n> server would lead to insanity.\n>\n\nASCII unsernames might be common in the UNIX-world, but in the Windows\nworld this is very much not the case. These functions can be used for\nall kinds of services, so I don't think assuming ASCII makes much\nsense.\n\nAnd since OSX documents the encoding, I'm guessing that non-ASCII\nusernames isn't entirely unheard of there either.\n\nIn general, I think the whole \"let's try to get away with not\nspecifying encoding\" is a bit dangerous. Without knowing what encoding\nthe input and output is, the helpers are pretty much useless for\npeople. Sure, saying \"at least ASCII\" helps, but it just takes us\nhalfway there. And, I think UTF-8 is the least insane option here.\nAfter all, this is an internal protocol; if the credential helper\nneeds to store something else (like we do for Windows), we can\nconvert the string. Likewise, if the network protocol the caller is\ngoing to pass this to assumes something else, convert.\n\n>>  2) Encoding of passwords. I'm assuming UTF-8, as mixing encodings\n>>  here would be insane :P\n>\n> Same as above.\n>\n\nAgain, my objection is pretty much the same as above, modulo the\ncomment about OSX.\n\nBinary blob password doesn't make sense; human beings have\ntext-strings as passwords. They don't remember binary blobs.\n\nAbout the rest of my questions: I think what you're saying pretty much\nmakes sense. I think some details could be mentioned in\nDocumentation/technical/api-credentials.txt, and I'll have a look at\nadding what I think makes sense after reading through your mail a bit\ncloser.\n\nLuckily, I'll have some coding time this Easter, so hopefully I'll be\nable to finish up a new version soon-ish.\n"},{"id":"188395","messageId":"CABPQNSZzboU+p9UbRBt19WhhxwbwCLDRnGUyDwY_myH2kXbOag@mail.gmail.com","threadId":"30000","inReplyTo":"CABPQNSac0opcTDVBwr4VeuA7pxqbtsqU7mdmYWms9tu3MjcpeQ@mail.gmail.com","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-04-03T08:44:16Z","receivedAt":"2012-04-03T08:44:16Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Apr 2, 2012 at 5:53 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Fri, Mar 23, 2012 at 10:10 PM, Jeff King <peff@peff.net> wrote:\n>> On Tue, Mar 20, 2012 at 12:06:54AM +0100, Erik Faye-Lund wrote:\n>>\n>>> This one pretty much sucks. Mem-leaks and a sketchy deletion-filter.\n>>\n>> Thanks for moving forward on this. I'm sorry I can't be much help on the\n>> Windows-specific knowledge, but I'll answer what I can.\n>>\n>\n> Thanks for following up :)\n>\n>>> Currently uses \"::\" as an attribute-separator, but this is not robust\n>>> without encoding if the attribute values themselves contains \"::\".\n>>\n>> Yeah. Can you store arbitrary bytes? If so, NUL would be a good\n>> terminator.\n>\n> No. TargetName is an LPWSTR, which is supposed to be zero-terminated.\n> There's no way of specifying it's length directly.\n>\n>> Otherwise, newline is a reasonable choice, as the protocol\n>> already can't communicate usernames/passwords with newlines (a\n>> limitation that I accepted to make the protocol much simpler for\n>> scripting use).\n>>\n>\n> This works, but it causes Windows 7's credential manager to glitch in\n> rendering the credential (adding all the newlines to the end of the\n> line, and stretching an icon - yuck), which is also a bit unfortunate.\n>\n> So I'm thinking that escaping the string needs to be done. It can't be\n> that big of a deal ;)\n>\n\nHmm, but I must have been a complete moron; the credential structure\nin windows has a general purpose credential-attribute system, where I\ncan store the meta-data I want.\n\nSo I can probably use that and get away without any sort of encoding. Yay.\n\nI'll look deeper into it when I find the time.\n"},{"id":"188480","messageId":"20120404101014.GB13870@sigill.intra.peff.net","threadId":"30000","inReplyTo":"CABPQNSac0opcTDVBwr4VeuA7pxqbtsqU7mdmYWms9tu3MjcpeQ@mail.gmail.com","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-04T10:10:14Z","receivedAt":"2012-04-04T10:10:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 02, 2012 at 05:53:47PM +0200, Erik Faye-Lund wrote:\n\n> > Otherwise, newline is a reasonable choice, as the protocol\n> > already can't communicate usernames/passwords with newlines (a\n> > limitation that I accepted to make the protocol much simpler for\n> > scripting use).\n> \n> This works, but it causes Windows 7's credential manager to glitch in\n> rendering the credential (adding all the newlines to the end of the\n> line, and stretching an icon - yuck), which is also a bit unfortunate.\n>\n> So I'm thinking that escaping the string needs to be done. It can't be\n> that big of a deal ;)\n\nGross. I guess backslash-escaping or something similar would be pretty\neasy to implement.\n\n> > Like many other parts of git, we treat the data as binary goo as much as\n> > possible. So git hands the helper whatever bytes the user provided, and\n> > ships off whatever bytes are provided by the helper over http without\n> > any further processing. The only two exceptions are:\n> [...]\n> \n> ASCII unsernames might be common in the UNIX-world, but in the Windows\n> world this is very much not the case. These functions can be used for\n> all kinds of services, so I don't think assuming ASCII makes much\n> sense.\n> \n> And since OSX documents the encoding, I'm guessing that non-ASCII\n> usernames isn't entirely unheard of there either.\n\nBut what does it mean to have an encoding for credentials that will be\nsent over http basic-auth? As far as I know, the binary bytes are simply\nbase64-encoded and sent to the server in a header, and there is no room\nfor an encoding or any normalization in the password. You just hope that\nthe remote side is using the same encoding as you, and that there are no\nnormalization issues (e.g., your username contains \"e with accent\", but\nyour input system provides \"e\" + \"combining accent\").\n\nIt looks like there is a draft rfc to try to help with this:\n\n  http://tools.ietf.org/html/draft-reschke-basicauth-enc-05\n\nbut I have no idea if it is adopted at all yet.\n\n> In general, I think the whole \"let's try to get away with not\n> specifying encoding\" is a bit dangerous. Without knowing what encoding\n> the input and output is, the helpers are pretty much useless for\n> people.\n\nNot necessarily. By being a pass-through, things will usually just work.\nThat is, if you hand the helper UTF-8 to store, then it should hand you\nback UTF-8. Ditto for any other encoding. And if it doesn't work, it's\nnot the helper's problem, but a mismatch between the http client and\nserver. The helper's job is to reproduce some bytes.\n\n> Sure, saying \"at least ASCII\" helps, but it just takes us\n> halfway there. And, I think UTF-8 is the least insane option here.\n\nOf course. UTF-8 is pretty much always the least insane option, and I\nwould hope that everybody is using it. But by being a pass-through, we\ndon't restrict it. If the client and server both want to use Shift JIS,\nthen the stock git helpers will work fine. On systems where the OS\nmandates a particular encoding (I didn't even check for the OS X helper,\nbut let us imagine it will barf on non-UTF8 data), then I would expect\neverybody to be using that encoding, and the pass-through just works.\n\nIf your helper storage really cares, then I think it's sane to assume\nyou'll get UTF-8 input, and to pass that back out. But I really didn't\nwant to open the can of worms that is having \"getpass()\" figure out what\nencoding it is getting from the user.\n\n-Peff\n"},{"id":"188481","messageId":"CABPQNSYRxZHSW_urpH7yh=Bct2-X2TAsd3=jaJF_9Rb-foegLA@mail.gmail.com","threadId":"30000","inReplyTo":"20120404101014.GB13870@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-04-04T10:37:25Z","receivedAt":"2012-04-04T10:37:25Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Apr 4, 2012 at 12:10 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Apr 02, 2012 at 05:53:47PM +0200, Erik Faye-Lund wrote:\n>\n>> > Otherwise, newline is a reasonable choice, as the protocol\n>> > already can't communicate usernames/passwords with newlines (a\n>> > limitation that I accepted to make the protocol much simpler for\n>> > scripting use).\n>>\n>> This works, but it causes Windows 7's credential manager to glitch in\n>> rendering the credential (adding all the newlines to the end of the\n>> line, and stretching an icon - yuck), which is also a bit unfortunate.\n>>\n>> So I'm thinking that escaping the string needs to be done. It can't be\n>> that big of a deal ;)\n>\n> Gross. I guess backslash-escaping or something similar would be pretty\n> easy to implement.\n>\n\nAs I said in a follow-up mail; Windows' credential stuff has a\ngeneral-purpose application-defined data-store. I can use that\ninstead, it turns out.\n\n>> > Like many other parts of git, we treat the data as binary goo as much as\n>> > possible. So git hands the helper whatever bytes the user provided, and\n>> > ships off whatever bytes are provided by the helper over http without\n>> > any further processing. The only two exceptions are:\n>> [...]\n>>\n>> ASCII unsernames might be common in the UNIX-world, but in the Windows\n>> world this is very much not the case. These functions can be used for\n>> all kinds of services, so I don't think assuming ASCII makes much\n>> sense.\n>>\n>> And since OSX documents the encoding, I'm guessing that non-ASCII\n>> usernames isn't entirely unheard of there either.\n>\n> But what does it mean to have an encoding for credentials that will be\n> sent over http basic-auth? As far as I know, the binary bytes are simply\n> base64-encoded and sent to the server in a header, and there is no room\n> for an encoding or any normalization in the password. You just hope that\n> the remote side is using the same encoding as you, and that there are no\n> normalization issues (e.g., your username contains \"e with accent\", but\n> your input system provides \"e\" + \"combining accent\").\n>\n> It looks like there is a draft rfc to try to help with this:\n>\n>  http://tools.ietf.org/html/draft-reschke-basicauth-enc-05\n>\n> but I have no idea if it is adopted at all yet.\n>\n\nSure, but this is a problem with HTTP-auth, we don't have to make it a\nproblem of our credential-api as well. For instance,  we could try to\nsend the credentials to the server as UTF-8 first, and if that fails\nre-try with ISO-8859-1. If neither works, we probably need more info;\nif it's worth it somebody could implement a per-remote locale or\nsomething. I'm not saying we should, but it's something that could be\ndone if the HTTP-auth issue proves to be real.\n\nLosing the encoding EARLY isn't any better than trying to preserve it\nfor as long as possible, and only lose it when we KNOW we're sending\nthis down a path where the encoding is undefined? And passing UTF-8\ndata when the encoding is undefined is probably one of the less insane\nthings to attempt, no?\n\n>> In general, I think the whole \"let's try to get away with not\n>> specifying encoding\" is a bit dangerous. Without knowing what encoding\n>> the input and output is, the helpers are pretty much useless for\n>> people.\n>\n> Not necessarily. By being a pass-through, things will usually just work.\n> That is, if you hand the helper UTF-8 to store, then it should hand you\n> back UTF-8. Ditto for any other encoding. And if it doesn't work, it's\n> not the helper's problem, but a mismatch between the http client and\n> server. The helper's job is to reproduce some bytes.\n>\n\nThe problem is that for some helpers you need to know the encoding in\norder to have a user-interface show the data. This goes both for OSX\nand Windows. If you mess up the encoding, in the helper, then you'll\nmess up the entries in the OS'es key-managers. While this might not be\na problem in practice for US-English users, we've seen for Git for\nWindows that e.g. some Asians are really unhappy with their username\nbecoming some completely incomprehensible string.\n\nSaying \"Hey, I'm just passing on what I got here, don't blame the\nmessenger\" isn't a solution, it's adding to the problem. I'm sorry if\nI'm a bit blunt here, but being a pass-through is a sorry excuse. User\nnames and passwords are text-strings by definition; \"name\" and \"word\"\nboth imply it. No one has a binary blob of 0xbadf00d as a username or\npassword.\n\nNow, my suggestion was to define that these are UTF-8; if we pass it\nUTF-8 it will still give back UTF-8. But it can be converted\ninternally so the OS can show the credential in it's manager.\n\n>> Sure, saying \"at least ASCII\" helps, but it just takes us\n>> halfway there. And, I think UTF-8 is the least insane option here.\n>\n> Of course. UTF-8 is pretty much always the least insane option, and I\n> would hope that everybody is using it. But by being a pass-through, we\n> don't restrict it. If the client and server both want to use Shift JIS,\n> then the stock git helpers will work fine. On systems where the OS\n> mandates a particular encoding (I didn't even check for the OS X helper,\n> but let us imagine it will barf on non-UTF8 data), then I would expect\n> everybody to be using that encoding, and the pass-through just works.\n>\n> If your helper storage really cares, then I think it's sane to assume\n> you'll get UTF-8 input, and to pass that back out.\n\nUTF-8-by-convention is just pretending the issue wasn't there; why\ncan't we simply fix it? I really don't think we should actively try to\ninherit HTTP-auth's problems...\n\n> But I really didn't\n> want to open the can of worms that is having \"getpass()\" figure out what\n> encoding it is getting from the user.\n\nWhat can of worms? Shouldn't getpass simply get the password in the\ncurrent system locale? When the user inputs the string is exactly the\nmoment we want to convert the \"external locale\" into a known encoding.\n"},{"id":"188482","messageId":"20120404112341.GA28354@sigill.intra.peff.net","threadId":"30000","inReplyTo":"CABPQNSYRxZHSW_urpH7yh=Bct2-X2TAsd3=jaJF_9Rb-foegLA@mail.gmail.com","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-04T11:23:42Z","receivedAt":"2012-04-04T11:23:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 04, 2012 at 12:37:25PM +0200, Erik Faye-Lund wrote:\n\n> Sure, but this is a problem with HTTP-auth, we don't have to make it a\n> problem of our credential-api as well. For instance,  we could try to\n> send the credentials to the server as UTF-8 first, and if that fails\n> re-try with ISO-8859-1. If neither works, we probably need more info;\n> if it's worth it somebody could implement a per-remote locale or\n> something. I'm not saying we should, but it's something that could be\n> done if the HTTP-auth issue proves to be real.\n\nUgh. The retry thing is just hideous. I'd much rather wait for a real\nsolution like the draft rfc I mentioned, and then implement that (or\nbetter yet, it is probably something that curl would implement). But in\nthe meantime, I haven't seen a single report on the list that what git\nis doing is a problem, and I'd much rather come to the same solution\nthat browsers do than invent some git-specific junk.\n\n> Losing the encoding EARLY isn't any better than trying to preserve it\n> for as long as possible, and only lose it when we KNOW we're sending\n> this down a path where the encoding is undefined?\n\nI see what you're saying, but there are two points against this:\n\n  1. We don't necessarily even know what the original encoding is.\n\n  2. All code paths currently end in a place where we drop the encoding\n     (http auth and certificate passwords). So it's sort of a non-issue\n     at this point.\n\n> And passing UTF-8 data when the encoding is undefined is probably one\n> of the less insane things to attempt, no?\n\nNot necessarily. If the user types latin1 and their webserver expects\nlatin1, then we will be breaking their setup. Preserving the bytes as-is\nmeans we will do the right thing in that case. It means we will do the\nwrong thing when the user types latin1 and the webserver expects utf-8,\nbut that is _already_ broken even without credential helpers. Have we\nseen any bug reports?\n\n> The problem is that for some helpers you need to know the encoding in\n> order to have a user-interface show the data. This goes both for OSX\n> and Windows. If you mess up the encoding, in the helper, then you'll\n> mess up the entries in the OS'es key-managers. While this might not be\n> a problem in practice for US-English users, we've seen for Git for\n> Windows that e.g. some Asians are really unhappy with their username\n> becoming some completely incomprehensible string.\n\nHow are they inputting their username? If they are seeing garbage, then\nsurely it is coming to git in some encoding that is different than what\nthe password manager expects. Then we give it to a helper, which hands\nit to system storage, which assumes that what we give it is utf-8. So\nthe error there seems to me to be the layer between the helper and the\nsystem storage, where the string is converted from \"a stream of bytes\"\nto \"a utf-8 encoded string\".\n\nSo why is this a credential helper protocol problem, and not an\nimplementation issue for the specific helper? How does git find out the\nencoding? Presumably by checking $LANG, or whatever system-specific\nlocale information there is. Can't the helper do the same thing?\n\n> Saying \"Hey, I'm just passing on what I got here, don't blame the\n> messenger\" isn't a solution, it's adding to the problem. I'm sorry if\n> I'm a bit blunt here, but being a pass-through is a sorry excuse. User\n> names and passwords are text-strings by definition; \"name\" and \"word\"\n> both imply it. No one has a binary blob of 0xbadf00d as a username or\n> password.\n\nI never argued that people are putting in binary blobs. I do argue that\nwe don't necessarily know the encoding of the input, nor what encoding\nis expected for the output. So by preserving bytes, we can at least not\nmake the situation worse.  The username git sees is some bytes from a\nfile handle. How do we know what encoding it's in? The concept of \"text\nstring\" is not well-defined in the Unix world, and data tends to be\nconsidered as streams of bytes.\n\nWe can guess from $LANG, though I'm not sure how foolproof that is in\npractice (more on that below).\n\n> Now, my suggestion was to define that these are UTF-8; if we pass it\n> UTF-8 it will still give back UTF-8. But it can be converted\n> internally so the OS can show the credential in it's manager.\n\nWhich implies that git must be converting what the user gives us into\nUTF-8. And then converting it back (to what?) when we get the data back\nfrom the helper, before we send it to the webserver? Or do we always\nsend UTF-8 to the webserver? Won't that break setups that currently\nwork?\n\n> What can of worms? Shouldn't getpass simply get the password in the\n> current system locale? When the user inputs the string is exactly the\n> moment we want to convert the \"external locale\" into a known encoding.\n\nIs that always accurate on every system? If so, then why don't we do it\nin every place that we take input from the user? I suspect the answer is\na fear that we would end up corrupting data as often as we end up\nhelping.\n\nMaybe that fear is unfounded (it is not my decision, but rather general\ngit design). Maybe this is just a Unix thing, where the attitude is\nusually that data is a stream of bytes. I don't know.\n\n-Peff\n"},{"id":"188501","messageId":"7vaa2r9ytl.fsf@alter.siamese.dyndns.org","threadId":"30000","inReplyTo":"20120404112341.GA28354@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] contrib: add win32 credential-helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-04T17:44:22Z","receivedAt":"2012-04-04T17:44:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Apr 04, 2012 at 12:37:25PM +0200, Erik Faye-Lund wrote:\n> ...\n>> The problem is that for some helpers you need to know the encoding in\n>> order to have a user-interface show the data. This goes both for OSX\n>> and Windows. If you mess up the encoding, in the helper, then you'll\n>> mess up the entries in the OS'es key-managers. While this might not be\n>> a problem in practice for US-English users, we've seen for Git for\n>> Windows that e.g. some Asians are really unhappy with their username\n>> becoming some completely incomprehensible string.\n>\n> How are they inputting their username? If they are seeing garbage, then\n> surely it is coming to git in some encoding that is different than what\n> the password manager expects. Then we give it to a helper, which hands\n> it to system storage, which assumes that what we give it is utf-8. So\n> the error there seems to me to be the layer between the helper and the\n> system storage, where the string is converted from \"a stream of bytes\"\n> to \"a utf-8 encoded string\".\n>\n> So why is this a credential helper protocol problem, and not an\n> implementation issue for the specific helper? How does git find out the\n> encoding? Presumably by checking $LANG, or whatever system-specific\n> locale information there is. Can't the helper do the same thing?\n\nFWIW, this matches exactly my reaction.\n\nIf a helper on a specific system somehow *knows* the user means to give\nUTF-8, uses system UI that interacts with the user and gives UTF-16 back,\nand uses system store that takes UCS-2, it is entirely sensible for the\nhelper to convert between these encodings internally and use UTF-8 when\ninteracting with credential-helper API which only expects \"stream of\nbytes\".  But that is just a specific implementation detail of the helper.\n"}]}